diff mbox series

blk-settings: round down io_opt to physical_block_size

Message ID 3dc0014b-9690-dc38-81c9-4a316a2d4fb2@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Mikulas Patocka
Headers show
Series blk-settings: round down io_opt to physical_block_size | expand

Commit Message

Mikulas Patocka Nov. 18, 2024, 2:52 p.m. UTC
On Mon, 18 Nov 2024, Mikulas Patocka wrote:

> On Sat, 16 Nov 2024, dap581 wrote:
> 
> > Hello,
> > Starting with kernel 6.11.x series up to current 6.12-rc7
> > when I unlock my encrypted hdd (storage, not root) with cryptsetup:
> > 
> > cryptsetup luksOpen /dev/sda1 hdd5 -d keyfile_hdd5
> > 
> > it triggers an alignment inconsistency:
> > In dmesg log, I read:
> > ----
> > [  105.841278] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > [  105.841292] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > [  105.841601] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > [  105.841607] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > ----

Hi

Does this patch fix it?

Mikulas



From: Mikulas Patocka <mpatocka@redhat.com>

There was a bug report [1] where the user got a warning alignment
inconsistency. The user has optimal I/O 16776704 (0xFFFE00) and physical
block size 4096. Note that the optimal I/O size may be set by the DMA
engines or SCSI controllers and they have no knowledge about the disks
attached to them, so the situation with optimal I/O not aligned to
physical block size may happen.

This commit makes blk_validate_limits round down optimal I/O size to the
physical block size of the block device.

Closes: https://lore.kernel.org/dm-devel/1426ad71-79b4-4062-b2bf-84278be66a5d@redhat.com/T/ [1]
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Fixes: a23634644afc ("block: take io_opt and io_min into account for max_sectors")
Cc: stable@vger.kernel.org	# v6.11+

---
 block/blk-settings.c |    7 +++++++
 1 file changed, 7 insertions(+)

Comments

Christoph Hellwig Nov. 18, 2024, 4:26 p.m. UTC | #1
On Mon, Nov 18, 2024 at 03:52:50PM +0100, Mikulas Patocka wrote:
> 
> 
> On Mon, 18 Nov 2024, Mikulas Patocka wrote:
> 
> > On Sat, 16 Nov 2024, dap581 wrote:
> > 
> > > Hello,
> > > Starting with kernel 6.11.x series up to current 6.12-rc7
> > > when I unlock my encrypted hdd (storage, not root) with cryptsetup:
> > > 
> > > cryptsetup luksOpen /dev/sda1 hdd5 -d keyfile_hdd5
> > > 
> > > it triggers an alignment inconsistency:
> > > In dmesg log, I read:
> > > ----
> > > [  105.841278] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > [  105.841292] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > [  105.841601] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > [  105.841607] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > ----
> 
> Hi
> 
> Does this patch fix it?

FYI, this rounding down looks reasonable to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jens Axboe Nov. 18, 2024, 9:54 p.m. UTC | #2
On Mon, 18 Nov 2024 15:52:50 +0100, Mikulas Patocka wrote:
> On Mon, 18 Nov 2024, Mikulas Patocka wrote:
> 
> > On Sat, 16 Nov 2024, dap581 wrote:
> >
> > > Hello,
> > > Starting with kernel 6.11.x series up to current 6.12-rc7
> > > when I unlock my encrypted hdd (storage, not root) with cryptsetup:
> > >
> > > cryptsetup luksOpen /dev/sda1 hdd5 -d keyfile_hdd5
> > >
> > > it triggers an alignment inconsistency:
> > > In dmesg log, I read:
> > > ----
> > > [  105.841278] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > [  105.841292] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > [  105.841601] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > [  105.841607] device-mapper: table: 254:1: adding target device sda1 caused an alignment inconsistency: physical_block_size=4096, logical_block_size=512, alignment_offset=0, start=16777216
> > > ----
> 
> [...]

Applied, thanks!

[1/1] blk-settings: round down io_opt to physical_block_size
      commit: df199821cd4bf0c77c4fc4ca2fa978cedf5c8bcf

Best regards,
diff mbox series

Patch

Index: linux-2.6/block/blk-settings.c
===================================================================
--- linux-2.6.orig/block/blk-settings.c	2024-11-18 15:24:18.000000000 +0100
+++ linux-2.6/block/blk-settings.c	2024-11-18 15:29:24.000000000 +0100
@@ -250,6 +250,13 @@  static int blk_validate_limits(struct qu
 		lim->io_min = lim->physical_block_size;
 
 	/*
+	 * The optimal I/O size may not be aligned to physical block size
+	 * (because it may be limited by dma engines which have no clue about
+	 * block size of the disks attached to them), so we round it down here.
+	 */
+	lim->io_opt = round_down(lim->io_opt, lim->physical_block_size);
+
+	/*
 	 * max_hw_sectors has a somewhat weird default for historical reason,
 	 * but driver really should set their own instead of relying on this
 	 * value.