Re: [PATCH v4 6/8] fsverity: improve performance by using multibuffer hashing

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Mon, Jun 03, 2024 at 11:37:29AM -0700, Eric Biggers wrote:
>
> +	for (i = 0; i < ctx->num_pending; i++) {
> +		data[i] = ctx->pending_blocks[i].data;
> +		outs[i] = ctx->pending_blocks[i].hash;
> +	}
> +
> +	desc->tfm = params->hash_alg->tfm;
> +	if (params->hashstate)
> +		err = crypto_shash_import(desc, params->hashstate);
> +	else
> +		err = crypto_shash_init(desc);
> +	if (err) {
> +		fsverity_err(inode, "Error %d importing hash state", err);
> +		return false;
> +	}
> +	err = crypto_shash_finup_mb(desc, data, params->block_size, outs,
> +				    ctx->num_pending);
> +	if (err) {
> +		fsverity_err(inode, "Error %d computing block hashes", err);
> +		return false;
> +	}

So with ahash operating in synchronous mode (callback == NULL), this
would look like:

	struct ahash_request *reqs[FS_VERITY_MAX_PENDING_DATA_BLOCKS];

	for (i = 0; i < ctx->num_pending; i++) {
		reqs[i] = fsverity_alloc_hash_request();
		if (!req) {
			free all reqs;
			return false;
		}

		if (params->hashstate)
			err = crypto_ahash_import(&reqs[i], params->hashstate);
		else
			err = crypto_ahash_init(&reqs[i]);

		if (err) {
			fsverity_err(inode, "Error %d importing hash state", err);
			free all reqs;
			return false;
		}
	}

	for (i = 0; i < ctx->num_pending; i++) {
		unsigned more;

		if (params->hashstate)
			err = crypto_ahash_import(req, params->hashstate);
		else
			err = crypto_ahash_init(req);

		if (err) {
			fsverity_err(inode, "Error %d importing hash state", err);
			free all requests;
			return false;
		}

		more = 0;
		if (i + 1 < ctx->num_pending)
			more = CRYPTO_TFM_REQ_MORE;
		ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP | more,
					   NULL, NULL);
		ahash_request_set_crypt(req, ctx->pending_blocks[i].sg,
					ctx->pending_blocks[i].hash,
					params->block_size);

		err = crypto_ahash_finup(req);
		if (err) {
			fsverity_err(inode, "Error %d computing block hashes", err);
			free all requests;
			return false;
		}
	}

You're hiding some of the complexity by not allocating memory
explicitly for each hash state.  This might fit on the stack
for two requests, but eventually you will have to allocate memory.

With the ahash API, the allocation is explicit.

Cheers,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt