Re: [PATCH v1 2/2] fs/writeback: skip inodes with potential writeback hang in wait_sb_inodes()

[Joanne Koong <joannelkoong@gmail.com>]

On Mon, Nov 24, 2025 at 5:58 AM David Hildenbrand (Red Hat)
<david@kernel.org> wrote:
>
> On 11/20/25 22:20, Joanne Koong wrote:
> > On Thu, Nov 20, 2025 at 12:23 PM David Hildenbrand (Red Hat)
> > <david@kernel.org> wrote:
> >>
> >> On 11/20/25 19:42, Joanne Koong wrote:
> >>> During superblock writeback waiting, skip inodes where writeback may
> >>> take an indefinite amount of time or hang, as denoted by the
> >>> AS_WRITEBACK_MAY_HANG mapping flag.
> >>>
> >>> Currently, fuse is the only filesystem with this flag set. For a
> >>> properly functioning fuse server, writeback requests are completed and
> >>> there is no issue. However, if there is a bug in the fuse server and it
> >>> hangs on writeback, then without this change, wait_sb_inodes() will wait
> >>> forever.
> >>>
> >>> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> >>> Fixes: 0c58a97f919c ("fuse: remove tmp folio for writebacks and internal rb tree")
> >>> Reported-by: Athul Krishna <athul.krishna.kr@protonmail.com>
> >>> ---
> >>>    fs/fs-writeback.c | 3 +++
> >>>    1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> >>> index 2b35e80037fe..eb246e9fbf3d 100644
> >>> --- a/fs/fs-writeback.c
> >>> +++ b/fs/fs-writeback.c
> >>> @@ -2733,6 +2733,9 @@ static void wait_sb_inodes(struct super_block *sb)
> >>>                if (!mapping_tagged(mapping, PAGECACHE_TAG_WRITEBACK))
> >>>                        continue;
> >>>
> >>> +             if (mapping_writeback_may_hang(mapping))
> >>> +                     continue;
> >>
> >> I think I raised it in the past, but simply because it could happen, why
> >> would we unconditionally want to do that for all fuse mounts? That just
> >> seems wrong :(
> >
> > I think it's considered a userspace regression if we don't revert the
> > program behavior back to its previous version, even if it is from the
> > program being incorrectly written, as per the conversation in [1].
> >
> > [1] https://lore.kernel.org/regressions/CAJnrk1Yh4GtF-wxWo_2ffbr90R44u0WDmMAEn9vr9pFgU0Nc6w@mail.gmail.com/T/#m73cf4b4828d51553caad3209a5ac92bca78e15d2
> >
> >>
> >> To phrase it in a different way, if any writeback could theoretically
> >> hang, why are we even waiting on writeback in the first place?
> >>
> >
> > I think it's because on other filesystems, something has to go
> > seriously wrong for writeback to hang, but on fuse a server can easily
> > make writeback hang and as it turns out, there are already existing
> > userspace programs that do this accidentally.
>
> Sorry, I only found the time to reply now. I wanted to reply in more
> detail why what you propose here does not make sense to me.

No worries at all, thank you for taking the time to write out your thoughts.

>
> I understand that it might make one of the weird fuse scenarios (buggy
> fuse server) work again, but it sounds like we are adding more hacks on
> top of broken semantics. If we want to tackle the writeback problem, we
> should find a proper way to deal with that for good.

I agree that this doesn't solve the underlying problem that folios
belonging to a malfunctioning fuse server may be stuck in writeback
state forever. To properly and comprehensively address that and the
other issues (which you alluded to a bit in section 3 below) would I
think be a much larger effort, but as I understand it, a userspace
regression needs to be resolved more immediately. I wasn't aware that
if the regression is caused by a faulty userspace program, that rule
still holds, but I was made aware of that. Even though there are other
ways that sync could be held up by a faulty/malicious userspace
program prior to the changes that were added in commit 0c58a97f919c
("fuse: remove tmp folio for writebacks and internal rb tree"), I
think the issue is that that commit gives malfunctioning servers
another way, which may be a way that some well-intended but buggy
servers could trigger, which is considered a regression. If it's
acceptable to delay addressing this until the actual solution that
addresses the entire problem, then I agree that this patchset is
unnecessary and we should just wait for the more comprehensive
solution.

>
>
> (1) AS_WRITEBACK_MAY_HANG semantics
>
> As discussed in the past, writeeback of pretty much any filesystem might
> hang forever on I/O errors.
>
> On network filesystems apparently as well fairly easily.
>
> It's completely unclear when to set AS_WRITEBACK_MAY_HANG.
>
> So as writeback on any filesystem may hang, AS_WRITEBACK_MAY_HANG would
> theoretically have to be set on any mapping out there.
>
> The semantics don't make sense to me, unfortuantely.

I'm not sure what a better name here would be unfortunately. I
considered AS_WRITEBACK_UNRELIABLE and AS_WRITEBACK_UNSTABLE but I
think those run into the same issue where that could technically be
true of any filesystem (eg the block layer may fail the writeback, so
it's not completely reliable/stable).

>
>
> (2) AS_WRITEBACK_MAY_HANG usage
>
> It's unclear in which scenarios we would not want to wait for writeback,
> and what the effects of that are.
>
> For example, wait_sb_inodes() documents "Data integrity sync. Must wait
> for all pages under writeback, because there may have been pages dirtied
> before our sync call ...".
>
> It's completely unclear why it might be okay to skip that simply because
> a mapping indicated that waiting for writeback is maybe more sketchy
> than on other filesystems.
>
> But what concerns me more is what we do about other
> folio_wait_writeback() callers. Throwing in AS_WRITEBACK_MAY_HANG
> wherever somebody reproduced a hang is not a good approach.

If I'm recalling this correctly (I'm looking back at this patchset [1]
to trigger my memory), there were 3 cases where folio_wait_writeback()
callers run into issues: reclaim, sync, and migration. The reclaim
issue is addressed. For the migration case, I don't think this results
in any user-visible regressions. Not that the migration case is not a
big issue, I think we should find a proper fix for it, but the
migration stall is already easily caused by a server indefinitely
holding a folio lock, so the writeback case didn't add this stall as a
new side effect.

[1] https://lore.kernel.org/linux-fsdevel/20241122232359.429647-1-joannelkoong@gmail.com/
>
> We need something more robust where we can just not break the kernel in
> weird ways because user space is buggy or malicious.
>
>
> (3) Other operations
>
> If my memory serves me right, there are similar issues on readahead. It
> wouldn't surprise me if there are yet other operations where fuse Et al
> can trick the kernel into hanging forever.
>
> So I'm wondering if there is more to this than just "writeback may hang".
>
>
>
> Obviously, getting the kernel to hang, controlled by user space that
> easily, is extremely unpleasant and probably the thing that I really
> dislike about fuse. Amir mentioned that maybe the iomap changes from
> Darrick might improve the situation in the long run, I would hope it
> would allow for de-nastifying fuse in that sense, at least in some
> scenarios.
>
>
> I cannot really say what would be better here (maybe aborting writeback
> after a short timeout), but AS_WRITEBACK_MAY_HANG to then just skip
> selected waits for writeback is certainly something that does not make
> sense to me.
>
>
> Regarding the patch here, is there a good reason why fuse does not have
> to wait for the "Data integrity sync. Must wait for all pages under
> writeback ..."?
>
> IOW, is the documented "must" not a "must" for fuse? In that case,

Prior to the changes added in commit 0c58a97f919c ("fuse: remove tmp
folio for writebacks and internal rb tree"), fuse didn't ensure that
data was written back for sync. The folio was marked as not under
writeback anymore, even if it was still under writeback.

> having a flag that states something like that that
> "AS_NO_WRITEBACK_WAIT_ON_DATA_SYNC" would probable be what we would want
> to add to avoid waiting for writeback with clear semantics why it is ok
> in that specific scenario.

Having a separate AS_NO_WRITEBACK_WAIT_ON_DATA_SYNC mapping flag
sounds reasonable to me and I agree is more clearer semantically.

>
> Hope that helps, and happy to be convinced why AS_WRITEBACK_MAY_HANG is
> the right thing to do in this way proposed here.

This was helpful, thanks for your thoughts!

>
> --
> Cheers
>
> David