Hi

The patch seems OK. Should it go in via the device mapper tree or the 
block layer tree?


On Mon, 16 Sep 2024, Md Sadre Alam wrote:

> +#define DM_CRYPT_DEFAULT_MAX_READ_SIZE               131072
> +#define DM_CRYPT_DEFAULT_MAX_WRITE_SIZE              131072
> +
> +static unsigned int get_max_request_size(struct inlinecrypt_config *cc, bool 
> wrt)
> +{
> +     unsigned int val, sector_align;
> +
> +     val = !wrt ? DM_CRYPT_DEFAULT_MAX_READ_SIZE : 
> DM_CRYPT_DEFAULT_MAX_WRITE_SIZE;
> +     if (wrt) {
> +             if (unlikely(val > BIO_MAX_VECS << PAGE_SHIFT))
> +                     val = BIO_MAX_VECS << PAGE_SHIFT;
> +     }
> +     sector_align = max(bdev_logical_block_size(cc->dev->bdev), (unsigned 
> int)cc->sector_size);
> +     val = round_down(val, sector_align);
> +     if (unlikely(!val))
> +             val = sector_align;
> +     return val >> SECTOR_SHIFT;
> +}

This piece of code was copied from the dm-crypt target. For dm-crypt, I 
was actually benchmarking the performance for various 
DM_CRYPT_DEFAULT_MAX_READ_SIZE and DM_CRYPT_DEFAULT_MAX_WRITE_SIZE values 
and I selected the values that resulted in the best performance.

You should benchmark it too to find the optimal I/O size. Perhaps you find 
out that there is no need to split big requests and this piece of code can 
be dropped.

> +               /* Omit the key for now. */
> +               DMEMIT("%s - %llu %s %llu", ctx->cipher_string, 
> ctx->iv_offset,
> +                      ctx->dev->name, (unsigned long long)ctx->start);

What if someone reloads the table? I think you should display the key. 
dmsetup does not display the key if the "--showkeys" parameter is not 
specified.

Mikulas


Reply via email to