Re: [PATCH] Btrfs: fix BUG_ON in btrfs_mark_buffer_dirty

[Liu Bo <bo.li.liu@oracle.com>]

On Wed, Nov 23, 2016 at 05:48:45PM +0000, Filipe Manana wrote:
> On Wed, Nov 23, 2016 at 1:15 PM, Filipe Manana <fdmanana@gmail.com> wrote:
> > On Mon, Oct 17, 2016 at 4:44 PM, Liu Bo <bo.li.liu@oracle.com> wrote:
> >> On Mon, Oct 17, 2016 at 03:00:25PM +0200, David Sterba wrote:
> >>> On Thu, Oct 13, 2016 at 09:47:11AM +0100, Filipe Manana wrote:
> >>> > > Since the crash is similar to the call chains from Jeff's report,
> >>> > > ie.
> >>> > > btrfs_del_csums
> >>> > >   -> btrfs_search_slot
> >>> > >      -> btrfs_cow_block
> >>> > >         -> btrfs_mark_buffer_dirty
> >>> > >
> >>> > > I just wonder that whether 4.8.0-rc8-btrfs-next-35+ has
> >>> > >
> >>> > > "[PATCH] Btrfs: fix BUG_ON in btrfs_mark_buffer_dirty" ?
> >>> >
> >>> > It isn't there, this 4.8.0-rc8-btrfs-next-35+ is a checkout of Chris'
> >>> > for-linus-4.9 branch.
> >>> > That patch should have been there, I was convinced that all these
> >>> > related patches were already there, as it's impossible to run xfstests
> >>> > with the integrity checker enabled.
> >>>
> >>> The referenced patch is the one in this thread, no? You've reported that
> >>> even with that applied you can still reproduce a crash with integrity
> >>> checker enabled. I haven't queued it as it seems it's an incomplete fix,
> >>> thus waiting for another version.
> >>
> >> Yes, it's one of three patches in this thread, and they fixed different
> >> problems,
> >>
> >> - the original patch and its v2 are to make check_leaf check non-root
> >> leaf with zero-item,
> >> - "[PATCH] Btrfs: fix BUG_ON in btrfs_mark_buffer_dirty" is to fix
> >> check_leaf, which fixes the crash from Jeff's.
> >> - "[PATCH] Btrfs: remove unnecessary btrfs_mark_buffer_dirty in split_leaf"
> >>  is targeting a different crash with check integrity enabled, which
> >> comes from Filipe's report.
> >>
> >> So to make sure I understand the whole thing, Filipe, can you reproduce the
> >> crash around btrfs_del_csums() after applying this patch
> >>  "[PATCH] Btrfs: fix BUG_ON in btrfs_mark_buffer_dirty"?
> >
> > So indeed, what is missing is patch from this thread, subject "Btrfs:
> > fix BUG_ON in btrfs_mark_buffer_dirty".
> > That is clearly missing in Linus' tree:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/?id=refs%2Ftags%2Fv4.9-rc6&qt=author&q=liu+bo
> >
> > On the other hand, the other patch attached later to this thread, that
> > removes the unnecessary btrfs_mark_buffer_dirty() was sent to Linus:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=196e02490c934398f894e5cb0ee1ac8ad13ca576
> >
> > It seems I'm the only one running xfstests with the integrity checker
> > enabled... Because it fails right away at btrfs/001 either with
> > 4.9-rcs or Chris' for-linus-4.9.
> > Applying "Btrfs: fix BUG_ON in btrfs_mark_buffer_dirty" fixes it.
> 
> Can you please resend the patch with a cc stable tag? Like:
> 
>     Fixes: 1ba98d086fe3 (Btrfs: detect corruption when non-root leaf
> has zero item)
>     Cc: stable@vger.kernel.org  # 4.8+
> 
> Since the offending patch is in 4.8 already.

Sure, will do.

> 
> Also I spoke too early before. Even with that patch we still one more
> issue: https://patchwork.kernel.org/patch/9444045/

Had given it a reviewed-by.

Thanks,

-liubo
> 
> 
> Thanks Bo.
> 
> >
> > thanks
> >
> >
> >
> >>
> >> Thanks,
> >>
> >> -liubo
> >
> >
> >
> > --
> > Filipe David Manana,
> >
> > "People will forget what you said,
> >  people will forget what you did,
> >  but people will never forget how you made them feel."
> 
> 
> 
> -- 
> Filipe David Manana,
> 
> "People will forget what you said,
>  people will forget what you did,
>  but people will never forget how you made them feel."