Re: [PATCH v2 4/4] dm crypt: batch all sectors of a bio per crypto request

Hi

On Wed, 27 May 2026, Leonid Ravich wrote:

> +/*
> + * Multi-data-unit variant of crypt_convert_block_skcipher.  Submits all
> + * remaining sectors of the current bio in one skcipher request whose
> + * data_unit_size is cc->sector_size.  The cipher walks the IV between
> + * data units (see crypto_skcipher_set_data_unit_size()).
> + *
> + * Returns the same set of values as crypt_convert_block_skcipher:
> + *   0 on synchronous success (full chunk processed),
> + *   -EINPROGRESS / -EBUSY on asynchronous dispatch,
> + *   -EAGAIN if the per-bio scatterlist allocation cannot be made.  The
> + *           caller MUST disable multi-data-unit batching for the rest
> + *           of this bio and re-enter the per-sector path, which uses
> + *           only mempool reserves and is therefore safe even on the
> + *           swap-out-to-dm-crypt path under total memory exhaustion.
> + *   negative errno otherwise.
> + *
> + * On success the bio iterators have been advanced by the chunk size.
> + *
> + * Walks the bio with __bio_for_each_bvec so that multi-page folios
> + * produce one scatterlist entry rather than N (one per PAGE_SIZE).
> + */
> +static int crypt_convert_block_skcipher_multi(struct crypt_config *cc,
> +					      struct convert_context *ctx,
> +					      struct skcipher_request *req,
> +					      unsigned int *out_processed)
> +{
> +	const unsigned int sector_size = cc->sector_size;
> +	const gfp_t gfp = GFP_NOIO | __GFP_NORETRY | __GFP_NOWARN;
> +	unsigned int total_in = ctx->iter_in.bi_size;
> +	unsigned int total_out = ctx->iter_out.bi_size;
> +	unsigned int total = min(total_in, total_out);
> +	unsigned int n_sectors;
> +	unsigned int n_sg_in = 0, n_sg_out = 0;
> +	struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
> +	struct scatterlist *sg_in = NULL, *sg_out = NULL;
> +	struct bvec_iter iter_in, iter_out;
> +	struct bio_vec bv;
> +	u8 *iv, *org_iv;
> +	int r;
> +
> +	if (unlikely(total < sector_size))
> +		return -EIO;
> +	n_sectors = total / sector_size;
> +	total = n_sectors * sector_size;

Division is slow. There should be this:
	n_sectors = total >> cc->sector_shift;


> +     if (unlikely(total < sector_size))
> +             return -EIO;

The condition total < sector_size is true if total is small but it goes 
through if total is bigger but unaligned. I think that it should be:

	if (unlikely(total & (sector_size - 1)))
		return -EIO;

(then, we can drop the line "total = n_sectors * sector_size")

ctx->iter_in.bi_size is supposed to be the same as ctx->iter_out.bi_size, 
so do we really need total = min(total_in, total_out)? Should it instead 
warn if they differ? (where the warning would indicate a bug in the code)

Mikulas