On Wed, 18 Mar 2026, Keith Busch wrote:
> On Wed, Mar 18, 2026 at 05:19:39PM +0100, Mikulas Patocka wrote:
> > On Mon, 16 Mar 2026, Keith Busch wrote:
> > > + if (!unaligned_allowed) {
> > > + cc->io_alignment = cc->sector_size - 1;
> > > + } else {
> > > + set_bit(CRYPT_DISCONTIGUOUS_SEGS, &cc->cipher_flags);
> > > + cc->io_alignment = 3;
> > > + }
> > > return 0;
> > > }
> >
> > Why is "3" there? Should there be the dma_alignment of the underlying
> > block device instead?
>
> It doesn't matter what the underlying block device reports because
> dm-crypt bounces the incoming plain text buffers to something aligned to
> the backing device for the cypher text.
dm-crypt bounces writes, but doesn't bounce reads. When doing reads, it
reads the data into the target buffer and decompresses them in place.
So, it must respect the alignment provided by the underlying device.
> Why 3? This can theoretically do 0, but you can't express that as a
> block limit; block layer will assume you didn't set anything and use a
> 511 default, which is exactly what I don't want. So why not 1 instead of
> 3? Three is the smallest non-zero cra_alignmask currently reported, and
> its inefficient to do byte aligned direct io anyway.
>
> > > --- a/drivers/md/dm-table.c
> > > +++ b/drivers/md/dm-table.c
> > > @@ -1767,6 +1767,7 @@ int dm_calculate_queue_limits(struct dm_table *t,
> > > bool zoned = false;
> > >
> > > dm_set_stacking_limits(limits);
> > > + limits->dma_alignment = 0;
> > >
> > > t->integrity_supported = true;
> > > for (unsigned int i = 0; i < t->num_targets; i++) {
> >
> > dm-integrity doesn't set dma_alignment if it is using 512-byte sector size
> > (assuming that there is default 511). This should be fixed in dm-integrity
> > before making this change.
> >
> > Other device mapper targets should also be reviewed to make sure that this
> > change doens't break them.
>
> The previous call to dm_set_stacking_limits() sets it to 511, which sets
> an unnecessary minimum. Setting it to 0 here means that the stacked
> device will inhereit whatever the mapper overrides it to, or go back to
> 511 if it decides not to override it.
If we have dm-integrity with 512-byte sector size on the top of a device
that has dma_alignment less than 512, the code would set
limits->dma_alignment to the value of the underlying block device and
break dm-integrity (which assumes sector-aligned bios).
But that's a problem of dm-integrity - this "if (ic->sectors_per_block >
1)" condition in dm_integrity_io_hints is weird, it should be dropped and
the limits should be properly merged.
Mikulas
