> +static void __iomap_finish_folio_read(struct folio *folio, size_t off,
> + size_t len, int error, bool update_bitmap)
> {
> struct iomap_folio_state *ifs = folio->private;
> bool uptodate = !error;
> @@ -340,7 +340,7 @@ void iomap_finish_folio_read(struct folio *folio, size_t off, size_t len,
> unsigned long flags;
>
> spin_lock_irqsave(&ifs->state_lock, flags);
> - if (!error)
> + if (!error && update_bitmap)
> uptodate = ifs_set_range_uptodate(folio, ifs, off, len);
This code sharing keeps confusing me a bit. I think it's technically
perfectly fine, but not helpful for readability. We'd solve that by
open coding the !update_bitmap case in iomap_read_folio_iter. Which
would also allow to use spin_lock_irq instead of spin_lock_irqsave there
as a nice little micro-optimization. If we'd then also get rid of the
error return from ->read_folio_range and always do asynchronous error
returns it would be even simpler.
Or maybe I just need to live with the magic bitmap update, but the
fact that "len" sometimes is an actual length, and sometimes just a
counter for read_bytes_pending keeps confusing me
