Re: [PATCH v3 00/13] btrfs: support read-write for subpage metadata

[riteshh <riteshh@linux.ibm.com>]

On 21/04/19 09:24PM, Qu Wenruo wrote:
> [...]
> > >
> > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > > index 45ec3f5ef839..49f78d643392 100644
> > > --- a/fs/btrfs/file.c
> > > +++ b/fs/btrfs/file.c
> > > @@ -1341,7 +1341,17 @@ static int prepare_uptodate_page(struct inode
> > > *inode,
> > >                         unlock_page(page);
> > >                         return -EIO;
> > >                 }
> > > -               if (page->mapping != inode->i_mapping) {
> > > +
> > > +               /*
> > > +                * Since btrfs_readpage() will get the page unlocked, we
> > > have
> > > +                * a window where fadvice() can try to release the page.
> > > +                * Here we check both inode mapping and PagePrivate() to
> > > +                * make sure the page is not released.
> > > +                *
> > > +                * The priavte flag check is essential for subpage as we
> > > need
> > > +                * to store extra bitmap using page->private.
> > > +                */
> > > +               if (page->mapping != inode->i_mapping ||
> > > PagePrivate(page)) {
> >   ^ Obviously it should be !PagePrivate(page).
>
> Hi Ritesh,
>
> Mind to have another try on generic/095?
>
> This time the branch is updated with the following commit at top:
>
> commit d700b16dced6f2e2b47e1ca5588a92216ce84dfb (HEAD -> subpage,
> github/subpage)
> Author: Qu Wenruo <wqu@suse.com>
> Date:   Mon Apr 19 13:41:31 2021 +0800
>
>     btrfs: fix a crash caused by race between prepare_pages() and
>     btrfs_releasepage()
>
> The fix uses the PagePrivate() check to avoid the problem, and passes
> several generic/auto loops without any sign of crash.
>
> But considering I always have difficult in reproducing the bug with previous
> improper fix, your verification would be very helpful.
>

Hi Qu,

Thanks for the patch. I did try above patch but even with this I could still
reproduce the issue.

1. I think the original problem could be due to below logs.
	[   79.079641] run fstests generic/095 at 2021-04-21 06:46:23
	<...>
	[   83.634710] Page cache invalidation failure on direct I/O.  Possible data corruption due to collision with buffered I/O!

Meaning, there might be a race here between DIO and buffered IO.
So from DIO path we call invalidate_inode_pages2_range(). Somehow this maybe
causing call of btrfs_releasepage().

Now from code, invalidate_inode_pages2_range() can be called from both
__iomap_dio_rw() and from iomap_dio_complete(). So it is not clear as to from
where this might be triggering this bug.

I will try and debug more. But I thought I will update you with above findings.

-ritesh