Re: kernel BUG at fs/ext4/inode.c:LINE!

[Linus Torvalds]

On Mon, Aug 31, 2020 at 3:03 AM Jan Kara <jack@xxxxxxx> wrote:
>
> On Fri 28-08-20 12:07:55, Jan Kara wrote:
> >
> > Doh, so this is:
> >
> >                         wait_on_page_writeback(page);
> > >>>                     BUG_ON(PageWriteback(page));
> >
> > in mpage_prepare_extent_to_map(). So we have PageWriteback() page after we
> > have called wait_on_page_writeback() on a locked page. Not sure how this
> > could ever happen even less how ext4 could cause this...
>
> I was poking a bit into this and there were actually recent changes into
> page bit waiting logic by Linus. Linus, any idea?

So the main change is that now if somebody does a wake_up_page(), the
page waiter will be released - even if somebody else then set the bit
again (or possible if the waker never cleared it!).

It used to be that the waiter went back to sleep.

Which really shouldn't matter, but if we had any code that did something like

        end_page_writeback();
        .. something does set_page_writeback() on the page again ..

then the old BUG_ON() would likely never have triggered (because the
waiter would have seen the writeback bit being set again and gone back
to sleep), but now it will.

So I would suspect a pre-existing issue that was just hidden by the
old behavior and was basically impossible to trigger unless you hit
*just* the right timing.

And now it's easy to trigger, because the first time somebody clears
PG_writeback, the wait_on_page_writeback() will just return *without*
re-testing and *without* going back to sleep.

Could there be somebody who does set_page_writeback() without holding
the page lock?

Maybe adding a

        WARN_ON_ONCE(!PageLocked(page));

at the top of __test_set_page_writeback() might find something?

Note that it looks like this problem has been reported on Android
before according to that syzbot thing. Ie, this thing:

    https://groups.google.com/g/syzkaller-android-bugs/c/2CfEdQd4EE0/m/xk_GRJEHBQAJ

looks very similar, and predates the wake_up_page() changes.

So it was probably just much _harder_ to hit before, and got easier to hit.

Hmm. In fact, googling for

        mpage_prepare_extent_to_map "kernel BUG"

seems to find stuff going back years. Here's a patchwork discussion
where you had a debug patch to try to figure it out back in 2016:

    https://patchwork.ozlabs.org/project/linux-ext4/patch/20161122133452.GF3973@xxxxxxxxxxxxxx/

although that one seems to be a different BUG_ON() in the same area.

Maybe entirely unrelated, but the fact that this function shows up a
fair amount is perhaps a sign of some long-running issue..

              Linus