On Wed, 25 Jun 2025, Damien Le Moal wrote:
> Read and write operations issued to a dm-crypt target may be split
> according to the dm-crypt internal limits defined by the max_read_size
> and max_write_size module parameters (default is 128 KB). The intent is
> to improve processing time of large BIOs by splitting them into smaller
> operations that can be parallelized on different CPUs.
>
> For zoned dm-crypt targets, this BIO splitting is still done but without
> the parallel execution to ensure that the issuing order of write
> operations to the underlying devices remains sequential. However, the
> splitting itself causes other problems:
>
> 1) Since dm-crypt relies on the block layer zone write plugging to
> handle zone append emulation using regular write operations, the
> reminder of a split write BIO will always be plugged into the target
> zone write plugged. Once the on-going write BIO finishes, this
> reminder BIO is unplugged and issued from the zone write plug work.
> If this reminder BIO itself needs to be split, the reminder will be
> re-issued and plugged again, but that causes a call to a
> blk_queue_enter(), which may block if a queue freeze operation was
> initiated. This results in a deadlock as DM submission still holds
> BIOs that the queue freeze side is waiting for.
>
> 2) dm-crypt relies on the emulation done by the block layer using
> regular write operations for processing zone append operations. This
> still requires to properly return the written sector as the BIO
> sector of the original BIO. However, this can be done correctly only
> and only if there is a single clone BIO used for processing the
> original zone append operation issued by the user. If the size of a
> zone append operation is larger than dm-crypt max_write_size, then
> the orginal BIO will be split and processed as a chain of regular
> write operations. Such chaining result in an incorrect written sector
> being returned to the zone append issuer using the original BIO
> sector. This in turn results in file system data corruptions using
> xfs or btrfs.
>
> Fix this by modifying get_max_request_size() to always return the size
> of the BIO to avoid it being split with dm_accpet_partial_bio() in
> crypt_map(). get_max_request_size() is renamed to
> get_max_request_sectors() to clarify the unit of the value returned
> and its interface is changed to take a struct dm_target pointer and a
> pointer to the struct bio being processed. In addition to this change,
> to ensure that crypt_alloc_buffer() works correctly, set the dm-crypt
> device max_hw_sectors limit to be at most
> BIO_MAX_VECS << PAGE_SECTORS_SHIFT (1 MB with a 4KB page architecture).
> This forces DM core to split write BIOs before passing them to
> crypt_map(), and thus guaranteeing that dm-crypt can always accept an
> entire write BIO without needing to split it.
>
> This change does not have any effect on the read path of dm-crypt. Read
> operations can still be split and the BIO fragments processed in
> parallel. There is also no impact on the performance of the write path
> given that all zone write BIOs were already processed inline instead of
> in parallel.
>
> This change also does not affect in any way regular dm-crypt block
> devices.
>
> Fixes: f211268ed1f9 ("dm: Use the block layer zone append emulation")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlem...@kernel.org>
> ---
> drivers/md/dm-crypt.c | 49 ++++++++++++++++++++++++++++++++++---------
> 1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 17157c4216a5..4e80784d1734 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -253,17 +253,35 @@ MODULE_PARM_DESC(max_read_size, "Maximum size of a read
> request");
> static unsigned int max_write_size = 0;
> module_param(max_write_size, uint, 0644);
> MODULE_PARM_DESC(max_write_size, "Maximum size of a write request");
> -static unsigned get_max_request_size(struct crypt_config *cc, bool wrt)
> +
> +static unsigned get_max_request_sectors(struct dm_target *ti, struct bio
> *bio)
> {
> + struct crypt_config *cc = ti->private;
> unsigned val, sector_align;
> - val = !wrt ? READ_ONCE(max_read_size) : READ_ONCE(max_write_size);
> - if (likely(!val))
> - val = !wrt ? DM_CRYPT_DEFAULT_MAX_READ_SIZE :
> DM_CRYPT_DEFAULT_MAX_WRITE_SIZE;
> - if (wrt || cc->used_tag_size) {
> - 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)cc->sector_size);
> + bool wrt = op_is_write(bio_op(bio));
> +
> + if (wrt) {
> + /*
> + * For zoned devices, splitting write operations creates the
> + * risk of deadlocking queue freeze operations with zone write
> + * plugging BIO work when the reminder of a split BIO is
> + * issued. So always allow the entire BIO to proceed.
> + */
> + if (ti->emulate_zone_append)
> + return bio_sectors(bio);
The overrun may still happen (if the user changes the dm table while some
bio is in progress) and if it happens, you should terminate the bio with
DM_MAPIO_KILL (like it was in my original patch).
Mikulas
