On Tue, Jul 19, 2016 at 11:43:52AM +0300, Alex Lyakas wrote:
> Hello Holger,
>
> Thank you for your response. I see that xfs_finish_page_writeback()
> has been added very recently and is called from xfs_destroy_ioend().
> In my kernel (3.18.19), the xfs_destroy_ioend() is [1]. I think it
> doesn't suffer from the problem of xfs_finish_page_writeback().
> Looking at other usage of "b_this_page" in my kernel, they all seem
> valid, and similar to what Linus's tree has.
>
> Looking at b_private usage to link buffer heads, the only suspicious
> code is in xfs_submit_ioend():
>
> for (bh = ioend->io_buffer_head; bh; bh = bh->b_private) {
>
> if (!bio) {
> retry:
> bio = xfs_alloc_ioend_bio(bh);
> } else if (bh->b_blocknr != lastblock + 1) {
> xfs_submit_ioend_bio(wbc, ioend, bio);
> goto retry;
> }
>
> if (xfs_bio_add_buffer(bio, bh) != bh->b_size) {
> xfs_submit_ioend_bio(wbc, ioend, bio);
> goto retry;
> }
>
> lastblock = bh->b_blocknr;
> }
>
> Can it happen that when the for loop does "bh = bh->b_private", the
> bh has already been completed and freed?
> With this in mind, the "goto retry" also seem suspicious for the
> same reason.
>
> What do you think?
No, because the bh cannot run completion callbacks (via
xfs_destroy_ioend) while there is an active reference on the ioend.
The reference protecting submission is not dropped until after the
entire loop above is finished and xfs_finish_ioend() is called.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|