* [PATCHv5 0/3] dm-crypt: finer grained memory alignment
@ 2026-07-09 16:15 Keith Busch
2026-07-09 16:15 ` [PATCHv5 1/3] dm: initialize dma_alignment to 0 Keith Busch
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Keith Busch @ 2026-07-09 16:15 UTC (permalink / raw)
To: dm-devel, mpatocka; +Cc: snitzer, bmarzins, Keith Busch
From: Keith Busch <kbusch@kernel.org>
This is the updated series for allowing arbitrary memory alignment,
enabling direct-io without bounce buffers for dm-crypt.
The previous version was posted here:
https://lore.kernel.org/dm-devel/20260330170114.764606-1-kbusch@meta.com/
Changes from v4:
* Updated the mempool count to MIN_IOS, as requested.
* Fixed the issue observed from cryptsetup testsuite. The problem was
an early return path accessing unitialized fields. The fix just
initializes them before.
* I discovered we may be in softirq context when allocating, so
had to add handling for ENOMEM. But then also noticed existing
ENOMEM cases were just being generically handled in the default case
returning EIO, so I introduced a prep patch to make these transient
resource issues retryable.
Keith Busch (3):
dm: initialize dma_alignment to 0
dm-crypt: handle memory allocation failures
dm-crypt: allow unaligned bio_vecs for direct io
drivers/md/dm-crypt.c | 183 ++++++++++++++++++++++++++++++++++++------
drivers/md/dm-table.c | 1 +
2 files changed, 159 insertions(+), 25 deletions(-)
--
2.52.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv5 1/3] dm: initialize dma_alignment to 0
2026-07-09 16:15 [PATCHv5 0/3] dm-crypt: finer grained memory alignment Keith Busch
@ 2026-07-09 16:15 ` Keith Busch
2026-10-22 17:01 ` [PATCH] block: let a stacking device inherit its members' DMA alignment Mike Snitzer
2026-07-09 16:15 ` [PATCHv5 2/3] dm-crypt: handle memory allocation failures Keith Busch
2026-07-09 16:15 ` [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io Keith Busch
2 siblings, 1 reply; 12+ messages in thread
From: Keith Busch @ 2026-07-09 16:15 UTC (permalink / raw)
To: dm-devel, mpatocka; +Cc: snitzer, bmarzins, Keith Busch
From: Keith Busch <kbusch@kernel.org>
All device mappers that want to override the dma_alignment do so.
However, because the default stacking limit is set to 511, a device
mapper couldn't make it smaller even if it is capable of handling
finer grained alignment.
Set the default dm stacking dma_alignment limit to 0 so that it can be
stacked with any value a device mapper wants. If a mapper doesn't
override the value, the initial 0 value will get changed to the original
default 511 value when the limits are validated after the device
stacking is complete.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/md/dm-table.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index dc2eff6b739df..80326f8096cb7 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -591,6 +591,7 @@ static void dm_set_stacking_limits(struct queue_limits *limits)
{
blk_set_stacking_limits(limits);
limits->features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT | BLK_FEAT_POLL;
+ limits->dma_alignment = 0;
}
/*
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv5 2/3] dm-crypt: handle memory allocation failures
2026-07-09 16:15 [PATCHv5 0/3] dm-crypt: finer grained memory alignment Keith Busch
2026-07-09 16:15 ` [PATCHv5 1/3] dm: initialize dma_alignment to 0 Keith Busch
@ 2026-07-09 16:15 ` Keith Busch
2026-07-09 16:15 ` [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io Keith Busch
2 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2026-07-09 16:15 UTC (permalink / raw)
To: dm-devel, mpatocka; +Cc: snitzer, bmarzins, Keith Busch
From: Keith Busch <kbusch@kernel.org>
There are a few paths that can fail with -ENOMEM, like skcipher_copy_iv,
which had been treated as a generic failure. This is a transient host
resource issue that can be retried later.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/md/dm-crypt.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 608b617fb817f..fc9c875cccfd2 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1338,6 +1338,8 @@ static int crypt_convert_block_aead(struct crypt_config *cc,
dm_audit_log_bio(DM_MSG_PREFIX, "integrity-aead",
ctx->bio_in, s, 0);
}
+ } else if (unlikely(r == -ENOMEM)) {
+ return r;
}
if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post)
@@ -1417,6 +1419,9 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,
else
r = crypto_skcipher_decrypt(req);
+ if (unlikely(r == -ENOMEM))
+ return r;
+
if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post)
cc->iv_gen_ops->post(cc, org_iv, dmreq);
@@ -1593,6 +1598,13 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
case -EBADMSG:
atomic_dec(&ctx->cc_pending);
return BLK_STS_PROTECTION;
+ /*
+ * There was a host resource issue.
+ */
+ case -ENOMEM:
+ atomic_dec(&ctx->cc_pending);
+ complete(&ctx->restart);
+ return BLK_STS_RESOURCE;
/*
* There was an error while processing the request.
*/
@@ -2072,10 +2084,11 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io)
test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags), true);
/*
* Crypto API backlogged the request, because its queue was full
- * and we're in softirq context, so continue from a workqueue
- * (TODO: is it actually possible to be in softirq in the write path?)
+ * and we're in softirq context, or a host memory allocation
+ * temporarily failed, so continue from a workqueue (TODO: is it
+ * actually possible to be in softirq in the write path?)
*/
- if (r == BLK_STS_DEV_RESOURCE) {
+ if (r == BLK_STS_DEV_RESOURCE || r == BLK_STS_RESOURCE) {
INIT_WORK(&io->work, kcryptd_crypt_write_continue);
queue_work(cc->crypt_queue, &io->work);
return;
@@ -2148,9 +2161,10 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io)
}
/*
* Crypto API backlogged the request, because its queue was full
- * and we're in softirq context, so continue from a workqueue
+ * and we're in softirq context, or a host memory allocation
+ * temporarily failed, so continue from a workqueue
*/
- if (r == BLK_STS_DEV_RESOURCE) {
+ if (r == BLK_STS_DEV_RESOURCE || r == BLK_STS_RESOURCE) {
INIT_WORK(&io->work, kcryptd_crypt_read_continue);
queue_work(cc->crypt_queue, &io->work);
return;
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io
2026-07-09 16:15 [PATCHv5 0/3] dm-crypt: finer grained memory alignment Keith Busch
2026-07-09 16:15 ` [PATCHv5 1/3] dm: initialize dma_alignment to 0 Keith Busch
2026-07-09 16:15 ` [PATCHv5 2/3] dm-crypt: handle memory allocation failures Keith Busch
@ 2026-07-09 16:15 ` Keith Busch
2026-07-11 0:54 ` Eric Biggers
2 siblings, 1 reply; 12+ messages in thread
From: Keith Busch @ 2026-07-09 16:15 UTC (permalink / raw)
To: dm-devel, mpatocka; +Cc: snitzer, bmarzins, Keith Busch
From: Keith Busch <kbusch@kernel.org>
Many storage devices can handle DMA for data that is not aligned to the
logical block size. The block and filesystem layers have introduced
updates to allow that kind of memory alignment flexibility when
possible.
dm-crypt, however, currently constrains itself to aligned memory because
it sends a single scatterlist element for the in/out list to the encrypt
and decrypt algorithms. This forces applications that have unaligned
data to copy through a bounce buffer, increasing CPU and memory
utilization.
Use multiple scatterlist elements as needed to relax the memory
alignment requirement. To keep this simple, this more flexible
constraint is enabled only for certain encryption and initialization
vector types, specifically the ones that don't have additional use for
the request scatterlist elements beyond pointing to user data.
In the unlikely case where the incoming bio uses a highly fragmented
vector, the four inline scatterlist elements may not be enough, so
allocate a temporary scatterlist when needed, falling back to a mempool
for the in and out buffers to guarantee forward progress if the initial
allocation fails.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
drivers/md/dm-crypt.c | 161 ++++++++++++++++++++++++++++++++++++------
1 file changed, 140 insertions(+), 21 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index fc9c875cccfd2..66f666d9812f0 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -101,6 +101,10 @@ struct dm_crypt_request {
struct scatterlist sg_in[4];
struct scatterlist sg_out[4];
u64 iv_sector;
+ struct scatterlist *__sg_in;
+ struct scatterlist *__sg_out;
+ bool sg_in_pooled;
+ bool sg_out_pooled;
};
struct crypt_config;
@@ -216,6 +220,9 @@ struct crypt_config {
unsigned int key_extra_size; /* additional keys length */
unsigned int key_mac_size; /* MAC key size for authenc(...) */
+ unsigned int io_alignment;
+ mempool_t sg_in_pool;
+ mempool_t sg_out_pool;
unsigned int integrity_tag_size;
unsigned int integrity_iv_size;
unsigned int used_tag_size;
@@ -1351,22 +1358,92 @@ static int crypt_convert_block_aead(struct crypt_config *cc,
return r;
}
+static void crypt_free_sg(struct scatterlist *sg, struct scatterlist *inline_sg,
+ mempool_t *pool, bool from_pool)
+{
+ if (sg == inline_sg)
+ return;
+ if (from_pool)
+ mempool_free(sg, pool);
+ else
+ kfree(sg);
+}
+
+static void crypt_free_sgls(struct crypt_config *cc,
+ struct dm_crypt_request *dmreq)
+{
+ crypt_free_sg(dmreq->__sg_in, dmreq->sg_in,
+ &cc->sg_in_pool, dmreq->sg_in_pooled);
+ crypt_free_sg(dmreq->__sg_out, dmreq->sg_out,
+ &cc->sg_out_pool, dmreq->sg_out_pooled);
+ dmreq->__sg_in = NULL;
+ dmreq->__sg_out = NULL;
+}
+
+static int crypt_build_sgl(struct crypt_config *cc, struct scatterlist **psg,
+ struct bvec_iter iter, struct bio *bio,
+ int max_segs, mempool_t *pool, bool *pooled)
+{
+ unsigned int bytes = cc->sector_size;
+ struct scatterlist *sg = *psg;
+ struct bvec_iter tmp = iter;
+ int segs, i = 0;
+
+ *pooled = false;
+ bio_advance_iter(bio, &tmp, bytes);
+ segs = tmp.bi_idx - iter.bi_idx + !!tmp.bi_bvec_done;
+ if (segs > max_segs) {
+ if (unlikely(segs > BIO_MAX_VECS))
+ return -EIO;
+ sg = kmalloc_array(segs, sizeof(struct scatterlist),
+ GFP_NOWAIT | __GFP_NOMEMALLOC);
+ if (!sg) {
+ sg = mempool_alloc(pool, in_interrupt() ? GFP_ATOMIC :
+ GFP_NOIO);
+ if (!sg)
+ return -ENOMEM;
+ *pooled = true;
+ }
+ }
+
+ sg_init_table(sg, segs);
+ do {
+ struct bio_vec bv = mp_bvec_iter_bvec(bio->bi_io_vec, iter);
+ int len = min(bytes, bv.bv_len);
+
+ /* Reject unexpected unaligned bio. */
+ if (unlikely((len | bv.bv_offset) & cc->io_alignment))
+ goto error;
+
+ sg_set_page(&sg[i++], bv.bv_page, len, bv.bv_offset);
+ bio_advance_iter_single(bio, &iter, len);
+ bytes -= len;
+ } while (bytes);
+
+ if (WARN_ON_ONCE(i != segs))
+ goto error;
+ *psg = sg;
+ return 0;
+error:
+ if (sg != *psg) {
+ if (*pooled)
+ mempool_free(sg, pool);
+ else
+ kfree(sg);
+ }
+ return -EIO;
+}
+
static int crypt_convert_block_skcipher(struct crypt_config *cc,
struct convert_context *ctx,
struct skcipher_request *req,
unsigned int tag_offset)
{
- struct bio_vec bv_in = bio_iter_iovec(ctx->bio_in, ctx->iter_in);
- struct bio_vec bv_out = bio_iter_iovec(ctx->bio_out, ctx->iter_out);
struct scatterlist *sg_in, *sg_out;
struct dm_crypt_request *dmreq;
u8 *iv, *org_iv, *tag_iv;
__le64 *sector;
- int r = 0;
-
- /* Reject unexpected unaligned bio. */
- if (unlikely(bv_in.bv_len & (cc->sector_size - 1)))
- return -EIO;
+ int r;
dmreq = dmreq_of_req(cc, req);
dmreq->iv_sector = ctx->cc_sector;
@@ -1383,15 +1460,23 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,
sector = org_sector_of_dmreq(cc, dmreq);
*sector = cpu_to_le64(ctx->cc_sector - cc->iv_offset);
- /* For skcipher we use only the first sg item */
- sg_in = &dmreq->sg_in[0];
- sg_out = &dmreq->sg_out[0];
+ dmreq->__sg_in = &dmreq->sg_in[0];
+ dmreq->__sg_out = &dmreq->sg_out[0];
+
+ r = crypt_build_sgl(cc, &dmreq->__sg_in, ctx->iter_in, ctx->bio_in,
+ ARRAY_SIZE(dmreq->sg_in), &cc->sg_in_pool,
+ &dmreq->sg_in_pooled);
+ if (r < 0)
+ goto out;
- sg_init_table(sg_in, 1);
- sg_set_page(sg_in, bv_in.bv_page, cc->sector_size, bv_in.bv_offset);
+ r = crypt_build_sgl(cc, &dmreq->__sg_out, ctx->iter_out, ctx->bio_out,
+ ARRAY_SIZE(dmreq->sg_out), &cc->sg_out_pool,
+ &dmreq->sg_out_pooled);
+ if (r < 0)
+ goto out;
- sg_init_table(sg_out, 1);
- sg_set_page(sg_out, bv_out.bv_page, cc->sector_size, bv_out.bv_offset);
+ sg_in = dmreq->__sg_in;
+ sg_out = dmreq->__sg_out;
if (cc->iv_gen_ops) {
/* For READs use IV stored in integrity metadata */
@@ -1400,7 +1485,7 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,
} else {
r = cc->iv_gen_ops->generator(cc, org_iv, dmreq);
if (r < 0)
- return r;
+ goto out;
/* Data can be already preprocessed in generator */
if (test_bit(CRYPT_ENCRYPT_PREPROCESS, &cc->cipher_flags))
sg_in = sg_out;
@@ -1420,13 +1505,16 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,
r = crypto_skcipher_decrypt(req);
if (unlikely(r == -ENOMEM))
- return r;
+ goto out;
if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post)
cc->iv_gen_ops->post(cc, org_iv, dmreq);
bio_advance_iter(ctx->bio_in, &ctx->iter_in, cc->sector_size);
bio_advance_iter(ctx->bio_out, &ctx->iter_out, cc->sector_size);
+out:
+ if (r != -EINPROGRESS && r != -EBUSY)
+ crypt_free_sgls(cc, dmreq);
return r;
}
@@ -1492,7 +1580,9 @@ static void crypt_free_req_skcipher(struct crypt_config *cc,
struct skcipher_request *req, struct bio *base_bio)
{
struct dm_crypt_io *io = dm_per_bio_data(base_bio, cc->per_bio_data_size);
+ struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
+ crypt_free_sgls(cc, dmreq);
if ((struct skcipher_request *)(io + 1) != req)
mempool_free(req, &cc->req_pool);
}
@@ -2731,6 +2821,8 @@ static void crypt_dtr(struct dm_target *ti)
mempool_exit(&cc->page_pool);
mempool_exit(&cc->req_pool);
+ mempool_exit(&cc->sg_in_pool);
+ mempool_exit(&cc->sg_out_pool);
mempool_exit(&cc->tag_pool);
WARN_ON(percpu_counter_sum(&cc->n_allocated_pages) != 0);
@@ -2765,10 +2857,12 @@ static int crypt_ctr_ivmode(struct dm_target *ti, const char *ivmode)
{
struct crypt_config *cc = ti->private;
- if (crypt_integrity_aead(cc))
+ if (crypt_integrity_aead(cc)) {
cc->iv_size = crypto_aead_ivsize(any_tfm_aead(cc));
- else
+ cc->io_alignment = cc->sector_size - 1;
+ } else {
cc->iv_size = crypto_skcipher_ivsize(any_tfm(cc));
+ }
if (cc->iv_size)
/* at least a 64 bit sector number should fit in our buffer */
@@ -2803,6 +2897,7 @@ static int crypt_ctr_ivmode(struct dm_target *ti, const char *ivmode)
if (cc->key_extra_size > ELEPHANT_MAX_KEY_SIZE)
return -EINVAL;
set_bit(CRYPT_ENCRYPT_PREPROCESS, &cc->cipher_flags);
+ cc->io_alignment = cc->sector_size - 1;
} else if (strcmp(ivmode, "lmk") == 0) {
cc->iv_gen_ops = &crypt_iv_lmk_ops;
/*
@@ -2815,10 +2910,12 @@ static int crypt_ctr_ivmode(struct dm_target *ti, const char *ivmode)
cc->key_parts++;
cc->key_extra_size = cc->key_size / cc->key_parts;
}
+ cc->io_alignment = cc->sector_size - 1;
} else if (strcmp(ivmode, "tcw") == 0) {
cc->iv_gen_ops = &crypt_iv_tcw_ops;
cc->key_parts += 2; /* IV + whitening */
cc->key_extra_size = cc->iv_size + TCW_WHITENING_SIZE;
+ cc->io_alignment = cc->sector_size - 1;
} else if (strcmp(ivmode, "random") == 0) {
cc->iv_gen_ops = &crypt_iv_random_ops;
/* Need storage space in integrity fields. */
@@ -3285,6 +3382,20 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
ALIGN(sizeof(struct dm_crypt_io) + cc->dmreq_start + additional_req_size,
ARCH_DMA_MINALIGN);
+ ret = mempool_init_kmalloc_pool(&cc->sg_in_pool, MIN_IOS,
+ BIO_MAX_VECS * sizeof(struct scatterlist));
+ if (ret) {
+ ti->error = "Cannot allocate crypt scatterlist mempool";
+ goto bad;
+ }
+
+ ret = mempool_init_kmalloc_pool(&cc->sg_out_pool, MIN_IOS,
+ BIO_MAX_VECS * sizeof(struct scatterlist));
+ if (ret) {
+ ti->error = "Cannot allocate crypt scatterlist mempool";
+ goto bad;
+ }
+
ret = mempool_init(&cc->page_pool, BIO_MAX_VECS, crypt_page_alloc, crypt_page_free, cc);
if (ret) {
ti->error = "Cannot allocate page mempool";
@@ -3495,10 +3606,16 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
}
}
- if (crypt_integrity_aead(cc))
+ if (crypt_integrity_aead(cc)) {
io->ctx.r.req_aead = (struct aead_request *)(io + 1);
- else
+ } else {
+ struct dm_crypt_request *dmreq;
+
io->ctx.r.req = (struct skcipher_request *)(io + 1);
+ dmreq = dmreq_of_req(cc, io->ctx.r.req);
+ dmreq->__sg_in = &dmreq->sg_in[0];
+ dmreq->__sg_out = &dmreq->sg_out[0];
+ }
if (bio_data_dir(io->base_bio) == READ) {
if (kcryptd_io_read(io, CRYPT_MAP_READ_GFP))
@@ -3694,7 +3811,9 @@ static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
struct crypt_config *cc = ti->private;
dm_stack_bs_limits(limits, cc->sector_size);
- limits->dma_alignment = limits->logical_block_size - 1;
+ limits->dma_alignment = max(bdev_dma_alignment(cc->dev->bdev),
+ cc->io_alignment);
+ cc->io_alignment = limits->dma_alignment;
/*
* For zoned dm-crypt targets, there will be no internal splitting of
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io
2026-07-09 16:15 ` [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io Keith Busch
@ 2026-07-11 0:54 ` Eric Biggers
2026-07-11 4:14 ` Keith Busch
0 siblings, 1 reply; 12+ messages in thread
From: Eric Biggers @ 2026-07-11 0:54 UTC (permalink / raw)
To: Keith Busch; +Cc: dm-devel, mpatocka, snitzer, bmarzins, Keith Busch
On Thu, Jul 09, 2026 at 09:15:19AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> Many storage devices can handle DMA for data that is not aligned to the
> logical block size. The block and filesystem layers have introduced
> updates to allow that kind of memory alignment flexibility when
> possible.
>
> dm-crypt, however, currently constrains itself to aligned memory because
> it sends a single scatterlist element for the in/out list to the encrypt
> and decrypt algorithms. This forces applications that have unaligned
> data to copy through a bounce buffer, increasing CPU and memory
> utilization.
>
> Use multiple scatterlist elements as needed to relax the memory
> alignment requirement. To keep this simple, this more flexible
> constraint is enabled only for certain encryption and initialization
> vector types, specifically the ones that don't have additional use for
> the request scatterlist elements beyond pointing to user data.
>
> In the unlikely case where the incoming bio uses a highly fragmented
> vector, the four inline scatterlist elements may not be enough, so
> allocate a temporary scatterlist when needed, falling back to a mempool
> for the in and out buffers to guarantee forward progress if the initial
> allocation fails.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
So what is the minimum SG entry length alignment this can result in?
Once that get misaligned enough, crypto_skcipher_encrypt() and
crypto_skcipher_decrypt() start allocating bounce buffers internally.
That code has no mempool fallback and can fail with ENOMEM.
I think you'll need to take the limitations of crypto_skcipher into
account. See the comment above CRYPTO_ALG_ALLOCATES_MEMORY.
Note: I'm working on fixing at least AES-XTS to not allocate memory
(https://lore.kernel.org/linux-crypto/20260707053503.209874-12-ebiggers@kernel.org/).
But segment lengths not a multiple of 16 bytes will always be
inefficient to process and really should be avoided.
- Eric
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io
2026-07-11 0:54 ` Eric Biggers
@ 2026-07-11 4:14 ` Keith Busch
2026-07-11 4:30 ` Eric Biggers
0 siblings, 1 reply; 12+ messages in thread
From: Keith Busch @ 2026-07-11 4:14 UTC (permalink / raw)
To: Eric Biggers; +Cc: Keith Busch, dm-devel, mpatocka, snitzer, bmarzins
On Fri, Jul 10, 2026 at 08:54:40PM -0400, Eric Biggers wrote:
>
> So what is the minimum SG entry length alignment this can result in?
> Once that get misaligned enough, crypto_skcipher_encrypt() and
> crypto_skcipher_decrypt() start allocating bounce buffers internally.
> That code has no mempool fallback and can fail with ENOMEM.
I'm not the one introducing ENOMEM consideration here. Indeed, patch 2
in this series addresses this very thing ahead of what I'm asking for.
> I think you'll need to take the limitations of crypto_skcipher into
> account. See the comment above CRYPTO_ALG_ALLOCATES_MEMORY.
The block layer's queue limits is the single source of truth for what
the backing device at any layer can accept. If you've some constraints
that are not appropriately described there, then let's fix that.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io
2026-07-11 4:14 ` Keith Busch
@ 2026-07-11 4:30 ` Eric Biggers
2026-07-13 18:54 ` Keith Busch
0 siblings, 1 reply; 12+ messages in thread
From: Eric Biggers @ 2026-07-11 4:30 UTC (permalink / raw)
To: Keith Busch; +Cc: Keith Busch, dm-devel, mpatocka, snitzer, bmarzins
On Fri, Jul 10, 2026 at 10:14:19PM -0600, Keith Busch wrote:
> On Fri, Jul 10, 2026 at 08:54:40PM -0400, Eric Biggers wrote:
> >
> > So what is the minimum SG entry length alignment this can result in?
> > Once that get misaligned enough, crypto_skcipher_encrypt() and
> > crypto_skcipher_decrypt() start allocating bounce buffers internally.
> > That code has no mempool fallback and can fail with ENOMEM.
>
> I'm not the one introducing ENOMEM consideration here.
You are. This patch starts allowing misaligned segments, which trigger
the use of bounce buffers in the crypto code as I explained. Currently,
dm-crypt relies on its alignment requirement to avoid those cases.
> Indeed, patch 2 in this series addresses this very thing ahead of what
> I'm asking for.
I don't think patch 2 is sufficient, as BLK_STS_RESOURCE is often not
going to get handled correctly.
> > I think you'll need to take the limitations of crypto_skcipher into
> > account. See the comment above CRYPTO_ALG_ALLOCATES_MEMORY.
>
> The block layer's queue limits is the single source of truth for what
> the backing device at any layer can accept. If you've some constraints
> that are not appropriately described there, then let's fix that.
I agree that dm-crypt's queue limits should accurately describe its
constraints. This patch makes it no longer do so.
- Eric
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io
2026-07-11 4:30 ` Eric Biggers
@ 2026-07-13 18:54 ` Keith Busch
0 siblings, 0 replies; 12+ messages in thread
From: Keith Busch @ 2026-07-13 18:54 UTC (permalink / raw)
To: Eric Biggers; +Cc: Keith Busch, dm-devel, mpatocka, snitzer, bmarzins
On Sat, Jul 11, 2026 at 12:30:18AM -0400, Eric Biggers wrote:
> On Fri, Jul 10, 2026 at 10:14:19PM -0600, Keith Busch wrote:
>
> I don't think patch 2 is sufficient, as BLK_STS_RESOURCE is often not
> going to get handled correctly.
In practice, I could only get the memory allocation failure to happen in
interrupt context due to the GFP_ATOMIC use, and that appears to be
handled correctly. If I synthesize memory failure in the process
context, then that is getting sent back to the application, so I assume
that's what you mean about BLK_STS_RESOURCE not always handled
correctly. I'll get that fixed up.
> > > I think you'll need to take the limitations of crypto_skcipher into
> > > account. See the comment above CRYPTO_ALG_ALLOCATES_MEMORY.
> >
> > The block layer's queue limits is the single source of truth for what
> > the backing device at any layer can accept. If you've some constraints
> > that are not appropriately described there, then let's fix that.
>
> I agree that dm-crypt's queue limits should accurately describe its
> constraints. This patch makes it no longer do so.
Which one(s)?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] block: let a stacking device inherit its members' DMA alignment
2026-07-09 16:15 ` [PATCHv5 1/3] dm: initialize dma_alignment to 0 Keith Busch
@ 2026-10-22 17:01 ` Mike Snitzer
2026-10-22 17:25 ` sashiko-bot
0 siblings, 1 reply; 12+ messages in thread
From: Mike Snitzer @ 2026-10-22 17:01 UTC (permalink / raw)
To: Keith Busch, axboe; +Cc: dm-devel, mpatocka, bmarzins, Keith Busch, linux-block
On Thu, Jul 09, 2026 at 09:15:17AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> All device mappers that want to override the dma_alignment do so.
> However, because the default stacking limit is set to 511, a device
> mapper couldn't make it smaller even if it is capable of handling
> finer grained alignment.
>
> Set the default dm stacking dma_alignment limit to 0 so that it can be
> stacked with any value a device mapper wants. If a mapper doesn't
> override the value, the initial 0 value will get changed to the original
> default 511 value when the limits are validated after the device
> stacking is complete.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
> drivers/md/dm-table.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index dc2eff6b739df..80326f8096cb7 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -591,6 +591,7 @@ static void dm_set_stacking_limits(struct queue_limits *limits)
> {
> blk_set_stacking_limits(limits);
> limits->features |= BLK_FEAT_IO_STAT | BLK_FEAT_NOWAIT | BLK_FEAT_POLL;
> + limits->dma_alignment = 0;
> }
>
> /*
> --
> 2.52.0
>
This isn't a DM-specific concern (just happens to be that you're
focused on dm-crypt in this patchset). dm-crypt is still imposing a
too strict dma_alignment, and so that'll need fixing too (along with
other DM targets that aren't properly considering the underlying
devices' queue_limits provided dma_alignment).
But this is a proper fix that I needed to allow MD RAID0 to allow
O_DIRECT to NVMe devices beneath the MD device:
From: Mike Snitzer <snitzer@kernel.org>
Date: Fri, 9 Oct 2026 06:26:29 -0600
Subject: [PATCH] block: let a stacking device inherit its members' DMA alignment
blk_set_stacking_limits() initializes dma_alignment to SECTOR_SIZE - 1,
and blk_stack_limits() then takes the maximum of that and each member's
value. As a result every MD array and device-mapper table requires
512-byte aligned memory regardless of what its members need: a raid0
of NVMe namespaces reports a dma_alignment of 511 while each member
reports 3.
Direct I/O from memory every member accepts is therefore rejected at
the stacking device, and callers that check the advertised alignment
(for instance through STATX_DIOALIGN) fall back to buffered I/O.
Leave dma_alignment at zero in blk_set_stacking_limits() so that the
stacked value is the strictest of the members'. blk_validate_limits()
still applies the SECTOR_SIZE - 1 default when nothing was stacked,
such as for a table with no devices. Targets that process the data
themselves and need logical-block-aligned memory already set
dma_alignment in their io_hints (dm-crypt, dm-integrity, dm-verity,
dm-log-writes), and NVMe multipath sets its own.
Assisted-by: LLM
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
block/blk-settings.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 8274631290db..c4b0fba01c4b 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -41,7 +41,14 @@ void blk_set_stacking_limits(struct queue_limits *lim)
lim->physical_block_size = SECTOR_SIZE;
lim->io_min = SECTOR_SIZE;
lim->discard_granularity = SECTOR_SIZE;
- lim->dma_alignment = SECTOR_SIZE - 1;
+ /*
+ * No dma_alignment of its own: blk_stack_limits() keeps the strictest
+ * of the devices stacked under it, and blk_validate_limits() gives the
+ * sector-size default to a device left with none. A driver that looks
+ * at the data and needs it laid out by logical block says so in its
+ * own limits, as dm-crypt, dm-integrity, dm-verity and dm-log-writes
+ * do.
+ */
lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
/* Inherit limits from component devices */
--
2.52.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] block: let a stacking device inherit its members' DMA alignment
2026-10-22 17:01 ` [PATCH] block: let a stacking device inherit its members' DMA alignment Mike Snitzer
@ 2026-10-22 17:25 ` sashiko-bot
2026-10-23 15:41 ` Mike Snitzer
0 siblings, 1 reply; 12+ messages in thread
From: sashiko-bot @ 2026-10-22 17:25 UTC (permalink / raw)
To: Mike Snitzer, Keith Busch, axboe
Cc: dm-devel, mpatocka, bmarzins, Keith Busch
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] block: blk_set_stacking_limits() corrupts data in dm-ebs and dm-writecache
--
commit da5c2e4cde4a12a4f8b254dc27480cf9078b36d2
Author: Mike Snitzer <snitzer@kernel.org>
block: let a stacking device inherit its members' DMA alignment
This commit removes the default sector-size dma_alignment initialization in
blk_set_stacking_limits() so that stacking devices inherit the strictest
dma_alignment of their underlying member devices. When no member devices are
stacked, blk_validate_limits() still applies the default sector alignment.
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index e469baa1f08b7..36fcbecdf793b 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -41,7 +41,14 @@ void blk_set_stacking_limits(struct queue_limits *lim)
> lim->physical_block_size = SECTOR_SIZE;
> lim->io_min = SECTOR_SIZE;
> lim->discard_granularity = SECTOR_SIZE;
> - lim->dma_alignment = SECTOR_SIZE - 1;
> + /*
> + * No dma_alignment of its own: blk_stack_limits() keeps the strictest
> + * of the devices stacked under it, and blk_validate_limits() gives the
> + * sector-size default to a device left with none. A driver that looks
> + * at the data and needs it laid out by logical block says so in its
> + * own limits, as dm-crypt, dm-integrity, dm-verity and dm-log-writes
> + * do.
> + */
[Severity: Critical]
Could leaving lim->dma_alignment at 0 in blk_set_stacking_limits() cause data
corruption regressions in targets like dm-ebs and dm-writecache that inspect
bio data but do not set limits->dma_alignment in their io_hints callbacks?
When stacked over a device with sub-sector dma_alignment (such as an nvme
device with dma_alignment = 3), direct I/O using a 4-byte-aligned user buffer
that crosses a page boundary or uses sub-sector iovecs can produce bio_vec
segments whose lengths are not multiples of 512 bytes (for example,
bv_len = 4092 followed by bv_len = 4).
In drivers/md/dm-ebs-target.c, ebs_io_hints() does not set
limits->dma_alignment:
drivers/md/dm-ebs-target.c:ebs_io_hints() {
struct ebs_c *ec = ti->private;
limits->logical_block_size = to_bytes(ec->e_bs);
limits->physical_block_size = to_bytes(ec->u_bs);
limits->alignment_offset = limits->physical_block_size;
limits->io_min = limits->logical_block_size;
}
When __ebs_rw_bio() iterates over the bio with bio_for_each_bvec() and calls
__ebs_rw_bvec(), __ebs_rw_bvec() computes the buffer offset and block number
from iter->bi_sector:
drivers/md/dm-ebs-target.c:__ebs_rw_bvec() {
unsigned int bv_len = bv->bv_len;
unsigned int buf_off = to_bytes(__block_mod(iter->bi_sector, ec->u_bs));
sector_t block = __sector_to_block(ec, iter->bi_sector);
Because bio_for_each_bvec() advances iter via bio_advance_iter_single() in
include/linux/bio.h:
include/linux/bio.h:bio_advance_iter_single() {
iter->bi_sector += bytes >> 9;
a segment with bv_len = 4092 advances iter.bi_sector by 4092 >> 9 = 7 sectors
(3584 bytes) instead of 4092 bytes, wouldn't the subsequent bio_vec read or
write at the wrong offset in the dm-bufio block?
Similarly, in drivers/md/dm-writecache.c, writecache_io_hints() only calls
dm_stack_bs_limits() and leaves limits->dma_alignment untouched:
drivers/md/dm-writecache.c:writecache_io_hints() {
struct dm_writecache *wc = ti->private;
dm_stack_bs_limits(limits, wc->block_size);
}
In persistent memory mode, writecache_map_read() and writecache_map_write()
process multi-block bios in a loop using bio->bi_iter.bi_sector to look up
and record cache entry sectors via write_original_sector_seq_count(), calling
bio_copy_block() for each block:
drivers/md/dm-writecache.c:bio_copy_block() {
do {
struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
...
size = bv.bv_len;
if (unlikely(size > remaining_size))
size = remaining_size;
...
data = (char *)data + size;
remaining_size -= size;
bio_advance(bio, size);
} while (unlikely(remaining_size));
}
When a cache block spans multiple bio_vec segments with sub-sector lengths
(such as 4092 and 4 for a 4096-byte block), bio_advance() ->
bio_advance_iter() updates bio->bi_iter.bi_sector += size >> 9 on each step,
advancing bi_sector by 7 sectors instead of 8 and corrupting
bio->bi_iter.bi_sector for subsequent blocks in the bio.
In addition, when bio_copy_block() calls memcpy_flushcache_optimized() with a
non-64-byte-multiple size >= 768 (such as 4092), doesn't the loop return
early without copying the remaining size % 64 bytes?
drivers/md/dm-writecache.c:memcpy_flushcache_optimized() {
if (cpu_feature_enabled(X86_FEATURE_CLFLUSHOPT) &&
likely(boot_cpu_data.x86_clflush_size == 64) &&
likely(size >= 768)) {
do {
memcpy((void *)dest, (void *)source, 64);
clflushopt((void *)dest);
dest += 64;
source += 64;
size -= 64;
} while (size >= 64);
return;
}
}
Finally, in solid-state cache mode, writecache_iterate_devices() only stacks
limits from wc->dev and not wc->ssd_dev:
drivers/md/dm-writecache.c:writecache_iterate_devices() {
struct dm_writecache *wc = ti->private;
return fn(ti, wc->dev, 0, ti->len, data);
}
If wc->dev has dma_alignment = 3 while wc->ssd_dev has dma_alignment = 511,
wouldn't dm-writecache advertise dma_alignment = 3 and remap 4-byte-aligned
bios to wc->ssd_dev->bdev, causing bio_split_io_at() to reject them?
> lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
>
> /* Inherit limits from component devices */
--
Sashiko AI review · https://sashiko.dev/#/patchset/atpBVtPqGDWZ2AFo@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: block: let a stacking device inherit its members' DMA alignment
2026-10-22 17:25 ` sashiko-bot
@ 2026-10-23 15:41 ` Mike Snitzer
2026-10-23 20:47 ` Mikulas Patocka
0 siblings, 1 reply; 12+ messages in thread
From: Mike Snitzer @ 2026-10-23 15:41 UTC (permalink / raw)
To: sashiko-bot; +Cc: Keith Busch, axboe, dm-devel, mpatocka, bmarzins, Keith Busch
On Thu, Oct 22, 2026 at 05:25:02PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] block: blk_set_stacking_limits() corrupts data in dm-ebs and dm-writecache
> --
>
> commit da5c2e4cde4a12a4f8b254dc27480cf9078b36d2
> Author: Mike Snitzer <snitzer@kernel.org>
>
> block: let a stacking device inherit its members' DMA alignment
>
> This commit removes the default sector-size dma_alignment initialization in
> blk_set_stacking_limits() so that stacking devices inherit the strictest
> dma_alignment of their underlying member devices. When no member devices are
> stacked, blk_validate_limits() still applies the default sector alignment.
>
> > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > index e469baa1f08b7..36fcbecdf793b 100644
> > --- a/block/blk-settings.c
> > +++ b/block/blk-settings.c
> > @@ -41,7 +41,14 @@ void blk_set_stacking_limits(struct queue_limits *lim)
> > lim->physical_block_size = SECTOR_SIZE;
> > lim->io_min = SECTOR_SIZE;
> > lim->discard_granularity = SECTOR_SIZE;
> > - lim->dma_alignment = SECTOR_SIZE - 1;
> > + /*
> > + * No dma_alignment of its own: blk_stack_limits() keeps the strictest
> > + * of the devices stacked under it, and blk_validate_limits() gives the
> > + * sector-size default to a device left with none. A driver that looks
> > + * at the data and needs it laid out by logical block says so in its
> > + * own limits, as dm-crypt, dm-integrity, dm-verity and dm-log-writes
> > + * do.
> > + */
>
> [Severity: Critical]
> Could leaving lim->dma_alignment at 0 in blk_set_stacking_limits() cause data
> corruption regressions in targets like dm-ebs and dm-writecache that inspect
> bio data but do not set limits->dma_alignment in their io_hints callbacks?
>
> When stacked over a device with sub-sector dma_alignment (such as an nvme
> device with dma_alignment = 3), direct I/O using a 4-byte-aligned user buffer
> that crosses a page boundary or uses sub-sector iovecs can produce bio_vec
> segments whose lengths are not multiples of 512 bytes (for example,
> bv_len = 4092 followed by bv_len = 4).
The code comment in the patch calls out the fact that
blk_validate_limits() will backfill dma_alignment if none is set (by
usual limits stacking or explicitly by DM targets).
But sure, there is potential for dragons if stacked block devices
like DM implicitly relied on block-core's coarse-grained default
dma_alignment.
Mikulas, would you be open to auditing the DM targets relative to
dma_alignment stacking after you're done with reviewing Keith's
related dm-crypt patchset?
Sorry for the extra work... but its worthwhile for DM to be more
precise with how it stacks dma_alignment
Thanks,
Mike
> In drivers/md/dm-ebs-target.c, ebs_io_hints() does not set
> limits->dma_alignment:
>
> drivers/md/dm-ebs-target.c:ebs_io_hints() {
> struct ebs_c *ec = ti->private;
>
> limits->logical_block_size = to_bytes(ec->e_bs);
> limits->physical_block_size = to_bytes(ec->u_bs);
> limits->alignment_offset = limits->physical_block_size;
> limits->io_min = limits->logical_block_size;
> }
>
> When __ebs_rw_bio() iterates over the bio with bio_for_each_bvec() and calls
> __ebs_rw_bvec(), __ebs_rw_bvec() computes the buffer offset and block number
> from iter->bi_sector:
>
> drivers/md/dm-ebs-target.c:__ebs_rw_bvec() {
> unsigned int bv_len = bv->bv_len;
> unsigned int buf_off = to_bytes(__block_mod(iter->bi_sector, ec->u_bs));
> sector_t block = __sector_to_block(ec, iter->bi_sector);
>
> Because bio_for_each_bvec() advances iter via bio_advance_iter_single() in
> include/linux/bio.h:
>
> include/linux/bio.h:bio_advance_iter_single() {
> iter->bi_sector += bytes >> 9;
>
> a segment with bv_len = 4092 advances iter.bi_sector by 4092 >> 9 = 7 sectors
> (3584 bytes) instead of 4092 bytes, wouldn't the subsequent bio_vec read or
> write at the wrong offset in the dm-bufio block?
>
> Similarly, in drivers/md/dm-writecache.c, writecache_io_hints() only calls
> dm_stack_bs_limits() and leaves limits->dma_alignment untouched:
>
> drivers/md/dm-writecache.c:writecache_io_hints() {
> struct dm_writecache *wc = ti->private;
>
> dm_stack_bs_limits(limits, wc->block_size);
> }
>
> In persistent memory mode, writecache_map_read() and writecache_map_write()
> process multi-block bios in a loop using bio->bi_iter.bi_sector to look up
> and record cache entry sectors via write_original_sector_seq_count(), calling
> bio_copy_block() for each block:
>
> drivers/md/dm-writecache.c:bio_copy_block() {
> do {
> struct bio_vec bv = bio_iter_iovec(bio, bio->bi_iter);
> ...
> size = bv.bv_len;
> if (unlikely(size > remaining_size))
> size = remaining_size;
> ...
> data = (char *)data + size;
> remaining_size -= size;
> bio_advance(bio, size);
> } while (unlikely(remaining_size));
> }
>
> When a cache block spans multiple bio_vec segments with sub-sector lengths
> (such as 4092 and 4 for a 4096-byte block), bio_advance() ->
> bio_advance_iter() updates bio->bi_iter.bi_sector += size >> 9 on each step,
> advancing bi_sector by 7 sectors instead of 8 and corrupting
> bio->bi_iter.bi_sector for subsequent blocks in the bio.
>
> In addition, when bio_copy_block() calls memcpy_flushcache_optimized() with a
> non-64-byte-multiple size >= 768 (such as 4092), doesn't the loop return
> early without copying the remaining size % 64 bytes?
>
> drivers/md/dm-writecache.c:memcpy_flushcache_optimized() {
> if (cpu_feature_enabled(X86_FEATURE_CLFLUSHOPT) &&
> likely(boot_cpu_data.x86_clflush_size == 64) &&
> likely(size >= 768)) {
> do {
> memcpy((void *)dest, (void *)source, 64);
> clflushopt((void *)dest);
> dest += 64;
> source += 64;
> size -= 64;
> } while (size >= 64);
> return;
> }
> }
>
> Finally, in solid-state cache mode, writecache_iterate_devices() only stacks
> limits from wc->dev and not wc->ssd_dev:
>
> drivers/md/dm-writecache.c:writecache_iterate_devices() {
> struct dm_writecache *wc = ti->private;
>
> return fn(ti, wc->dev, 0, ti->len, data);
> }
>
> If wc->dev has dma_alignment = 3 while wc->ssd_dev has dma_alignment = 511,
> wouldn't dm-writecache advertise dma_alignment = 3 and remap 4-byte-aligned
> bios to wc->ssd_dev->bdev, causing bio_split_io_at() to reject them?
>
> > lim->seg_boundary_mask = BLK_SEG_BOUNDARY_MASK;
> >
> > /* Inherit limits from component devices */
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/atpBVtPqGDWZ2AFo@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: block: let a stacking device inherit its members' DMA alignment
2026-10-23 15:41 ` Mike Snitzer
@ 2026-10-23 20:47 ` Mikulas Patocka
0 siblings, 0 replies; 12+ messages in thread
From: Mikulas Patocka @ 2026-10-23 20:47 UTC (permalink / raw)
To: Mike Snitzer
Cc: sashiko-bot, Keith Busch, axboe, dm-devel, bmarzins, Keith Busch
On Fri, 23 Oct 2026, Mike Snitzer wrote:
> On Thu, Oct 22, 2026 at 05:25:02PM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Critical] block: blk_set_stacking_limits() corrupts data in dm-ebs and dm-writecache
> > --
> >
> > commit da5c2e4cde4a12a4f8b254dc27480cf9078b36d2
> > Author: Mike Snitzer <snitzer@kernel.org>
> >
> > block: let a stacking device inherit its members' DMA alignment
> >
> > This commit removes the default sector-size dma_alignment initialization in
> > blk_set_stacking_limits() so that stacking devices inherit the strictest
> > dma_alignment of their underlying member devices. When no member devices are
> > stacked, blk_validate_limits() still applies the default sector alignment.
> >
> > > diff --git a/block/blk-settings.c b/block/blk-settings.c
> > > index e469baa1f08b7..36fcbecdf793b 100644
> > > --- a/block/blk-settings.c
> > > +++ b/block/blk-settings.c
> > > @@ -41,7 +41,14 @@ void blk_set_stacking_limits(struct queue_limits *lim)
> > > lim->physical_block_size = SECTOR_SIZE;
> > > lim->io_min = SECTOR_SIZE;
> > > lim->discard_granularity = SECTOR_SIZE;
> > > - lim->dma_alignment = SECTOR_SIZE - 1;
> > > + /*
> > > + * No dma_alignment of its own: blk_stack_limits() keeps the strictest
> > > + * of the devices stacked under it, and blk_validate_limits() gives the
> > > + * sector-size default to a device left with none. A driver that looks
> > > + * at the data and needs it laid out by logical block says so in its
> > > + * own limits, as dm-crypt, dm-integrity, dm-verity and dm-log-writes
> > > + * do.
> > > + */
> >
> > [Severity: Critical]
> > Could leaving lim->dma_alignment at 0 in blk_set_stacking_limits() cause data
> > corruption regressions in targets like dm-ebs and dm-writecache that inspect
> > bio data but do not set limits->dma_alignment in their io_hints callbacks?
> >
> > When stacked over a device with sub-sector dma_alignment (such as an nvme
> > device with dma_alignment = 3), direct I/O using a 4-byte-aligned user buffer
> > that crosses a page boundary or uses sub-sector iovecs can produce bio_vec
> > segments whose lengths are not multiples of 512 bytes (for example,
> > bv_len = 4092 followed by bv_len = 4).
>
> The code comment in the patch calls out the fact that
> blk_validate_limits() will backfill dma_alignment if none is set (by
> usual limits stacking or explicitly by DM targets).
>
> But sure, there is potential for dragons if stacked block devices
> like DM implicitly relied on block-core's coarse-grained default
> dma_alignment.
>
> Mikulas, would you be open to auditing the DM targets relative to
> dma_alignment stacking after you're done with reviewing Keith's
> related dm-crypt patchset?
Yes, I think that this is the only way forward - and this audit should be
done before accepting the dm-crypt patches.
DM cannot rely on stacking limits, because the DM stack may be
reconfigured anytime - so there is a race condition that a dm-device may
advertise dma alignment 3, someone may construct a bio that uses 4-byte
aligned vectors, the stack may be reconfigured with a completely different
target and the bio may be sent to this different target and cause trouble.
So, we need to be prepared to handle any alignment at device mapper entry.
I think we should add a new flag "accepts_unaligned_bio_vector" to struct
dm_target. If the flag is not present, then the dm-core would check the
alignment and reject bios with BLK_STS_INVAL in the dm core.
Then, we can gradually enable the flag for more and more targets as they
are checked.
Mikulas
> Sorry for the extra work... but its worthwhile for DM to be more
> precise with how it stacks dma_alignment
>
> Thanks,
> Mike
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-10-23 20:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 16:15 [PATCHv5 0/3] dm-crypt: finer grained memory alignment Keith Busch
2026-07-09 16:15 ` [PATCHv5 1/3] dm: initialize dma_alignment to 0 Keith Busch
2026-10-22 17:01 ` [PATCH] block: let a stacking device inherit its members' DMA alignment Mike Snitzer
2026-10-22 17:25 ` sashiko-bot
2026-10-23 15:41 ` Mike Snitzer
2026-10-23 20:47 ` Mikulas Patocka
2026-07-09 16:15 ` [PATCHv5 2/3] dm-crypt: handle memory allocation failures Keith Busch
2026-07-09 16:15 ` [PATCHv5 3/3] dm-crypt: allow unaligned bio_vecs for direct io Keith Busch
2026-07-11 0:54 ` Eric Biggers
2026-07-11 4:14 ` Keith Busch
2026-07-11 4:30 ` Eric Biggers
2026-07-13 18:54 ` Keith Busch
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.