On Fri, Jul 30, 2021 at 07:27:41PM +0800, Qu Wenruo wrote:

On 2021/7/30 下午7:08, David Sterba wrote:

quoted

On Fri, Jul 30, 2021 at 01:58:57PM +0800, Qu Wenruo wrote:
Yeah it's hard to believe, but it's been there since almost the
beginning. Back then there was a hard BUG() in the fixup worker, I've
hit it randomly on x86_64,

https://lore.kernel.org/linux-btrfs/20111031154139.GF19328@twin.jikos.cz/ (local)

you could find a lot of other reports where it crashed inside
btrfs_writepage_fixup_worker.

Well, over 10 years ago.

But finally I have seen a real world report for it.

This makes me wonder, wouldn't other sites like iomap, which also 
utilize a private structure for subpage bitmaps, also hit such crash?

I don't know.

quoted

quoted

Fix it by moving set_page_extent_mapped() call before
btrfs_page_clear_error().
And make sure in the error path we won't call anything subpage helper.

I'm not sure about the fix, because the whole fixup thing is not
entirely clear.

Indeed, but the idea should be straightfoward:

Don't call any subpage helper before set_page_extent_mapped().

So that such page without private get passed in, we can still handle it 
well.

quoted

quoted

Fixes: 32443de3382b ("btrfs: introduce btrfs_subpage for data inodes")
Signed-off-by: Qu Wenruo <redacted>
---
I really hope we can have a more explicit comment about in exactly which
cases we can have such page, and maybe some test cases for it.

The only reliable test case was on s390 with a particular seed for fsx,
I still have it stored somewhere. On x86_64 it's very hard to hit.

Can it be reproduced by S390 qemu tcc emulation?

I think the reproducer was deterministic, depending on the fsx seed it
created a file layout that at some point triggered the desync between hw
page and memory management page, requiring the fixup.

So even if the emulation is slow, it should work to verify the test. If
you set that up, you could also restore the BUG() inside the fixup
worker to see for yourself that this mysterious fixup thing is real.

And by S390, did you mean modern Power8/9/10 systems too?

I haven't been testing on Power machines a lot so I don't know, it's
possible that the success rate hitting that would be similar as on
x86_64.

quoted

quoted

In fact, I haven't really seen any case like this, and it doesn't really
make sense for me to make some MM layer code to mark a page dirty
without going through set_page_dirty() interface.

On s390 it's quick because the page state bits are stored in 2 places
and need to be synced. On x86_64 it's very unclear and low level arch
specific MM stuff but it is still a problem.

The hard to hit part is really harming our test coverage...

I don't disagree with that.