Re: [PATCH 02/17] iomap: Protect read_bytes_pending with the state_lock

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Fri, Sep 15, 2023 at 05:11:55PM -0700, Linus Torvalds wrote:
> I think it ends up looking like this:
> 
>   static void iomap_finish_folio_read(struct folio *folio, size_t off,
>                   size_t len, int error)
>   {
>         struct iomap_folio_state *ifs = folio->private;
>         bool uptodate = true;
>         bool finished = true;
> 
>         if (ifs) {
>                 unsigned long flags;
> 
>                 spin_lock_irqsave(&ifs->state_lock, flags);
> 
>                 if (!error)
>                         uptodate = ifs_set_range_uptodate(folio, ifs,
> off, len);
> 
>                 ifs->read_bytes_pending -= len;
>                 finished = !ifs->read_bytes_pending;
>                 spin_unlock_irqrestore(&ifs->state_lock, flags);
>         }
> 
>         if (unlikely(error))
>                 folio_set_error(folio);
>         else if (uptodate)
>                 folio_mark_uptodate(folio);
>         if (finished)
>                 folio_unlock(folio);
>   }
> 
> but that was just a quick hack-work by me (the above does, for
> example, depend on folio_mark_uptodate() not needing the
> ifs->state_lock, so the shared parts then got moved out).

I really like this.  One tweak compared to your version:

        bool uptodate = !error;
...
        if (error)
                folio_set_error(folio);
        if (uptodate)
                folio_mark_uptodate(folio);
        if (finished)
                folio_unlock(folio);

... and then the later patch becomes

	if (finished)
		folio_end_read(folio, uptodate);