NVDIMM Device and Persistent Memory development
 help / color / mirror / Atom feed
* remove calls to blk_queue_max_integrity_segments
@ 2024-03-06 14:27 Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 1/3] nvdimm: remove nd_integrity_init Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Christoph Hellwig @ 2024-03-06 14:27 UTC (permalink / raw
  To: Jens Axboe, Mike Snitzer, Mikulas Patocka, Vishal Verma,
	Dan Williams, Dave Jiang, Ira Weiny
  Cc: dm-devel, nvdimm, linux-block

Hi all,

I forgot to convert two callers of blk_queue_max_integrity_segments to
the atomic queue_limits API.  This series fixes that and also cleans up
the nvdimm code a bit.

The patches are against Jens' for-6.9/block tree and it would be good if
we could still get them into the 6.9 merge window.

Diffstat:
 md/dm-integrity.c |    2 +-
 nvdimm/btt.c      |   12 ++++++++----
 nvdimm/core.c     |   30 ------------------------------
 nvdimm/nd.h       |    1 -
 4 files changed, 9 insertions(+), 36 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/3] nvdimm: remove nd_integrity_init
  2024-03-06 14:27 remove calls to blk_queue_max_integrity_segments Christoph Hellwig
@ 2024-03-06 14:27 ` Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 2/3] nvdimm/btt: always set max_integrity_segments Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2024-03-06 14:27 UTC (permalink / raw
  To: Jens Axboe, Mike Snitzer, Mikulas Patocka, Vishal Verma,
	Dan Williams, Dave Jiang, Ira Weiny
  Cc: dm-devel, nvdimm, linux-block

nd_integrity_init is only called from a single place.  Open code it
there, and use IS_ENABLED to remove the need for an extra stub.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvdimm/btt.c  | 12 ++++++++----
 drivers/nvdimm/core.c | 30 ------------------------------
 drivers/nvdimm/nd.h   |  1 -
 3 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 4d0c527e857678..8e855b4e3e383a 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -6,6 +6,7 @@
 #include <linux/highmem.h>
 #include <linux/debugfs.h>
 #include <linux/blkdev.h>
+#include <linux/blk-integrity.h>
 #include <linux/pagemap.h>
 #include <linux/module.h>
 #include <linux/device.h>
@@ -1514,10 +1515,13 @@ static int btt_blk_init(struct btt *btt)
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, btt->btt_disk->queue);
 	blk_queue_flag_set(QUEUE_FLAG_SYNCHRONOUS, btt->btt_disk->queue);
 
-	if (btt_meta_size(btt)) {
-		rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt));
-		if (rc)
-			goto out_cleanup_disk;
+	if (btt_meta_size(btt) && IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY)) {
+		struct blk_integrity bi = {
+			.tuple_size	= btt_meta_size(btt),
+			.tag_size	= btt_meta_size(btt),
+		};
+		blk_integrity_register(btt->btt_disk, &bi);
+		blk_queue_max_integrity_segments(btt->btt_disk->queue, 1);
 	}
 
 	set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9);
diff --git a/drivers/nvdimm/core.c b/drivers/nvdimm/core.c
index d91799b71d23a3..2023a661bbb0b8 100644
--- a/drivers/nvdimm/core.c
+++ b/drivers/nvdimm/core.c
@@ -7,7 +7,6 @@
 #include <linux/export.h>
 #include <linux/module.h>
 #include <linux/blkdev.h>
-#include <linux/blk-integrity.h>
 #include <linux/device.h>
 #include <linux/ctype.h>
 #include <linux/ndctl.h>
@@ -508,35 +507,6 @@ int nvdimm_bus_add_badrange(struct nvdimm_bus *nvdimm_bus, u64 addr, u64 length)
 }
 EXPORT_SYMBOL_GPL(nvdimm_bus_add_badrange);
 
-#ifdef CONFIG_BLK_DEV_INTEGRITY
-int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
-{
-	struct blk_integrity bi;
-
-	if (meta_size == 0)
-		return 0;
-
-	memset(&bi, 0, sizeof(bi));
-
-	bi.tuple_size = meta_size;
-	bi.tag_size = meta_size;
-
-	blk_integrity_register(disk, &bi);
-	blk_queue_max_integrity_segments(disk->queue, 1);
-
-	return 0;
-}
-EXPORT_SYMBOL(nd_integrity_init);
-
-#else /* CONFIG_BLK_DEV_INTEGRITY */
-int nd_integrity_init(struct gendisk *disk, unsigned long meta_size)
-{
-	return 0;
-}
-EXPORT_SYMBOL(nd_integrity_init);
-
-#endif
-
 static __init int libnvdimm_init(void)
 {
 	int rc;
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index ae2078eb6a6265..2dbb1dca17b534 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -489,7 +489,6 @@ enum nd_async_mode {
 	ND_ASYNC,
 };
 
-int nd_integrity_init(struct gendisk *disk, unsigned long meta_size);
 void wait_nvdimm_bus_probe_idle(struct device *dev);
 void nd_device_register(struct device *dev);
 void nd_device_unregister(struct device *dev, enum nd_async_mode mode);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] nvdimm/btt: always set max_integrity_segments
  2024-03-06 14:27 remove calls to blk_queue_max_integrity_segments Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 1/3] nvdimm: remove nd_integrity_init Christoph Hellwig
@ 2024-03-06 14:27 ` Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints Christoph Hellwig
  2 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2024-03-06 14:27 UTC (permalink / raw
  To: Jens Axboe, Mike Snitzer, Mikulas Patocka, Vishal Verma,
	Dan Williams, Dave Jiang, Ira Weiny
  Cc: dm-devel, nvdimm, linux-block

max_integrity_segments is just a hardware/driver limit and can be safely
set even when integrity data is not supported.  Set it in the initial
queue_limits passed to blk_alloc_disk to simplify the driver.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/nvdimm/btt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 8e855b4e3e383a..1e5aedaf8c7bd9 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1500,6 +1500,7 @@ static int btt_blk_init(struct btt *btt)
 	struct queue_limits lim = {
 		.logical_block_size	= btt->sector_size,
 		.max_hw_sectors		= UINT_MAX,
+		.max_integrity_segments	= 1,
 	};
 	int rc;
 
@@ -1521,7 +1522,6 @@ static int btt_blk_init(struct btt *btt)
 			.tag_size	= btt_meta_size(btt),
 		};
 		blk_integrity_register(btt->btt_disk, &bi);
