[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZQXO27KCTaWvuPPA@casper.infradead.org>
Date: Sat, 16 Sep 2023 16:50:51 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-arch@...r.kernel.org, Nicholas Piggin <npiggin@...il.com>
Subject: Re: [PATCH 02/17] iomap: Protect read_bytes_pending with the
state_lock
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);
Powered by blists - more mailing lists