Received-SPF: pass (google.com: domain of linux-crypto+bounces-4622-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Fri, 31 May 2024 11:51:26 -0700
From: Eric Biggers <ebiggers@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org, fsverity@lists.linux.dev,
	dm-devel@lists.linux.dev, x86@kernel.org,
	linux-arm-kernel@lists.infradead.org, ardb@kernel.org,
	samitolvanen@google.com, bvanassche@acm.org
Subject: Re: [PATCH v3 6/8] fsverity: improve performance by using
 multibuffer hashing
Message-ID: <20240531185126.GA1153@sol.localdomain>
References: <20240507002343.239552-7-ebiggers@kernel.org>
 <ZllXDOJKW2pHWBTz@gondor.apana.org.au>
 <20240531061348.GG6505@sol.localdomain>
 <20240531065258.GH6505@sol.localdomain>
 <ZlmPGEt68OyAfuWo@gondor.apana.org.au>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZlmPGEt68OyAfuWo@gondor.apana.org.au>

On Fri, May 31, 2024 at 04:49:28PM +0800, Herbert Xu wrote:
> On Thu, May 30, 2024 at 11:52:58PM -0700, Eric Biggers wrote:
> >
> > Looking at it again a bit more closely, both fsverity and dm-verity have
> > per-block information that they need to keep track of in the queue in addition
> > to the data buffers and hashes: the block number, and in dm-verity's case also a
> > bvec_iter pointing to that block.
> 
> Again I'm not asking you to make this API asynchronous at all.

What exactly are you suggesting, then?  It seems that you want multibuffer
hashing to be supported by the existing ahash API.  However, that only works if
it's made asynchronous, given how the messages would have to be queued up on a
global queue.  That has a huge number of issues which I've already explained.
(And it was even tried before, and it failed.)

> I was just commenting on the added complexity in fsverify due to
> the use of the linear shash API instead of the page-based ahash API.

It's the other way around.  The shash version is much simpler.  Just look at the
diff of commit 8fcd94add6c5 that changed from ahash to shash:

    4 files changed, 71 insertions(+), 200 deletions(-)

> This complexity was then compounded by the multi-buffer support.

fsverity and dm-verity will have to be updated to use multibuffer hashing
anyway, given that transparently supporting it in the existing API is not
viable.  If your concern is that in my current patchset fsverity and dm-verity
have separate code paths for multibuffer vs. single-buffer, as I mentioned I can
address that by restructuring them to operate on arrays (similar to what I
already did with the crypto API part).

> I think this would look a lot simpler if it moved back to ahash.
> 
> The original commit mentioned that ahash was bad for fsverify
> because of vmalloc.  But the only use of linear pointers in fsverify
> seems to be from kmalloc.  Where is the vmalloc coming from?

XFS is working on adding support for fsverity, and XFS was planning to provide
their Merkle tree blocks in vmalloced buffers.  Their plans have shifted several
times, and I think their latest plan no longer uses vmalloced buffers.  But in
any case it's still very convenient and much simpler to be able to just use
virtual addresses without having to worry about what type of memory it is.

- Eric