Re: writeback completion soft lockup BUG in folio_wake_bit()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 17, 2022 at 8:04 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> So how about we do something like this:
>
>  - Make folio_start_writeback() and set_page_writeback() return void,
>    fixing up AFS and NFS.
>  - Add a folio_wait_start_writeback() to use in the VFS
>  - Remove the calls to set_page_writeback() in the filesystems

That sounds lovely, but it does worry me a bit. Not just the odd
'keepwrite' thing, but also the whole ordering between the folio bit
and the tagging bits. Does the ordering possibly matter?

That whole "xyz_writeback_keepwrite()" thing seems odd. It's used in
only one place (the folio version isn't used at all):

  ext4_writepage():

     ext4_walk_page_buffers() fails:
                redirty_page_for_writepage(wbc, page);
                keep_towrite = true;
      ext4_bio_write_page().

which just looks odd. Why does it even try to continue to do the
writepage when the page buffer thing has failed?

In the regular write path (ie ext4_write_begin()), a
ext4_walk_page_buffers() failure is fatal or causes a retry). Why is
ext4_writepage() any different? Particularly since it wants to keep
the page dirty, then trying to do the writeback just seems wrong.

So this code is all a bit odd, I suspect there are decades of "people
continued to do what they historically did" changes, and it is all
worrisome.

Your cleanup sounds like the right thing, but I also think that
getting rid of that 'keepwrite' thing would also be the right thing.
And it all worries me.

            Linus



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux