lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