diff mbox series

nvme: Increase block size variable size to 32-bit

Message ID 20230530154231.8313-1-da.gomez@samsung.com
State New
Headers
Series nvme: Increase block size variable size to 32-bit |

Commit Message

Daniel Gomez May 30, 2023, 3:42 p.m. UTC 
  Increase block size variable size to 32-bit unsigned to be able to
support block devices larger than 32k (starting from 64 KiB).

Physical and logical block size already support unsigned 32-bit.

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---

While experimenting and doing code inspection for large block
devices, we found a limitation of a 32 KiB block size due to the bs
variable type. With that limitation and in combination with a large
block device, this results in a kernel BUG when the block layer
attempts to split a block size of 0 bytes.

Increasing the value to 32-bit unsigned, allows to support large block
devices starting from 64 KiB. In addition, this bs variable type
aligns with the queue_limits logical/physical_block_size types.


 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.40.1

Comments

Luis Chamberlain May 30, 2023, 4:24 p.m. UTC | #1 
On Tue, May 30, 2023 at 03:42:53PM +0000, Daniel Gomez wrote:
> Increase block size variable size to 32-bit unsigned to be able to
> support block devices larger than 32k (starting from 64 KiB).
> 
> Physical and logical block size already support unsigned 32-bit.
> 
> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>

Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis
Keith Busch May 30, 2023, 5:27 p.m. UTC | #2 
On Tue, May 30, 2023 at 03:42:53PM +0000, Daniel Gomez wrote:
> Increase block size variable size to 32-bit unsigned to be able to
> support block devices larger than 32k (starting from 64 KiB).

Looks good,

Reviewed-by: Keith Busch <kbusch@kernel.org>
Christoph Hellwig May 31, 2023, 1:03 p.m. UTC | #3 
> +	u32 bs = 1 << ns->lba_shift;

Make that 1 a 1U so that we're not going to run into sign extension
issues when using up all bits in the u32 :)

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
David Laight June 1, 2023, 10:28 a.m. UTC | #4 
From: Christoph Hellwig
> Sent: 31 May 2023 14:03
> 
> > +	u32 bs = 1 << ns->lba_shift;
> 
> Make that 1 a 1U so that we're not going to run into sign extension
> issues when using up all bits in the u32 :)

Not 'sign extension' but obscure integer shift/conversion issues
that really only affect 1's compliment and sign-overpunch cpu.
I'm not even sure gcc/clang support any non 2's compliment systems.

> Otherwise looks good:

Probably improves the generated code as well.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Keith Busch June 1, 2023, 5:10 p.m. UTC | #5 
Thanks, applied to 6.5 with the '1U' suggestion folded in.
diff mbox series

Patch

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 151b23400ada..b4bc48f2a011 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1835,7 +1835,7 @@  static void nvme_update_disk_info(struct gendisk *disk,
 		struct nvme_ns *ns, struct nvme_id_ns *id)
 {
 	sector_t capacity = nvme_lba_to_sect(ns, le64_to_cpu(id->nsze));
-	unsigned short bs = 1 << ns->lba_shift;
+	u32 bs = 1 << ns->lba_shift;
 	u32 atomic_bs, phys_bs, io_opt = 0;

 	/*