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

On 7/22/26 22:04, David Howells wrote:
> Yun Zhou <[email protected]> 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