-		blk_queue_max_integrity_segments(btt->btt_disk->queue, 1);
 	}
 
 	set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9);
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints
  2024-03-06 14:27 remove calls to blk_queue_max_integrity_segments Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 1/3] nvdimm: remove nd_integrity_init Christoph Hellwig
  2024-03-06 14:27 ` [PATCH 2/3] nvdimm/btt: always set max_integrity_segments Christoph Hellwig
@ 2024-03-06 14:27 ` Christoph Hellwig
  2024-03-06 17:25   ` Mike Snitzer
  2 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2024-03-06 14:27 UTC (permalink / raw
  To: Jens Axboe, Mike Snitzer, Mikulas Patocka, Vishal Verma,
	Dan Williams, Dave Jiang, Ira Weiny
  Cc: dm-devel, nvdimm, linux-block

Set max_integrity_segments with the other queue limits instead
of updating it later.  This also uncovered that the driver is trying
to set the limit to UINT_MAX while max_integrity_segments is an
unsigned short, so fix it up to use USHRT_MAX instead.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-integrity.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index c5f03aab455256..a2e5cfe84565ae 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -3419,6 +3419,7 @@ static void dm_integrity_io_hints(struct dm_target *ti, struct queue_limits *lim
 		blk_limits_io_min(limits, ic->sectors_per_block << SECTOR_SHIFT);
 		limits->dma_alignment = limits->logical_block_size - 1;
 	}
+	limits->max_integrity_segments = USHRT_MAX;
 }
 
 static void calculate_journal_section_size(struct dm_integrity_c *ic)
@@ -3586,7 +3587,6 @@ static void dm_integrity_set(struct dm_target *ti, struct dm_integrity_c *ic)
 	bi.interval_exp = ic->sb->log2_sectors_per_block + SECTOR_SHIFT;
 
 	blk_integrity_register(disk, &bi);
-	blk_queue_max_integrity_segments(disk->queue, UINT_MAX);
 }
 
 static void dm_integrity_free_page_list(struct page_list *pl)
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints
  2024-03-06 14:27 ` [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints Christoph Hellwig
@ 2024-03-06 17:25   ` Mike Snitzer
  2024-03-06 21:57     ` Christoph Hellwig
  0 siblings, 1 reply; 6+ messages in thread
From: Mike Snitzer @ 2024-03-06 17:25 UTC (permalink / raw
  To: Christoph Hellwig
  Cc: Jens Axboe, Mikulas Patocka, Vishal Verma, Dan Williams,
	Dave Jiang, Ira Weiny, dm-devel, nvdimm, linux-block

On Wed, Mar 06 2024 at  9:27P -0500,
Christoph Hellwig <hch@lst.de> wrote:

> Set max_integrity_segments with the other queue limits instead
> of updating it later.  This also uncovered that the driver is trying
> to set the limit to UINT_MAX while max_integrity_segments is an
> unsigned short, so fix it up to use USHRT_MAX instead.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/dm-integrity.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index c5f03aab455256..a2e5cfe84565ae 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -3419,6 +3419,7 @@ static void dm_integrity_io_hints(struct dm_target *ti, struct queue_limits *lim
>  		blk_limits_io_min(limits, ic->sectors_per_block << SECTOR_SHIFT);
>  		limits->dma_alignment = limits->logical_block_size - 1;
>  	}
> +	limits->max_integrity_segments = USHRT_MAX;
>  }
>  
>  static void calculate_journal_section_size(struct dm_integrity_c *ic)
> @@ -3586,7 +3587,6 @@ static void dm_integrity_set(struct dm_target *ti, struct dm_integrity_c *ic)
>  	bi.interval_exp = ic->sb->log2_sectors_per_block + SECTOR_SHIFT;
>  
>  	blk_integrity_register(disk, &bi);
> -	blk_queue_max_integrity_segments(disk->queue, UINT_MAX);
>  }
>  
>  static void dm_integrity_free_page_list(struct page_list *pl)
> -- 
> 2.39.2
> 

