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 A925AC25B75 for ; Fri, 31 May 2024 06:14:06 +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=dYYsmlXKEUxHz8QVrMBU9eoNlXBtBUwe4FCxqYfhplc=; b=OHME78yIyBEj5U dwMax9rzGxetYYNdYQXZTrfcOGz9Fe0119p5fnnXH+pmiTXsdufWLBFCiFDqvglbNTgqJ1kuNISwj dsDsFSdZuJe3RVKCiB45O3j33BdF7quqQUWn/5VBWyTxsnpkhkdYQaLdWU11OvrEkAYJ6s5IDndTj BQ33vtwbO36ZswLw0Xqmh3Y0ElexDkvkJODn9hd7jBZ8APjHOjmZ/TujP+zYsbl95MlegMFhkyo8Q 94xxtafcxlKRAFP3dfaYfKGGPxK6BepHyMJLvDy7+YdY0UrYHfB4Pyy8JtAMFfT2R3jWKU62Ib9IM hTe9h4n30Wht7TgxDhdw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sCvWV-00000009Mn4-2DQb; Fri, 31 May 2024 06:13:55 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sCvWS-00000009Mm9-0kAR for linux-arm-kernel@lists.infradead.org; Fri, 31 May 2024 06:13:53 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 5C48062993; Fri, 31 May 2024 06:13:50 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B58E6C116B1; Fri, 31 May 2024 06:13:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1717136030; bh=RwsGqrGzZfc9T/Pyy9DhR5luwagmo4FWO+rh19v6NW4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=nPzdf9Cu3FvihQInblNSELG24n3AUYO8ACQqjIF71UQEdW0RAKQXMUtjS8K8Shc7a YAiTOZcRYARi8CC4DQCCjmcDoenzTNkHYLftgLTMQ2AeGte60hnswj4r3bQ/oQSre1 dIzVSG/Xe3+H2tEkNr+X1bQKk9kMJwuHnYepOaMySGo1mknj9DWP6koxt8/erQlXIU AlnmPkKU7U15QlDoIE2B0kaH19jThQ3TZJBKNjmMZMoVJHBP37Rx1vO6mABI5xJXh0 auEsg/ljvHUxhtelB7bkSs6AGGs8Pi7TX8rue/+b27ltXRDmQty1pb2CJgAIVLnykb 5vMMJxUEyUGxg== Date: Thu, 30 May 2024 23:13:48 -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, ardb@kernel.org, samitolvanen@google.com, bvanassche@acm.org Subject: Re: [PATCH v3 6/8] fsverity: improve performance by using multibuffer hashing Message-ID: <20240531061348.GG6505@sol.localdomain> References: <20240507002343.239552-7-ebiggers@kernel.org> 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-20240530_231352_382598_209487F7 X-CRM114-Status: GOOD ( 21.70 ) 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 Fri, May 31, 2024 at 12:50:20PM +0800, Herbert Xu wrote: > Eric Biggers wrote: > > > > + if (multibuffer) { > > + if (ctx->pending_data) { > > + /* Hash and verify two data blocks. */ > > + err = fsverity_hash_2_blocks(params, > > + inode, > > + ctx->pending_data, > > + data, > > + ctx->hash1, > > + ctx->hash2); > > + kunmap_local(data); > > + kunmap_local(ctx->pending_data); > > + ctx->pending_data = NULL; > > + if (err != 0 || > > + !verify_data_block(inode, vi, ctx->hash1, > > + ctx->pending_pos, > > + ctx->max_ra_pages) || > > + !verify_data_block(inode, vi, ctx->hash2, > > + pos, ctx->max_ra_pages)) > > + return false; > > + } else { > > + /* Wait and see if there's another block. */ > > + ctx->pending_data = data; > > + ctx->pending_pos = pos; > > + } > > + } else { > > + /* Hash and verify one data block. */ > > + err = fsverity_hash_block(params, inode, data, > > + ctx->hash1); > > + kunmap_local(data); > > + if (err != 0 || > > + !verify_data_block(inode, vi, ctx->hash1, > > + pos, ctx->max_ra_pages)) > > + return false; > > + } > > + pos += block_size; > > I think this complexity is gross. Look at how we did GSO in > networking. There should be a unified code-path for aggregated > data and simple data, not an aggregated path versus a simple path. > > I think ultimately it stems from the fact that this code went from > ahash to shash. What were the issues back then? If it's just vmalloc > we should fix ahash to support that, rather than making users of the > Crypto API go through contortions like this. It can't be asynchronous, period. As I've explained, that would be far too complex, and it would also defeat the purpose because it would make performance worse. Messages *must* be queued up and hashed in the caller's context. What could make sense would be some helper functions and an associated struct for queueing up messages for a particular crypto_shash, up to its mb_max_msgs value, and then flushing them and retrieving the digests. These would be provided by the crypto API. I think this would address your concern, in that the users (fsverity and dm-verity) would have a unified code path for multiple vs. single blocks. I didn't think it would be worthwhile to go there yet, given that fsverity and dm-verity just want 2x or 1x, and it ends up being simpler and more efficient to handle those cases directly. But we could go with the more general queueing helper functions instead if you feel they should be included from the start. - Eric _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel