fsverity.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Andrey Albershteyn <aalbersh@redhat.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: fsverity@lists.linux.dev
Subject: Re: fs-verity spinlock in is_hash_block_verified()
Date: Thu, 25 Jan 2024 11:42:42 +0100	[thread overview]
Message-ID: <btyieqmpsljzw5ldlepdpyxc2ssvvknnjvfluivzthbliyfsip@ppne3o67hsnw> (raw)
In-Reply-To: <20240125020838.GH1088@sol.localdomain>

> I think you're correct that the spin lock isn't actually needed, considering
> that all the operations are atomic and are properly ordered.  It does seem a
> little weird that without the spinlock, one thread can be clearing a block's
> verified bit concurrently with another thread setting it.  (That can happen if
> the threads enter is_hash_block_verified() at the same time and both see
> !PageChecked, and then one thread gets scheduled out before it clears the bits.)
> But it works out because at worst the block gets verified more than once.  That
> case should be super rare, so removing the spinlock should be better
> performance-wise.  Are you interested in sending a patch that does that?
> 
> - Eric
> 

Thanks for quick reply, yes, I will prepare the patch.

I suppose the rare case you described happens with spinlock too. If
one thread is waiting for spinlock while the second one resets the
bits. If the second thread is rescheduled immediately after spinlock
is unlocked the first thread will see PG_checked=1 bit=0 (same as
the first one) -> both do verification. So, I suppose it should not
be a problem to remove it

-- 
- Andrey


      reply	other threads:[~2024-01-25 10:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-24 23:02 fs-verity spinlock in is_hash_block_verified() Andrey Albershteyn
2024-01-25  2:08 ` Eric Biggers
2024-01-25 10:42   ` Andrey Albershteyn [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=btyieqmpsljzw5ldlepdpyxc2ssvvknnjvfluivzthbliyfsip@ppne3o67hsnw \
    --to=aalbersh@redhat.com \
    --cc=ebiggers@kernel.org \
    --cc=fsverity@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).