Re: [PATCH v2 12/16] iomap: add bias for async read requests

[Christoph Hellwig <hch@infradead.org>]

On Tue, Sep 16, 2025 at 12:14:05PM -0700, Joanne Koong wrote:
> > I think you're right, this is probably clearer without trying to share
> > the function.
> >
> > I think maybe we can make this even simpler. Right now we mark the
> > bitmap uptodate every time a range is read in but I think instead we
> > can just do one bitmap uptodate operation for the entire folio when
> > the read has completely finished.  If we do this, then we can make
> > "ifs->read_bytes_pending" back to an atomic_t since we don't save one
> > atomic operation from doing it through a spinlock anymore (eg what
> > commit f45b494e2a "iomap: protect read_bytes_pending with the
> > state_lock" optimized). And then this bias thing can just become:
> >
> > if (ifs) {
> >     if (atomic_dec_and_test(&ifs->read_bytes_pending))
> >         folio_end_read(folio, !ret);
> >     *cur_folio_owned = true;
> > }
> >
> 
> This idea doesn't work unfortunately because reading in a range might fail.

As in the asynchronous read generats an error, but finishes faster
than the submitting context calling the atomic_dec_and_test here?

Yes, that is possible, although rare.  But having a way to pass
that information on somehow.  PG_uptodate/folio uptodate would make
sense for that, but right now we expect folio_end_read to set that.
And I fail to understand the logic folio_end_read - it should clear
the locked bit and add the updatodate one, but I have no idea how
it makes that happen.