mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Zhou, Yun" <yun.zhou@windriver.com>
To: David Howells <dhowells@redhat.com>,
	Matthew Wilcox <willy@infradead.org>,
	Christoph Hellwig <hch@infradead.org>,
	Christian Brauner <brauner@kernel.org>
Cc: pc@manguebit.org, netfs@lists.linux.dev,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: Is writeback_iter() missing some error handling? -- was Re: [PATCH] netfs: fix ENOMEM handling in netfs_writepages() to drain all dirty folios
Date: Thu, 23 Jul 2026 11:43:47 +0800	[thread overview]
Message-ID: <6c295e86-4499-474c-9806-c8ec2d7af745@windriver.com> (raw)
In-Reply-To: <82546.1784729043@warthog.procyon.org.uk>

On 7/22/26 22:04, David Howells wrote:
> Yun Zhou <yun.zhou@windriver.com> wrote:
> 
>>   couldnt_start:
>>        if (error == -ENOMEM) {
>> -             folio_redirty_for_writepage(wbc, folio);
>> -             folio_unlock(folio);
>> -             folio = writeback_iter(mapping, wbc, folio, &error);
>> -             WARN_ON_ONCE(folio != NULL);
>> +             /* Redirty all dirty folios and let writeback retry later. */
>> +             do {
>> +                     folio_redirty_for_writepage(wbc, folio);
>> +                     folio_unlock(folio);
>> +             } while ((folio = writeback_iter(mapping, wbc, folio, &error)));
>>        } else {
>>                netfs_kill_dirty_pages(mapping, wbc, folio);
>>        }
> 
> This seems like the wrong thing to do - or, at least, a bug in the
> writeback_iter() API.  Getting something like ENOMEM would seem to indicate
> that all subsequent writeback_iter() calls in this loop are pointless as it
> looks like the sequence will just go { lock, undirty, dirty, unlock } for each
> folio.
> 

Thanks for the feedback. I understand the concern — the loop is indeed 
doing mechanical { redirty, unlock } work that ideally writeback_iter() 
could handle internally when the caller has nothing meaningful to do 
with the folios.

However, given the current writeback_iter() API contract ("callers must 
keep calling until it returns NULL"), I'm not sure there's a cleaner way 
to handle this without a VFS/mm level change. And in the meantime, 
syzbot is hitting this — the WARN fires and any folio returned beyond 
the first is left locked.

Would it be acceptable to keep this as a minimal stopgap fix for the 
immediate bug, with the understanding that it can be cleaned up later if 
writeback_iter() gains an early-abort mechanism? Happy to adjust the 
approach if you have something else in mind.

BR,
Yun

  reply	other threads:[~2026-07-23  3:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  5:25 Yun Zhou
2026-07-07  6:13 ` Zhou, Yun
2026-07-19 12:32 ` Zhou, Yun
2026-07-22 14:04 ` Is writeback_iter() missing some error handling? -- was " David Howells
2026-07-23  3:43   ` Zhou, Yun [this message]
2026-07-23 12:44   ` Matthew Wilcox
2026-07-23  4:59 ` Christoph Hellwig
2026-07-23  7:25   ` Zhou, Yun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6c295e86-4499-474c-9806-c8ec2d7af745@windriver.com \
    --to=yun.zhou@windriver.com \
    --cc=brauner@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=hch@infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netfs@lists.linux.dev \
    --cc=pc@manguebit.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®