Re: [PATCH 1/2] block: Make __submit_bio_noacct() preserve the bio submission order

[Christoph Hellwig <hch@lst.de>]

On Tue, Jun 10, 2025 at 05:18:03PM -0600, Keith Busch wrote:
> I think you could just prep the encryption at the point the bio is split
> to its queue's limits, and then all you need to do after that is ensure
> the limits don't exceed what the fallback requires (which appears to
> just be a simple segment limitation). It looks like most of the bio
> based drivers split to limits already.

I'm still a bit confused about the interaction of block-crypto and
stacking drivers.  For native support you obviously always want to pass
the crypt context down to the lowest level hardware driver, and dm
has code to support this.  But if you stacking driver is not dm, or
the algorithm is not supported by the underlying hardware it looks
like we might still be able to run the fallback for a stacking
driver.  Or am I missing something here?  Eric/Bart: any chance to
get blktests (or xfstets if that's easier to wire up) to exercise all
these corner cases?

> And if that's all okay, it simplifies quite a lot of things in this path
> because the crypto fallback doesn't need to concern itself with
> splitting anymore. Here's a quick shot a it, but compile tested only:

Exactly.  And in the next step we can significantly simply and speed
up the bio submission recursion protection if __bio_split_to_limits
is the only place that can split/reinject bios for blk-mq drivers.

> diff --git a/block/blk-crypto-profile.c b/block/blk-crypto-profile.c
> index 94a155912bf1c..2804843310cc4 100644
> --- a/block/blk-crypto-profile.c
> +++ b/block/blk-crypto-profile.c
> @@ -459,6 +459,16 @@ bool blk_crypto_register(struct blk_crypto_profile *profile,
>  		pr_warn("Integrity and hardware inline encryption are not supported together. Disabling hardware inline encryption.\n");
>  		return false;
>  	}
> +
> +	if (queue_max_segments(q) > BIO_MAX_VECS) {
> +		struct queue_limits lim;
> +
> +		lim = queue_limits_start_update(q);
> +		lim.max_segments = BIO_MAX_VECS;
> +		if (queue_limits_commit_update(q, &lim))
> +			return false;
> +	}

I think this limit actually only exists when using the fallback, i.e.
when no profile is registered.  And even then only for encrypted bios.

We'll also need to ensure the queue is frozen and deal with limit
changes after the profile registration.  I suspect we should probably
integrate the queue profile with the queue limits, so things are don't
in one place, but that's out of scope for this patch.

It's also not just a segment limit, but also an overall size limit,
i.e. it must be no more than 256 * 4k.