Re: Is writeback_iter() missing some error handling? -- was Re: [PATCH] netfs: fix ENOMEM handling in netfs_writepages() to drain all dirty folios

[Zhou, Yun]

On 7/22/26 22:04, David Howells wrote:
> Yun Zhou <yun.zhou@xxxxxxxxxxxxx> 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