On Wed, Sep 3, 2025 at 11:03 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > On Wed, Sep 03, 2025 at 01:26:37PM -0700, Darrick J. Wong wrote: > > On Fri, Aug 29, 2025 at 04:56:13PM -0700, Joanne Koong wrote: > > > The purpose of struct iomap_readpage_ctx's cur_folio_in_bio is to track > > > if the folio needs to be unlocked or not. Rename this to folio_unlocked > > > to make the purpose more clear and so that when iomap read/readahead > > > logic is made generic, the name also makes sense for filesystems that > > > don't use bios. > > > > Hrmmm. The problem is, "cur_folio_in_bio" captures the meaning that the > > (locked) folio is attached to the bio, so the bio_io_end function has to > > unlock the folio. The readahead context is basically borrowing the > > folio and cannot unlock the folio itelf. > > > > The name folio_unlocked doesn't capture the change in ownership, it just > > fixates on the lock state which (imo) is a side effect of the folio lock > > ownership. > > Agreed. Not sure what a good name is in a world where the folio can be > in something else than the bio. Maybe just replace bio with ctx or > similar? cur_folio_in_ctx? cur_folio_locked_by_ctx? I find the ctx naming to be more confusing, the "ctx" imo is too easily confused with the iomap_readfolio_ctx struct. What about "cur_folio_owned" or "cur_folio_handled"? Or keeping it as "cur_folio_unlocked" and adding a comment to explain the change in ownership? > > > > + bool folio_unlocked; > > > > Maybe this ought to be called cur_folio_borrowed? > > I don't think 'borrow' makes much sense here. It's not like we're > borrowing it, we transfer the lock context to the bio (or whatever else > Joanne is going to use for fuse, I haven't read down to that yet).
