* [PATCH 1/4] block: better split mq vs non-mq code in add_disk_fwnode
2025-01-06 8:35 more BLK_MQ_F_* simplification v2 Christoph Hellwig
@ 2025-01-06 8:35 ` Christoph Hellwig
2025-01-06 8:35 ` [PATCH 2/4] block: remove blk_mq_init_bitmaps Christoph Hellwig
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2025-01-06 8:35 UTC (permalink / raw)
To: Jens Axboe
Cc: Damien Le Moal, Niklas Cassel, Martin K. Petersen, John Garry,
linux-block, linux-ide, linux-scsi
Add a big conditional for blk-mq vs not mq at the beginning of
add_disk_fwnode so that elevator_init_mq is only called for blk-mq disks,
and add checks that the right methods or set or not set based on the
queue type.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/genhd.c | 28 +++++++++++++++-------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 5da3c9a64e64..befb7a516bcf 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -400,21 +400,23 @@ int __must_check add_disk_fwnode(struct device *parent, struct gendisk *disk,
struct device *ddev = disk_to_dev(disk);
int ret;
- /* Only makes sense for bio-based to set ->poll_bio */
- if (queue_is_mq(disk->queue) && disk->fops->poll_bio)
- return -EINVAL;
-
- /*
- * The disk queue should now be all set with enough information about
- * the device for the elevator code to pick an adequate default
- * elevator if one is needed, that is, for devices requesting queue
- * registration.
- */
- elevator_init_mq(disk->queue);
+ if (queue_is_mq(disk->queue)) {
+ /*
+ * ->submit_bio and ->poll_bio are bypassed for blk-mq drivers.
+ */
+ if (disk->fops->submit_bio || disk->fops->poll_bio)
+ return -EINVAL;
- /* Mark bdev as having a submit_bio, if needed */
- if (disk->fops->submit_bio)
+ /*
+ * Initialize the I/O scheduler code and pick a default one if
+ * needed.
+ */
+ elevator_init_mq(disk->queue);
+ } else {
+ if (!disk->fops->submit_bio)
+ return -EINVAL;
bdev_set_flag(disk->part0, BD_HAS_SUBMIT_BIO);
+ }
/*
* If the driver provides an explicit major number it also must provide
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/4] block: remove blk_mq_init_bitmaps
2025-01-06 8:35 more BLK_MQ_F_* simplification v2 Christoph Hellwig
2025-01-06 8:35 ` [PATCH 1/4] block: better split mq vs non-mq code in add_disk_fwnode Christoph Hellwig
@ 2025-01-06 8:35 ` Christoph Hellwig
2025-01-06 8:35 ` [PATCH 3/4] block: remove BLK_MQ_F_NO_SCHED Christoph Hellwig
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2025-01-06 8:35 UTC (permalink / raw)
To: Jens Axboe
Cc: Damien Le Moal, Niklas Cassel, Martin K. Petersen, John Garry,
linux-block, linux-ide, linux-scsi
The little work done in blk_mq_init_bitmaps is easier done in the only
caller.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: John Garry <john.g.garry@oracle.com>
---
block/blk-mq-tag.c | 38 ++++++++++++--------------------------
block/blk-mq.h | 3 ---
2 files changed, 12 insertions(+), 29 deletions(-)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 2cafcf11ee8b..ab4a66791a20 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -544,30 +544,12 @@ static int bt_alloc(struct sbitmap_queue *bt, unsigned int depth,
node);
}
-int blk_mq_init_bitmaps(struct sbitmap_queue *bitmap_tags,
- struct sbitmap_queue *breserved_tags,
- unsigned int queue_depth, unsigned int reserved,
- int node, int alloc_policy)
-{
- unsigned int depth = queue_depth - reserved;
- bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR;
-
- if (bt_alloc(bitmap_tags, depth, round_robin, node))
- return -ENOMEM;
- if (bt_alloc(breserved_tags, reserved, round_robin, node))
- goto free_bitmap_tags;
-
- return 0;
-
-free_bitmap_tags:
- sbitmap_queue_free(bitmap_tags);
- return -ENOMEM;
-}
-
struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
unsigned int reserved_tags,
int node, int alloc_policy)
{
+ unsigned int depth = total_tags - reserved_tags;
+ bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR;
struct blk_mq_tags *tags;
if (total_tags > BLK_MQ_TAG_MAX) {
@@ -582,14 +564,18 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
tags->nr_tags = total_tags;
tags->nr_reserved_tags = reserved_tags;
spin_lock_init(&tags->lock);
+ if (bt_alloc(&tags->bitmap_tags, depth, round_robin, node))
+ goto out_free_tags;
+ if (bt_alloc(&tags->breserved_tags, reserved_tags, round_robin, node))
+ goto out_free_bitmap_tags;
- if (blk_mq_init_bitmaps(&tags->bitmap_tags, &tags->breserved_tags,
- total_tags, reserved_tags, node,
- alloc_policy) < 0) {
- kfree(tags);
- return NULL;
- }
return tags;
+
+out_free_bitmap_tags:
+ sbitmap_queue_free(&tags->bitmap_tags);
+out_free_tags:
+ kfree(tags);
+ return NULL;
}
void blk_mq_free_tags(struct blk_mq_tags *tags)
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 89a20fffa4b1..3bb9ea80f9b6 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -165,9 +165,6 @@ struct blk_mq_alloc_data {
struct blk_mq_tags *blk_mq_init_tags(unsigned int nr_tags,
unsigned int reserved_tags, int node, int alloc_policy);
void blk_mq_free_tags(struct blk_mq_tags *tags);
-int blk_mq_init_bitmaps(struct sbitmap_queue *bitmap_tags,
- struct sbitmap_queue *breserved_tags, unsigned int queue_depth,
- unsigned int reserved, int node, int alloc_policy);
unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data);
unsigned long blk_mq_get_tags(struct blk_mq_alloc_data *data, int nr_tags,
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/4] block: remove BLK_MQ_F_NO_SCHED
2025-01-06 8:35 more BLK_MQ_F_* simplification v2 Christoph Hellwig
2025-01-06 8:35 ` [PATCH 1/4] block: better split mq vs non-mq code in add_disk_fwnode Christoph Hellwig
2025-01-06 8:35 ` [PATCH 2/4] block: remove blk_mq_init_bitmaps Christoph Hellwig
@ 2025-01-06 8:35 ` Christoph Hellwig
2025-01-06 8:35 ` [PATCH 4/4] block: simplify tag allocation policy selection Christoph Hellwig
2025-01-06 14:39 ` more BLK_MQ_F_* simplification v2 Jens Axboe
4 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2025-01-06 8:35 UTC (permalink / raw)
To: Jens Axboe
Cc: Damien Le Moal, Niklas Cassel, Martin K. Petersen, John Garry,
linux-block, linux-ide, linux-scsi
The only queues that really can't support a scheduler are those that
do not have a gendisk associated with them, and thus can't be used for
non-passthrough commands. In addition to those null_blk can optionally
set the flag, which is a bad odd. Replace the null_blk usage with
BLK_MQ_F_NO_SCHED_BY_DEFAULT to keep the expected semantics and then
remove BLK_MQ_F_NO_SCHED as the non-disk queues never call into
elevator_init_mq or blk_register_queue which adds the sysfs attributes.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-mq-debugfs.c | 1 -
block/bsg-lib.c | 2 +-
block/elevator.c | 20 --------------------
drivers/block/null_blk/main.c | 4 ++--
drivers/nvme/host/apple.c | 1 -
drivers/nvme/host/core.c | 1 -
drivers/ufs/core/ufshcd.c | 1 -
include/linux/blk-mq.h | 2 --
8 files changed, 3 insertions(+), 29 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 4b6b20ccdb53..64b3c333aa47 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -185,7 +185,6 @@ static const char *const hctx_flag_name[] = {
HCTX_FLAG_NAME(STACKING),
HCTX_FLAG_NAME(TAG_HCTX_SHARED),
HCTX_FLAG_NAME(BLOCKING),
- HCTX_FLAG_NAME(NO_SCHED),
HCTX_FLAG_NAME(NO_SCHED_BY_DEFAULT),
};
#undef HCTX_FLAG_NAME
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 32da4a4429ce..93523d8f8195 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -381,7 +381,7 @@ struct request_queue *bsg_setup_queue(struct device *dev, const char *name,
set->queue_depth = 128;
set->numa_node = NUMA_NO_NODE;
set->cmd_size = sizeof(struct bsg_job) + dd_job_size;
- set->flags = BLK_MQ_F_NO_SCHED | BLK_MQ_F_BLOCKING;
+ set->flags = BLK_MQ_F_BLOCKING;
if (blk_mq_alloc_tag_set(set))
goto out_tag_set;
diff --git a/block/elevator.c b/block/elevator.c
index be6e994256ac..b81216c48b6b 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -547,14 +547,6 @@ void elv_unregister(struct elevator_type *e)
}
EXPORT_SYMBOL_GPL(elv_unregister);
-static inline bool elv_support_iosched(struct request_queue *q)
-{
- if (!queue_is_mq(q) ||
- (q->tag_set->flags & BLK_MQ_F_NO_SCHED))
- return false;
- return true;
-}
-
/*
* For single queue devices, default to using mq-deadline. If we have multiple
* queues or mq-deadline is not available, default to "none".
@@ -580,9 +572,6 @@ void elevator_init_mq(struct request_queue *q)
struct elevator_type *e;
int err;
- if (!elv_support_iosched(q))
- return;
-
WARN_ON_ONCE(blk_queue_registered(q));
if (unlikely(q->elevator))
@@ -714,9 +703,6 @@ void elv_iosched_load_module(struct gendisk *disk, const char *buf,
struct elevator_type *found;
const char *name;
- if (!elv_support_iosched(disk->queue))
- return;
-
strscpy(elevator_name, buf, sizeof(elevator_name));
name = strstrip(elevator_name);
@@ -734,9 +720,6 @@ ssize_t elv_iosched_store(struct gendisk *disk, const char *buf,
char elevator_name[ELV_NAME_MAX];
int ret;
- if (!elv_support_iosched(disk->queue))
- return count;
-
strscpy(elevator_name, buf, sizeof(elevator_name));
ret = elevator_change(disk->queue, strstrip(elevator_name));
if (!ret)
@@ -751,9 +734,6 @@ ssize_t elv_iosched_show(struct gendisk *disk, char *name)
struct elevator_type *cur = NULL, *e;
int len = 0;
- if (!elv_support_iosched(q))
- return sprintf(name, "none\n");
-
if (!q->elevator) {
len += sprintf(name+len, "[none] ");
} else {
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index 178e62cd9a9f..d94ef37480bd 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1792,7 +1792,7 @@ static int null_init_global_tag_set(void)
tag_set.queue_depth = g_hw_queue_depth;
tag_set.numa_node = g_home_node;
if (g_no_sched)
- tag_set.flags |= BLK_MQ_F_NO_SCHED;
+ tag_set.flags |= BLK_MQ_F_NO_SCHED_BY_DEFAULT;
if (g_shared_tag_bitmap)
tag_set.flags |= BLK_MQ_F_TAG_HCTX_SHARED;
if (g_blocking)
@@ -1817,7 +1817,7 @@ static int null_setup_tagset(struct nullb *nullb)
nullb->tag_set->queue_depth = nullb->dev->hw_queue_depth;
nullb->tag_set->numa_node = nullb->dev->home_node;
if (nullb->dev->no_sched)
- nullb->tag_set->flags |= BLK_MQ_F_NO_SCHED;
+ nullb->tag_set->flags |= BLK_MQ_F_NO_SCHED_BY_DEFAULT;
if (nullb->dev->shared_tag_bitmap)
nullb->tag_set->flags |= BLK_MQ_F_TAG_HCTX_SHARED;
if (nullb->dev->blocking)
diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
index 83c60468542c..1de11b722f04 100644
--- a/drivers/nvme/host/apple.c
+++ b/drivers/nvme/host/apple.c
@@ -1251,7 +1251,6 @@ static int apple_nvme_alloc_tagsets(struct apple_nvme *anv)
anv->admin_tagset.timeout = NVME_ADMIN_TIMEOUT;
anv->admin_tagset.numa_node = NUMA_NO_NODE;
anv->admin_tagset.cmd_size = sizeof(struct apple_nvme_iod);
- anv->admin_tagset.flags = BLK_MQ_F_NO_SCHED;
anv->admin_tagset.driver_data = &anv->adminq;
ret = blk_mq_alloc_tag_set(&anv->admin_tagset);
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 42283d268500..c2250ddef5a2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4564,7 +4564,6 @@ int nvme_alloc_admin_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
/* Reserved for fabric connect and keep alive */
set->reserved_tags = 2;
set->numa_node = ctrl->numa_node;
- set->flags = BLK_MQ_F_NO_SCHED;
if (ctrl->ops->flags & NVME_F_BLOCKING)
set->flags |= BLK_MQ_F_BLOCKING;
set->cmd_size = cmd_size;
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 8a01e4393159..fd53c9f402c3 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -10412,7 +10412,6 @@ static int ufshcd_add_scsi_host(struct ufs_hba *hba)
.nr_hw_queues = 1,
.queue_depth = hba->nutmrs,
.ops = &ufshcd_tmf_ops,
- .flags = BLK_MQ_F_NO_SCHED,
};
err = blk_mq_alloc_tag_set(&hba->tmf_tag_set);
if (err < 0)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 6340293511c9..f2ff0ffa0535 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -676,8 +676,6 @@ enum {
BLK_MQ_F_STACKING = 1 << 2,
BLK_MQ_F_TAG_HCTX_SHARED = 1 << 3,
BLK_MQ_F_BLOCKING = 1 << 4,
- /* Do not allow an I/O scheduler to be configured. */
- BLK_MQ_F_NO_SCHED = 1 << 5,
/*
* Select 'none' during queue registration in case of a single hwq
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/4] block: simplify tag allocation policy selection
2025-01-06 8:35 more BLK_MQ_F_* simplification v2 Christoph Hellwig
` (2 preceding siblings ...)
2025-01-06 8:35 ` [PATCH 3/4] block: remove BLK_MQ_F_NO_SCHED Christoph Hellwig
@ 2025-01-06 8:35 ` Christoph Hellwig
2025-01-06 8:50 ` John Garry
2025-01-06 14:39 ` more BLK_MQ_F_* simplification v2 Jens Axboe
4 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2025-01-06 8:35 UTC (permalink / raw)
To: Jens Axboe
Cc: Damien Le Moal, Niklas Cassel, Martin K. Petersen, John Garry,
linux-block, linux-ide, linux-scsi
Use a plain BLK_MQ_F_* flag to select the round robin tag selection
instead of overlaying an enum with just two possible values into the
flags space.
Doing so allows adding a BLK_MQ_F_MAX sentinel for simplified overflow
checking in the messy debugfs helpers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-mq-debugfs.c | 25 ++++---------------------
block/blk-mq-tag.c | 5 ++---
block/blk-mq.c | 3 +--
block/blk-mq.h | 2 +-
drivers/ata/ahci.h | 2 +-
drivers/ata/pata_macio.c | 2 +-
drivers/ata/sata_mv.c | 2 +-
drivers/ata/sata_nv.c | 4 ++--
drivers/ata/sata_sil24.c | 1 -
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 2 +-
drivers/scsi/scsi_lib.c | 4 ++--
include/linux/blk-mq.h | 22 +++++++---------------
include/linux/libata.h | 4 ++--
include/scsi/scsi_host.h | 6 ++++--
14 files changed, 29 insertions(+), 55 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 64b3c333aa47..adf5f0697b6b 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -172,19 +172,13 @@ static int hctx_state_show(void *data, struct seq_file *m)
return 0;
}
-#define BLK_TAG_ALLOC_NAME(name) [BLK_TAG_ALLOC_##name] = #name
-static const char *const alloc_policy_name[] = {
- BLK_TAG_ALLOC_NAME(FIFO),
- BLK_TAG_ALLOC_NAME(RR),
-};
-#undef BLK_TAG_ALLOC_NAME
-
#define HCTX_FLAG_NAME(name) [ilog2(BLK_MQ_F_##name)] = #name
static const char *const hctx_flag_name[] = {
HCTX_FLAG_NAME(TAG_QUEUE_SHARED),
HCTX_FLAG_NAME(STACKING),
HCTX_FLAG_NAME(TAG_HCTX_SHARED),
HCTX_FLAG_NAME(BLOCKING),
+ HCTX_FLAG_NAME(TAG_RR),
HCTX_FLAG_NAME(NO_SCHED_BY_DEFAULT),
};
#undef HCTX_FLAG_NAME
@@ -192,22 +186,11 @@ static const char *const hctx_flag_name[] = {
static int hctx_flags_show(void *data, struct seq_file *m)
{
struct blk_mq_hw_ctx *hctx = data;
- const int alloc_policy = BLK_MQ_FLAG_TO_ALLOC_POLICY(hctx->flags);
- BUILD_BUG_ON(ARRAY_SIZE(hctx_flag_name) !=
- BLK_MQ_F_ALLOC_POLICY_START_BIT);
- BUILD_BUG_ON(ARRAY_SIZE(alloc_policy_name) != BLK_TAG_ALLOC_MAX);
+ BUILD_BUG_ON(ARRAY_SIZE(hctx_flag_name) != ilog2(BLK_MQ_F_MAX));
- seq_puts(m, "alloc_policy=");
- if (alloc_policy < ARRAY_SIZE(alloc_policy_name) &&
- alloc_policy_name[alloc_policy])
- seq_puts(m, alloc_policy_name[alloc_policy]);
- else
- seq_printf(m, "%d", alloc_policy);
- seq_puts(m, " ");
- blk_flags_show(m,
- hctx->flags ^ BLK_ALLOC_POLICY_TO_MQ_FLAG(alloc_policy),
- hctx_flag_name, ARRAY_SIZE(hctx_flag_name));
+ blk_flags_show(m, hctx->flags, hctx_flag_name,
+ ARRAY_SIZE(hctx_flag_name));
seq_puts(m, "\n");
return 0;
}
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index ab4a66791a20..b9f417d980b4 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -545,11 +545,10 @@ static int bt_alloc(struct sbitmap_queue *bt, unsigned int depth,
}
struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
- unsigned int reserved_tags,
- int node, int alloc_policy)
+ unsigned int reserved_tags, unsigned int flags, int node)
{
unsigned int depth = total_tags - reserved_tags;
- bool round_robin = alloc_policy == BLK_TAG_ALLOC_RR;
+ bool round_robin = flags & BLK_MQ_F_TAG_RR;
struct blk_mq_tags *tags;
if (total_tags > BLK_MQ_TAG_MAX) {
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 17f10683d640..2e6132f778fd 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3476,8 +3476,7 @@ static struct blk_mq_tags *blk_mq_alloc_rq_map(struct blk_mq_tag_set *set,
if (node == NUMA_NO_NODE)
node = set->numa_node;
- tags = blk_mq_init_tags(nr_tags, reserved_tags, node,
- BLK_MQ_FLAG_TO_ALLOC_POLICY(set->flags));
+ tags = blk_mq_init_tags(nr_tags, reserved_tags, set->flags, node);
if (!tags)
return NULL;
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 3bb9ea80f9b6..c872bbbe6411 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -163,7 +163,7 @@ struct blk_mq_alloc_data {
};
struct blk_mq_tags *blk_mq_init_tags(unsigned int nr_tags,
- unsigned int reserved_tags, int node, int alloc_policy);
+ unsigned int reserved_tags, unsigned int flags, int node);
void blk_mq_free_tags(struct blk_mq_tags *tags);
unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data);
diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
index 8f40f75ba08c..06781bdde0d2 100644
--- a/drivers/ata/ahci.h
+++ b/drivers/ata/ahci.h
@@ -396,7 +396,7 @@ extern const struct attribute_group *ahci_sdev_groups[];
.shost_groups = ahci_shost_groups, \
.sdev_groups = ahci_sdev_groups, \
.change_queue_depth = ata_scsi_change_queue_depth, \
- .tag_alloc_policy = BLK_TAG_ALLOC_RR, \
+ .tag_alloc_policy_rr = true, \
.device_configure = ata_scsi_device_configure
extern struct ata_port_operations ahci_ops;
diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c
index f2f36e55a1f4..4b01bb6880b0 100644
--- a/drivers/ata/pata_macio.c
+++ b/drivers/ata/pata_macio.c
@@ -935,7 +935,7 @@ static const struct scsi_host_template pata_macio_sht = {
.device_configure = pata_macio_device_configure,
.sdev_groups = ata_common_sdev_groups,
.can_queue = ATA_DEF_QUEUE,
- .tag_alloc_policy = BLK_TAG_ALLOC_RR,
+ .tag_alloc_policy_rr = true,
};
static struct ata_port_operations pata_macio_ops = {
diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
index b8f363370e1a..21c72650f9cc 100644
--- a/drivers/ata/sata_mv.c
+++ b/drivers/ata/sata_mv.c
@@ -672,7 +672,7 @@ static const struct scsi_host_template mv6_sht = {
.dma_boundary = MV_DMA_BOUNDARY,
.sdev_groups = ata_ncq_sdev_groups,
.change_queue_depth = ata_scsi_change_queue_depth,
- .tag_alloc_policy = BLK_TAG_ALLOC_RR,
+ .tag_alloc_policy_rr = true,
.device_configure = ata_scsi_device_configure
};
diff --git a/drivers/ata/sata_nv.c b/drivers/ata/sata_nv.c
index 36d99043ef50..823cce5ea1e9 100644
--- a/drivers/ata/sata_nv.c
+++ b/drivers/ata/sata_nv.c
@@ -385,7 +385,7 @@ static const struct scsi_host_template nv_adma_sht = {
.device_configure = nv_adma_device_configure,
.sdev_groups = ata_ncq_sdev_groups,
.change_queue_depth = ata_scsi_change_queue_depth,
- .tag_alloc_policy = BLK_TAG_ALLOC_RR,
+ .tag_alloc_policy_rr = true,
};
static const struct scsi_host_template nv_swncq_sht = {
@@ -396,7 +396,7 @@ static const struct scsi_host_template nv_swncq_sht = {
.device_configure = nv_swncq_device_configure,
.sdev_groups = ata_ncq_sdev_groups,
.change_queue_depth = ata_scsi_change_queue_depth,
- .tag_alloc_policy = BLK_TAG_ALLOC_RR,
+ .tag_alloc_policy_rr = true,
};
/*
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 72c03cbdaff4..935b13e79dec 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -378,7 +378,6 @@ static const struct scsi_host_template sil24_sht = {
.can_queue = SIL24_MAX_CMDS,
.sg_tablesize = SIL24_MAX_SGE,
.dma_boundary = ATA_DMA_BOUNDARY,
- .tag_alloc_policy = BLK_TAG_ALLOC_FIFO,
.sdev_groups = ata_ncq_sdev_groups,
.change_queue_depth = ata_scsi_change_queue_depth,
.device_configure = ata_scsi_device_configure
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 79129c977704..35501d0aa655 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3345,7 +3345,7 @@ static const struct scsi_host_template sht_v3_hw = {
.slave_alloc = hisi_sas_slave_alloc,
.shost_groups = host_v3_hw_groups,
.sdev_groups = sdev_groups_v3_hw,
- .tag_alloc_policy = BLK_TAG_ALLOC_RR,
+ .tag_alloc_policy_rr = true,
.host_reset = hisi_sas_host_reset,
.host_tagset = 1,
.mq_poll = queue_complete_v3_hw,
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5cf124e13097..51c496ca9380 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2065,8 +2065,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
tag_set->queue_depth = shost->can_queue;
tag_set->cmd_size = cmd_size;
tag_set->numa_node = dev_to_node(shost->dma_dev);
- tag_set->flags |=
- BLK_ALLOC_POLICY_TO_MQ_FLAG(shost->hostt->tag_alloc_policy);
+ if (shost->hostt->tag_alloc_policy_rr)
+ tag_set->flags |= BLK_MQ_F_TAG_RR;
if (shost->queuecommand_may_block)
tag_set->flags |= BLK_MQ_F_BLOCKING;
tag_set->driver_data = shost;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index f2ff0ffa0535..a0a9007cc1e3 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -296,13 +296,6 @@ enum blk_eh_timer_return {
BLK_EH_RESET_TIMER,
};
-/* Keep alloc_policy_name[] in sync with the definitions below */
-enum {
- BLK_TAG_ALLOC_FIFO, /* allocate starting from 0 */
- BLK_TAG_ALLOC_RR, /* allocate starting from last allocated tag */
- BLK_TAG_ALLOC_MAX
-};
-
/**
* struct blk_mq_hw_ctx - State for a hardware queue facing the hardware
* block device
@@ -677,20 +670,19 @@ enum {
BLK_MQ_F_TAG_HCTX_SHARED = 1 << 3,
BLK_MQ_F_BLOCKING = 1 << 4,
+ /*
+ * Alloc tags on a round-robin base instead of the first available one.
+ */
+ BLK_MQ_F_TAG_RR = 1 << 5,
+
/*
* Select 'none' during queue registration in case of a single hwq
* or shared hwqs instead of 'mq-deadline'.
*/
BLK_MQ_F_NO_SCHED_BY_DEFAULT = 1 << 6,
- BLK_MQ_F_ALLOC_POLICY_START_BIT = 7,
- BLK_MQ_F_ALLOC_POLICY_BITS = 1,
+
+ BLK_MQ_F_MAX = 1 << 7,
};
-#define BLK_MQ_FLAG_TO_ALLOC_POLICY(flags) \
- ((flags >> BLK_MQ_F_ALLOC_POLICY_START_BIT) & \
- ((1 << BLK_MQ_F_ALLOC_POLICY_BITS) - 1))
-#define BLK_ALLOC_POLICY_TO_MQ_FLAG(policy) \
- ((policy & ((1 << BLK_MQ_F_ALLOC_POLICY_BITS) - 1)) \
- << BLK_MQ_F_ALLOC_POLICY_START_BIT)
#define BLK_MQ_MAX_DEPTH (10240)
#define BLK_MQ_NO_HCTX_IDX (-1U)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c1a85d46eba6..be5183d75736 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1467,13 +1467,13 @@ extern const struct attribute_group *ata_common_sdev_groups[];
#define ATA_SUBBASE_SHT(drv_name) \
__ATA_BASE_SHT(drv_name), \
.can_queue = ATA_DEF_QUEUE, \
- .tag_alloc_policy = BLK_TAG_ALLOC_RR, \
+ .tag_alloc_policy_rr = true, \
.device_configure = ata_scsi_device_configure
#define ATA_SUBBASE_SHT_QD(drv_name, drv_qd) \
__ATA_BASE_SHT(drv_name), \
.can_queue = drv_qd, \
- .tag_alloc_policy = BLK_TAG_ALLOC_RR, \
+ .tag_alloc_policy_rr = true, \
.device_configure = ata_scsi_device_configure
#define ATA_BASE_SHT(drv_name) \
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 2b4ab0369ffb..02823d6af37d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -438,8 +438,10 @@ struct scsi_host_template {
*/
short cmd_per_lun;
- /* If use block layer to manage tags, this is tag allocation policy */
- int tag_alloc_policy;
+ /*
+ * Allocate tags starting from last allocated tag.
+ */
+ bool tag_alloc_policy_rr : 1;
/*
* Track QUEUE_FULL events and reduce queue depth on demand.
--
2.45.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] block: simplify tag allocation policy selection
2025-01-06 8:35 ` [PATCH 4/4] block: simplify tag allocation policy selection Christoph Hellwig
@ 2025-01-06 8:50 ` John Garry
2025-01-06 8:55 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: John Garry @ 2025-01-06 8:50 UTC (permalink / raw)
To: Christoph Hellwig, Jens Axboe
Cc: Damien Le Moal, Niklas Cassel, Martin K. Petersen, linux-block,
linux-ide, linux-scsi
On 06/01/2025 08:35, Christoph Hellwig wrote:
> Use a plain BLK_MQ_F_* flag to select the round robin tag selection
> instead of overlaying an enum with just two possible values into the
> flags space.
>
> Doing so allows adding a BLK_MQ_F_MAX sentinel for simplified overflow
> checking in the messy debugfs helpers.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Looks fine, but some small comments still. However, feel free to add:
Reviewed-by: John Garry <john.g.garry@oracle.com>
> /*
> diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
> index 72c03cbdaff4..935b13e79dec 100644
> --- a/drivers/ata/sata_sil24.c
> +++ b/drivers/ata/sata_sil24.c
> @@ -378,7 +378,6 @@ static const struct scsi_host_template sil24_sht = {
> .can_queue = SIL24_MAX_CMDS,
> .sg_tablesize = SIL24_MAX_SGE,
> .dma_boundary = ATA_DMA_BOUNDARY,
> - .tag_alloc_policy = BLK_TAG_ALLOC_FIFO,
nit: maybe that could be a separate patch, but no biggie
> .sdev_groups = ata_ncq_sdev_groups,
> .change_queue_depth = ata_scsi_change_queue_depth,
> .device_configure = ata_scsi_device_configure
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 79129c977704..35501d0aa655 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -438,8 +438,10 @@ struct scsi_host_template {
> */
> short cmd_per_lun;
>
> - /* If use block layer to manage tags, this is tag allocation policy */
> - int tag_alloc_policy;
> + /*
> + * Allocate tags starting from last allocated tag.
> + */
> + bool tag_alloc_policy_rr : 1;
Is it proper to use bool here? I am not sure. Others use unsigned int or
unsigned.
nit: coding style elsewhere in scsi_host_template would be to use
"tag_alloc_policy_rr:1;"
>
> /*
> * Track QUEUE_FULL events and reduce queue depth on demand.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 4/4] block: simplify tag allocation policy selection
2025-01-06 8:50 ` John Garry
@ 2025-01-06 8:55 ` Christoph Hellwig
2025-01-06 10:40 ` John Garry
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2025-01-06 8:55 UTC (permalink / raw)
To: John Garry
Cc: Christoph Hellwig, Jens Axboe, Damien Le Moal, Niklas Cassel,
Martin K. Petersen, linux-block, linux-ide, linux-scsi
On Mon, Jan 06, 2025 at 08:50:20AM +0000, John Garry wrote:
>> .can_queue = SIL24_MAX_CMDS,
>> .sg_tablesize = SIL24_MAX_SGE,
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> - .tag_alloc_policy = BLK_TAG_ALLOC_FIFO,
>
> nit: maybe that could be a separate patch, but no biggie
I though about that, but if felt a bit overkill.
>> + /*
>> + * Allocate tags starting from last allocated tag.
>> + */
>> + bool tag_alloc_policy_rr : 1;
>
> Is it proper to use bool here? I am not sure. Others use unsigned int or
> unsigned.
Yes, you can use any unsigned type for a single-bit bitfield. Most of
the existing uses just predate the availability of bool or were copy
and pasted after that.
>
> nit: coding style elsewhere in scsi_host_template would be to use
> "tag_alloc_policy_rr:1;"
Yes, but that's against the normal kernel coding style, so we'd better
stop adding more of that.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] block: simplify tag allocation policy selection
2025-01-06 8:55 ` Christoph Hellwig
@ 2025-01-06 10:40 ` John Garry
0 siblings, 0 replies; 10+ messages in thread
From: John Garry @ 2025-01-06 10:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Damien Le Moal, Niklas Cassel, Martin K. Petersen,
linux-block, linux-ide, linux-scsi
On 06/01/2025 08:55, Christoph Hellwig wrote:
>>> bool tag_alloc_policy_rr : 1;
>> Is it proper to use bool here? I am not sure. Others use unsigned int or
>> unsigned.
> Yes, you can use any unsigned type for a single-bit bitfield. Most of
> the existing uses just predate the availability of bool or were copy
> and pasted after that.
>
>> nit: coding style elsewhere in scsi_host_template would be to use
>> "tag_alloc_policy_rr:1;"
> Yes, but that's against the normal kernel coding style, so we'd better
> stop adding more of that.
I suppose then that the rest should be updated afterwards to be
consistent (with the kernel coding style). That should be easier than -
for example - removing extern usage.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: more BLK_MQ_F_* simplification v2
2025-01-06 8:35 more BLK_MQ_F_* simplification v2 Christoph Hellwig
` (3 preceding siblings ...)
2025-01-06 8:35 ` [PATCH 4/4] block: simplify tag allocation policy selection Christoph Hellwig
@ 2025-01-06 14:39 ` Jens Axboe
4 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2025-01-06 14:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Damien Le Moal, Niklas Cassel, Martin K. Petersen, John Garry,
linux-block, linux-ide, linux-scsi
On Mon, 06 Jan 2025 09:35:07 +0100, Christoph Hellwig wrote:
> this series removes another BLK_MQ_F_ that just duplicates an implicit
> condition and cleans up the tag allocation policy selection by using
> an actual BLK_MQ_F_ flag instead of a two-value enum awkwardly encoded
> into it. If we'd ever grow another policy we'd be much better off just
> adding a separate field to the tagset for it.
>
> Changes since v1:
> - use a boolean bitfield for the SCSI RR tag allocation policy selection
>
> [...]
Applied, thanks!
[1/4] block: better split mq vs non-mq code in add_disk_fwnode
commit: 6783811569aef24b949992bd5c4e6eaac02a0c30
[2/4] block: remove blk_mq_init_bitmaps
commit: 68ed45122249083bf45593ed635474282583352c
[3/4] block: remove BLK_MQ_F_NO_SCHED
commit: e7602bb4f3a1234df8b75728ac3260bcb8242612
[4/4] block: simplify tag allocation policy selection
commit: ce32496ec1abe866225f2e2005ceda68cf4c7bf4
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread