From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4EA40C25B76 for ; Wed, 5 Jun 2024 18:58:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=kx9AHYXFmRnSp+ZySYzBxf8kCYk2revY2XqghmzQ20c=; b=HvWpw0og3mNfOE oX0OCmjTh5uXM+vpwO5HZuBgri6AhJKnKqAJm3bLJE5WD1XqGanZZxBDejkomMwuka+N5O0uZ7Nbs kfQm72m9hsF62UZnbJIyeOKB3R4W7xnEVZ8qruEkxbm+HIDwO27yD+ncNyrL61JxksuCgnRVTY+ll gzh/00e/hd/aWGlgkgjrlrDi5JbWwX6ULEMczIplFTyrum8DTFbsaaQjhQuiYN0jZ62OmI+ROLYPc 8pZE8/jMtwwupzPD7bFfSMTiCJrv+KgJo7dIxe+0lbR30/L6FgT2j1jp+fsV865PwKYsviyM1BnQt ppm7a0QyQ1JWs92eZLXg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEvq0-00000007Eaw-4C2M; Wed, 05 Jun 2024 18:58:21 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sEvpx-00000007EYd-05XD for linux-arm-kernel@lists.infradead.org; Wed, 05 Jun 2024 18:58:18 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 2761F619F2; Wed, 5 Jun 2024 18:58:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 860D9C2BD11; Wed, 5 Jun 2024 18:58:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1717613895; bh=vi1o/e9gmwqnbe2LC/NP+cXWDaYTONv/9BSHBsc5bk4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=M4BXj4xli/ZFwmGLAOvLGljdEnj/zSat1UrSG+Vbc+5Op13rKSxvEJaTy42KV8AO5 xj1iZmqMO5Pp1XZ5k0tvoRVBmHUmuJiDTJV7TU+ZuwEEikh/MvScIoXx5Xi55vNSTg V3eAnKLeaUuAmtsEX0Y2TdlSBbYWp0UDH0d0Kjm7YGnQK8U19BJKMJlVBt2ahFLmja rjeKseIaa4nBnYUZbLFUaMc7u0lV4SeFsi7tzq2CsanAEQD3oFZYWkwZxaEzIGLMvW Ac+ggcG/4Q2+jmh84OMMFnb+7tWUHO4U0uyxU8PRIfhnB6SqwF2BhJ1rSaYXnAvrTN PYy60e4x7Wctw== Date: Wed, 5 Jun 2024 11:58:13 -0700 From: Eric Biggers To: Herbert Xu Cc: linux-crypto@vger.kernel.org, fsverity@lists.linux.dev, dm-devel@lists.linux.dev, x86@kernel.org, linux-arm-kernel@lists.infradead.org, Ard Biesheuvel , Sami Tolvanen , Bart Van Assche Subject: Re: [PATCH v4 6/8] fsverity: improve performance by using multibuffer hashing Message-ID: <20240605185813.GA1222@sol.localdomain> References: <20240603183731.108986-1-ebiggers@kernel.org> <20240603183731.108986-7-ebiggers@kernel.org> <20240604184220.GC1566@sol.localdomain> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240605_115817_218046_316D2228 X-CRM114-Status: GOOD ( 31.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jun 05, 2024 at 05:19:01PM +0800, Herbert Xu wrote: > On Tue, Jun 04, 2024 at 11:42:20AM -0700, Eric Biggers wrote: > > > > This doesn't make any sense, though. First, the requests need to be enqueued > > for the task, but crypto_ahash_finup() would only have the ability to enqueue it > > in a queue associated with the tfm, which is shared by many tasks. So it can't > > OK I screwed up that one. But that's not hard to fix. We could > simply add request chaining: > > ahash_request_chain(req1, req2); > ahash_request_chain(req2, req3); > ... > ahash_request_chain(reqn1, reqn); > > err = crypto_ahash_finup(req1); So after completely changing several times your proposal is getting a bit closer to mine, but it still uses a very clumsy API based around ahash that would be much harder to use and implement correctly. It also says nothing about what the API would look like on the shash side, which would be needed anyway because ahash is almost always just a pointless wrapper for shash. Is there a different API that you are asking for on the shash side? If so, what? > > actually work unless the tfm maintained a separate queue for each task, which > > would be really complex. Second, it adds a memory allocation per block which is > > very undesirable. You claim that it's needed anyway, but actually it's not; > > with my API there is only one initial hash state regardless of how high the > > interleaving factor is. In fact, if multiple initial states were allowed, > > Sure you don't need it for two interleaved requests. But as you > scale up to 16 and beyond, surely at some point you're going to > want to move the hash states off the stack. To reiterate, with my proposal there is only one state in memory. It's a simple API that can't be misused by providing inconsistent properties in the requests. Yes, separate states would be needed if we were to support arbitrary updates, but why add all that complexity before it's actually needed? Also, "16 and beyond" is highly unlikely to be useful for kernel use cases. > > multibuffer hashing would become much more complex because the underlying > > algorithm would need to validate that these different states are synced up. My > > proposal is much simpler and avoids all this unnecessary overhead. > > We could simply state that these chained requests must be on block > boundaries, similar to how we handle block ciphers. This is not a > big deal. ... which would make it useless for most dm-verity users, as dm-verity uses a 32-byte salt with SHA-256 (which has a 64-byte block size) by default. > > > Really the only reason to even consider ahash at all would be try to support > > software hashing and off-CPU hardware accelerators using the "same" code. > > However, your proposal would not achieve that either, as it would not use the > > async callback. Note, as far as I know no one actually cares about off-CPU > > hardware accelerator support in fsverity anyway... > > The other thing is that verity doesn't benefit from shash at all. > It appears to be doing kmap's on every single request. The diff from switching fsverity from ahash to shash clearly demonstrates otherwise. Yes, fsverity has to map the pages to pass into shash, but that's a very minor thing compared to all the complexity of ahash that was saved. And fsverity already had to map most of the pages anyway to access them. - Eric _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel