diff mbox series

[for-next,v3] null_blk: add configfs variable shared_tags

Message ID 20240130042134.2463659-1-shinichiro.kawasaki@wdc.com (mailing list archive)
State New, archived
Headers show
Series [for-next,v3] null_blk: add configfs variable shared_tags | expand

Commit Message

Shin'ichiro Kawasaki Jan. 30, 2024, 4:21 a.m. UTC
Allow setting shared_tags through configfs, which could only be set as a
module parameter. For that purpose, delay tag_set initialization from
null_init() to null_add_dev(). Refer tag_set.ops as the flag to check if
tag_set is initialized or not.

The following parameters can not be set through configfs yet:

    timeout
    requeue
    init_hctx

Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
This patch will allow running the blktests test cases block/010 and block/022
using the built-in null_blk driver. Corresponding blktests side changes are
drafted here [1].

[1] https://github.com/kawasaki/blktests/tree/shared_tags

Changes from v2:
* Used tag_set.ops instead of tag_set_initialized

Changes from v1:
* Removed unnecessary global variable initializer

 drivers/block/null_blk/main.c     | 38 ++++++++++++++++---------------
 drivers/block/null_blk/null_blk.h |  1 +
 2 files changed, 21 insertions(+), 18 deletions(-)

Comments

Chaitanya Kulkarni Jan. 30, 2024, 7:49 a.m. UTC | #1
On 1/29/2024 8:21 PM, Shin'ichiro Kawasaki wrote:
> Allow setting shared_tags through configfs, which could only be set as a
> module parameter. For that purpose, delay tag_set initialization from
> null_init() to null_add_dev(). Refer tag_set.ops as the flag to check if
> tag_set is initialized or not.
> 
> The following parameters can not be set through configfs yet:
> 
>      timeout
>      requeue
>      init_hctx
> 
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---

I believe it is passing shared tag blktests, with that it looks good to
me.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck
Shin'ichiro Kawasaki Jan. 30, 2024, 8:27 a.m. UTC | #2
On Jan 30, 2024 / 07:49, Chaitanya Kulkarni wrote:
...
> I believe it is passing shared tag blktests, with that it looks good to
> me.
> 
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Thanks. I tested the patch using block/010 and block/022 and confirmed they
pass. I also added some printks and confirmed that null_blk uses shared tags
(or device unique tags) as the test cases expect.
Jens Axboe Feb. 8, 2024, 6:32 p.m. UTC | #3
On Tue, 30 Jan 2024 13:21:34 +0900, Shin'ichiro Kawasaki wrote:
> Allow setting shared_tags through configfs, which could only be set as a
> module parameter. For that purpose, delay tag_set initialization from
> null_init() to null_add_dev(). Refer tag_set.ops as the flag to check if
> tag_set is initialized or not.
> 
> The following parameters can not be set through configfs yet:
> 
> [...]

Applied, thanks!

[1/1] null_blk: add configfs variable shared_tags
      commit: 14509b748ff58df3f0980b1cd70ade0e4a805e99

Best regards,
diff mbox series

Patch

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 36755f263e8e..4281371c81fe 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -165,8 +165,8 @@  static bool g_blocking;
 module_param_named(blocking, g_blocking, bool, 0444);
 MODULE_PARM_DESC(blocking, "Register as a blocking blk-mq driver device");
 
-static bool shared_tags;
-module_param(shared_tags, bool, 0444);
+static bool g_shared_tags;
+module_param_named(shared_tags, g_shared_tags, bool, 0444);
 MODULE_PARM_DESC(shared_tags, "Share tag set between devices for blk-mq");
 
 static bool g_shared_tag_bitmap;
@@ -426,6 +426,7 @@  NULLB_DEVICE_ATTR(zone_max_open, uint, NULL);
 NULLB_DEVICE_ATTR(zone_max_active, uint, NULL);
 NULLB_DEVICE_ATTR(virt_boundary, bool, NULL);
 NULLB_DEVICE_ATTR(no_sched, bool, NULL);
+NULLB_DEVICE_ATTR(shared_tags, bool, NULL);
 NULLB_DEVICE_ATTR(shared_tag_bitmap, bool, NULL);
 
 static ssize_t nullb_device_power_show(struct config_item *item, char *page)
