On Mon, Jun 09, 2025 at 04:15:27PM -0700, Joanne Koong wrote: > ioends are used in fuse for cleaning up state. fuse implements a > ->submit_ioend() callback in fuse_iomap_submit_ioend() (in patch 7/8). But do you use struct iomap_ioend at all? (Sorry, still don't have a whole tree with the patches applied in front of me). > > Given that the patch that moved things around already wrapped the > > error propagation to the bio into a helpr, how does this differ > > from the main path in the function now? > > > > If we don't add this special casing for IOMAP_IN_MEM here, then in > this function it'll hit the "if (!wpc->ioend)" case right in the > beginning and return error without calling the ->submit_ioend() So this suggests you don't use struct iomap_ioend at all. Given that you add a private field to iomap_writepage_ctx I guess that is where you chain the fuse requests? Either way I think we should clean this up one way or another. If the non-block iomap writeback code doesn't use ioends we should probably move the ioend chain into the private field, and hide everything using it (or even the ioend name) in proper abstraction. In this case this means finding another way to check for a non-empty wpc. One way would be to check nr_folios as any non-empty wbc must have a number of folios attached to it, the other would be to move the check into the ->submit_ioend method including the block fallback. For a proper split the method should probably be renamed, and we'd probably want to require every use to provide the submit method, even if the trivial block users set it to the default one provided. > > > - if (!count) > > > + /* > > > + * If wpc->ops->writeback_folio is set, then it is responsible > > > + * for ending the writeback itself. > > > + */ > > > + if (!count && !wpc->ops->writeback_folio) > > > folio_end_writeback(folio); > > > > This fails to explain why writeback_folio does the unlocking itself. > > writeback_folio needs to do the unlocking itself because the writeback > may be done asynchronously (as in the case for fuse). I'll add that as > a comment in v2. So how do you end up with a zero count here and still want the fuse code to unlock?