I've picked this up for 6.9:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-6.9&id=f30e5ed1306be8a900b33317bc429dd3794d81a1

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints
  2024-03-06 17:25   ` Mike Snitzer
@ 2024-03-06 21:57     ` Christoph Hellwig
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2024-03-06 21:57 UTC (permalink / raw
  To: Mike Snitzer
  Cc: Christoph Hellwig, Jens Axboe, Mikulas Patocka, Vishal Verma,
	Dan Williams, Dave Jiang, Ira Weiny, dm-devel, nvdimm,
	linux-block

On Wed, Mar 06, 2024 at 12:25:42PM -0500, Mike Snitzer wrote:
> I've picked this up for 6.9:
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-6.9&id=f30e5ed1306be8a900b33317bc429dd3794d81a1

A yes, dm was already passing the limits around so there is no
dependency on the block changes.  The dm tree is defintively the
better place then.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-03-06 21:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-06 14:27 remove calls to blk_queue_max_integrity_segments Christoph Hellwig
2024-03-06 14:27 ` [PATCH 1/3] nvdimm: remove nd_integrity_init Christoph Hellwig
2024-03-06 14:27 ` [PATCH 2/3] nvdimm/btt: always set max_integrity_segments Christoph Hellwig
2024-03-06 14:27 ` [PATCH 3/3] dm-integrity: set max_integrity_segments in dm_integrity_io_hints Christoph Hellwig
2024-03-06 17:25   ` Mike Snitzer
2024-03-06 21:57     ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).