@@ -571,6 +572,7 @@  static struct configfs_attribute *nullb_device_attrs[] = {
 	&nullb_device_attr_zone_offline,
 	&nullb_device_attr_virt_boundary,
 	&nullb_device_attr_no_sched,
+	&nullb_device_attr_shared_tags,
 	&nullb_device_attr_shared_tag_bitmap,
 	NULL,
 };
@@ -653,10 +655,11 @@  static ssize_t memb_group_features_show(struct config_item *item, char *page)
 			"badblocks,blocking,blocksize,cache_size,"
 			"completion_nsec,discard,home_node,hw_queue_depth,"
 			"irqmode,max_sectors,mbps,memory_backed,no_sched,"
-			"poll_queues,power,queue_mode,shared_tag_bitmap,size,"
-			"submit_queues,use_per_node_hctx,virt_boundary,zoned,"
-			"zone_capacity,zone_max_active,zone_max_open,"
-			"zone_nr_conv,zone_offline,zone_readonly,zone_size\n");
+			"poll_queues,power,queue_mode,shared_tag_bitmap,"
+			"shared_tags,size,submit_queues,use_per_node_hctx,"
+			"virt_boundary,zoned,zone_capacity,zone_max_active,"
+			"zone_max_open,zone_nr_conv,zone_offline,zone_readonly,"
+			"zone_size\n");
 }
 
 CONFIGFS_ATTR_RO(memb_group_, features);
@@ -738,6 +741,7 @@  static struct nullb_device *null_alloc_dev(void)
 	dev->zone_max_active = g_zone_max_active;
 	dev->virt_boundary = g_virt_boundary;
 	dev->no_sched = g_no_sched;
+	dev->shared_tags = g_shared_tags;
 	dev->shared_tag_bitmap = g_shared_tag_bitmap;
 	return dev;
 }
@@ -2124,7 +2128,14 @@  static int null_add_dev(struct nullb_device *dev)
 		goto out_free_nullb;
 
 	if (dev->queue_mode == NULL_Q_MQ) {
-		if (shared_tags) {
+		if (dev->shared_tags) {
+			if (!tag_set.ops) {
+				rv = null_init_tag_set(NULL, &tag_set);
+				if (rv) {
+					tag_set.ops = NULL;
+					goto out_cleanup_queues;
+				}
+			}
 			nullb->tag_set = &tag_set;
 			rv = 0;
 		} else {
@@ -2311,18 +2322,12 @@  static int __init null_init(void)
 		g_submit_queues = 1;
 	}
 
-	if (g_queue_mode == NULL_Q_MQ && shared_tags) {
-		ret = null_init_tag_set(NULL, &tag_set);
-		if (ret)
-			return ret;
-	}
-
 	config_group_init(&nullb_subsys.su_group);
 	mutex_init(&nullb_subsys.su_mutex);
 
 	ret = configfs_register_subsystem(&nullb_subsys);
 	if (ret)
-		goto err_tagset;
+		return ret;
 
 	mutex_init(&lock);
 
@@ -2349,9 +2354,6 @@  static int __init null_init(void)
 	unregister_blkdev(null_major, "nullb");
 err_conf:
 	configfs_unregister_subsystem(&nullb_subsys);
-err_tagset:
-	if (g_queue_mode == NULL_Q_MQ && shared_tags)
-		blk_mq_free_tag_set(&tag_set);
 	return ret;
 }
 
@@ -2370,7 +2372,7 @@  static void __exit null_exit(void)
 	}
 	mutex_unlock(&lock);
 
-	if (g_queue_mode == NULL_Q_MQ && shared_tags)
+	if (tag_set.ops)
 		blk_mq_free_tag_set(&tag_set);
 }
 
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index 929f659dd255..7bcfc0922ae8 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -119,6 +119,7 @@  struct nullb_device {
 	bool zoned; /* if device is zoned */
 	bool virt_boundary; /* virtual boundary on/off for the device */
 	bool no_sched; /* no IO scheduler for the device */
+	bool shared_tags; /* share tag set between devices for blk-mq */
 	bool shared_tag_bitmap; /* use hostwide shared tags */
 };