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? > > + 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).