[PATCH RFC 00/11] block debugfs: Catch missing flag array members

[PATCH RFC 00/11] block debugfs: Catch missing flag array members

[John Garry]

Currently we rely on the developer to add the appropriate entry to the
debugfs flag array when we add a new member.

This has shown to be error prone.

As an attempt to solve this problem, add compile-time assertions that we
are not missing flag array entries.

This should solve the problem that we don't miss entries, but we still
rely on the developer to add in the proper order.

Marking as an RFC as I am not sure if this is the best approach. And the
enum-related changes will require further work, I think.

Christoph Hellwig (1):
  block: remove QUEUE_FLAG_STOPPED

John Garry (10):
  block: Make QUEUE_FLAG_x as an enum
  block: Add build-time assert for size of blk_queue_flag_name[]
  block: Catch possible entries missing from hctx_state_name[]
  block: Catch possible entries missing from hctx_flag_name[]
  block: Catch possible entries missing from alloc_policy_name[]
  block: Add missing entries from cmd_flag_name[]
  block: Catch possible entries missing from cmd_flag_name[]
  block: Make RQF_x as an enum
  block: Add zone write plugging entry to rqf_name[]
  block: Catch possible entries missing from rqf_name[]

 block/blk-mq-debugfs.c    | 20 +++++++--
 include/linux/blk-mq.h    | 88 +++++++++++++++++++++++----------------
 include/linux/blk_types.h |  1 +
 include/linux/blkdev.h    | 31 +++++++-------
 4 files changed, 86 insertions(+), 54 deletions(-)

-- 
2.31.1

[PATCH 01/11] block: remove QUEUE_FLAG_STOPPED

[John Garry]

From: Christoph Hellwig <hch@lst.de>

QUEUE_FLAG_STOPPED is entirely unused.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: John Garry <john.g.garry@oracke.com>
---
 block/blk-mq-debugfs.c | 1 -
 include/linux/blkdev.h | 2 --
 2 files changed, 3 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 344f9e503bdb..03d0409e5018 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -79,7 +79,6 @@ static int queue_pm_only_show(void *data, struct seq_file *m)
 
 #define QUEUE_FLAG_NAME(name) [QUEUE_FLAG_##name] = #name
 static const char *const blk_queue_flag_name[] = {
-	QUEUE_FLAG_NAME(STOPPED),
 	QUEUE_FLAG_NAME(DYING),
 	QUEUE_FLAG_NAME(NOMERGES),
 	QUEUE_FLAG_NAME(SAME_COMP),
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index dce4a6bf7307..942ad4e0f231 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -588,7 +588,6 @@ struct request_queue {
 };
 
 /* Keep blk_queue_flag_name[] in sync with the definitions below */
-#define QUEUE_FLAG_STOPPED	0	/* queue is stopped */
 #define QUEUE_FLAG_DYING	1	/* queue being torn down */
 #define QUEUE_FLAG_NOMERGES     3	/* disable merge attempts */
 #define QUEUE_FLAG_SAME_COMP	4	/* complete on same CPU-group */
@@ -608,7 +607,6 @@ struct request_queue {
 void blk_queue_flag_set(unsigned int flag, struct request_queue *q);
 void blk_queue_flag_clear(unsigned int flag, struct request_queue *q);
 
-#define blk_queue_stopped(q)	test_bit(QUEUE_FLAG_STOPPED, &(q)->queue_flags)
 #define blk_queue_dying(q)	test_bit(QUEUE_FLAG_DYING, &(q)->queue_flags)
 #define blk_queue_init_done(q)	test_bit(QUEUE_FLAG_INIT_DONE, &(q)->queue_flags)
 #define blk_queue_nomerges(q)	test_bit(QUEUE_FLAG_NOMERGES, &(q)->queue_flags)
-- 
2.31.1

[PATCH 02/11] block: Make QUEUE_FLAG_x as an enum

[John Garry]

This will allow us better keep in sync with blk_queue_flag_name[].

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 include/linux/blkdev.h | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 942ad4e0f231..bb521745c702 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -588,19 +588,22 @@ struct request_queue {
 };
 
 /* Keep blk_queue_flag_name[] in sync with the definitions below */
-#define QUEUE_FLAG_DYING	1	/* queue being torn down */
-#define QUEUE_FLAG_NOMERGES     3	/* disable merge attempts */
-#define QUEUE_FLAG_SAME_COMP	4	/* complete on same CPU-group */
-#define QUEUE_FLAG_FAIL_IO	5	/* fake timeout */
-#define QUEUE_FLAG_NOXMERGES	9	/* No extended merges */
-#define QUEUE_FLAG_SAME_FORCE	12	/* force complete on same CPU */
-#define QUEUE_FLAG_INIT_DONE	14	/* queue is initialized */
-#define QUEUE_FLAG_STATS	20	/* track IO start and completion times */
-#define QUEUE_FLAG_REGISTERED	22	/* queue has been registered to a disk */
-#define QUEUE_FLAG_QUIESCED	24	/* queue has been quiesced */
-#define QUEUE_FLAG_RQ_ALLOC_TIME 27	/* record rq->alloc_time_ns */
-#define QUEUE_FLAG_HCTX_ACTIVE	28	/* at least one blk-mq hctx is active */
-#define QUEUE_FLAG_SQ_SCHED     30	/* single queue style io dispatch */
+enum {
+	QUEUE_FLAG_DYING		= 0,	/* queue being torn down */
+	QUEUE_FLAG_NOMERGES,			/* disable merge attempts */
+	QUEUE_FLAG_SAME_COMP,			/* complete on same CPU-group */
+	QUEUE_FLAG_FAIL_IO,			/* fake timeout */
+	QUEUE_FLAG_NOXMERGES,			/* No extended merges */
+	QUEUE_FLAG_SAME_FORCE,			/* force complete on same CPU */
+	QUEUE_FLAG_INIT_DONE,			/* queue is initialized */
+	QUEUE_FLAG_STATS,			/* track IO start and completion times */
+	QUEUE_FLAG_REGISTERED,			/* queue has been registered to a disk */
+	QUEUE_FLAG_QUIESCED,			/* queue has been quiesced */
+	QUEUE_FLAG_RQ_ALLOC_TIME,		/* record rq->alloc_time_ns */
+	QUEUE_FLAG_HCTX_ACTIVE,			/* at least one blk-mq hctx is active */
+	QUEUE_FLAG_SQ_SCHED,			/* single queue style io dispatch */
+	QUEUE_FLAG_MAX
+};
 
 #define QUEUE_FLAG_MQ_DEFAULT	(1UL << QUEUE_FLAG_SAME_COMP)
 
-- 
2.31.1

[PATCH 03/11] block: Add build-time assert for size of blk_queue_flag_name[]

[John Garry]

Assert that we are not missing flag entries in blk_queue_flag_name[].

However, we are not catching mis-ordered flags. This should not be
necessary.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 03d0409e5018..9e18ba6b1c4d 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -5,6 +5,7 @@
 
 #include <linux/kernel.h>
 #include <linux/blkdev.h>
+#include <linux/build_bug.h>
 #include <linux/debugfs.h>
 
 #include "blk.h"
@@ -99,6 +100,7 @@ static int queue_state_show(void *data, struct seq_file *m)
 {
 	struct request_queue *q = data;
 
+	BUILD_BUG_ON(ARRAY_SIZE(blk_queue_flag_name) != QUEUE_FLAG_MAX);
 	blk_flags_show(m, q->queue_flags, blk_queue_flag_name,
 		       ARRAY_SIZE(blk_queue_flag_name));
 	seq_puts(m, "\n");
-- 
2.31.1

[PATCH 04/11] block: Catch possible entries missing from hctx_state_name[]

[John Garry]

Add a build-time assert that we are not missing entries from
hctx_state_name[]. For this, add a "max" entry for BLK_MQ_S_x flags.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c | 1 +
 include/linux/blk-mq.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 9e18ba6b1c4d..fca8b82464b4 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -165,6 +165,7 @@ static int hctx_state_show(void *data, struct seq_file *m)
 {
 	struct blk_mq_hw_ctx *hctx = data;
 
+	BUILD_BUG_ON(ARRAY_SIZE(hctx_state_name) != BLK_MQ_S_MAX);
 	blk_flags_show(m, hctx->state, hctx_state_name,
 		       ARRAY_SIZE(hctx_state_name));
 	seq_puts(m, "\n");
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 89ba6b16fe8b..225e51698470 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -664,12 +664,14 @@ enum {
 	BLK_MQ_F_ALLOC_POLICY_START_BIT = 8,
 	BLK_MQ_F_ALLOC_POLICY_BITS = 1,
 
+	/* Keep hctx_state_name[] in sync with the definitions below */
 	BLK_MQ_S_STOPPED	= 0,
 	BLK_MQ_S_TAG_ACTIVE	= 1,
 	BLK_MQ_S_SCHED_RESTART	= 2,
 
 	/* hw queue is inactive after all its CPUs become offline */
 	BLK_MQ_S_INACTIVE	= 3,
+	BLK_MQ_S_MAX,
 
 	BLK_MQ_MAX_DEPTH	= 10240,
 
-- 
2.31.1

[PATCH 05/11] block: Catch possible entries missing from hctx_flag_name[]

[John Garry]

Re-arrange members in hctx_flag_name[] to match the enum in which the flags
are declared.

Also add a BUILD_BUG_ON() call to ensure that we are not missing entries
in hctx_flag_name[].

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
BLK_MQ_F_MAX is a bit odd, as it is a bit index and not a flag.
 block/blk-mq-debugfs.c | 6 ++++--
 include/linux/blk-mq.h | 8 ++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index fca8b82464b4..74f470d0e1ea 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -183,10 +183,10 @@ static const char *const alloc_policy_name[] = {
 static const char *const hctx_flag_name[] = {
 	HCTX_FLAG_NAME(SHOULD_MERGE),
 	HCTX_FLAG_NAME(TAG_QUEUE_SHARED),
-	HCTX_FLAG_NAME(BLOCKING),
-	HCTX_FLAG_NAME(NO_SCHED),
 	HCTX_FLAG_NAME(STACKING),
 	HCTX_FLAG_NAME(TAG_HCTX_SHARED),
+	HCTX_FLAG_NAME(BLOCKING),
+	HCTX_FLAG_NAME(NO_SCHED),
 };
 #undef HCTX_FLAG_NAME
 
@@ -195,6 +195,8 @@ 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_MAX);
+
 	seq_puts(m, "alloc_policy=");
 	if (alloc_policy < ARRAY_SIZE(alloc_policy_name) &&
 	    alloc_policy_name[alloc_policy])
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 225e51698470..b3905b77f375 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -644,6 +644,7 @@ struct blk_mq_ops {
 #endif
 };
 
+/* Keep hctx_flag_name[] in sync with the definitions below */
 enum {
 	BLK_MQ_F_SHOULD_MERGE	= 1 << 0,
 	BLK_MQ_F_TAG_QUEUE_SHARED = 1 << 1,
@@ -653,9 +654,12 @@ enum {
 	 */
 	BLK_MQ_F_STACKING	= 1 << 2,
 	BLK_MQ_F_TAG_HCTX_SHARED = 1 << 3,
-	BLK_MQ_F_BLOCKING	= 1 << 5,
+	BLK_MQ_F_BLOCKING	= 1 << 4,
 	/* Do not allow an I/O scheduler to be configured. */
-	BLK_MQ_F_NO_SCHED	= 1 << 6,
+	BLK_MQ_F_NO_SCHED	= 1 << 5,
+
+	BLK_MQ_F_MAX		= 6,
+
 	/*
 	 * Select 'none' during queue registration in case of a single hwq
 	 * or shared hwqs instead of 'mq-deadline'.
-- 
2.31.1

[PATCH 06/11] block: Catch possible entries missing from alloc_policy_name[]

[John Garry]

Make BLK_TAG_ALLOC_x an enum and add a "max" entry.

Also add a BUILD_BUG_ON() call to ensure that we are not missing entries
in hctx_flag_name[].

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c |  1 +
 include/linux/blk-mq.h | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 74f470d0e1ea..e37d5a2dd942 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -196,6 +196,7 @@ static int hctx_flags_show(void *data, struct seq_file *m)
 	const int alloc_policy = BLK_MQ_FLAG_TO_ALLOC_POLICY(hctx->flags);
 
 	BUILD_BUG_ON(ARRAY_SIZE(hctx_flag_name) != BLK_MQ_F_MAX);
+	BUILD_BUG_ON(ARRAY_SIZE(alloc_policy_name) != BLK_TAG_ALLOC_MAX);
 
 	seq_puts(m, "alloc_policy=");
 	if (alloc_policy < ARRAY_SIZE(alloc_policy_name) &&
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index b3905b77f375..aefbf93d431a 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -278,8 +278,14 @@ enum blk_eh_timer_return {
 	BLK_EH_RESET_TIMER,
 };
 
-#define BLK_TAG_ALLOC_FIFO 0 /* allocate starting from 0 */
-#define BLK_TAG_ALLOC_RR 1 /* allocate starting from last allocated tag */
+/* Keep alloc_policy_name[] in sync with the definitions below */
+enum {
+	/* allocate starting from 0 */
+	BLK_TAG_ALLOC_FIFO	= 0,
+	/* allocate starting from last allocated tag */
+	BLK_TAG_ALLOC_RR,
+	BLK_TAG_ALLOC_MAX
+};
 
 /**
  * struct blk_mq_hw_ctx - State for a hardware queue facing the hardware
-- 
2.31.1

[PATCH 07/11] block: Add missing entries from cmd_flag_name[]

[John Garry]

Add missing entries for req_flag_bits.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index e37d5a2dd942..62ad3ddf0175 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -230,6 +230,11 @@ static const char *const cmd_flag_name[] = {
 	CMD_FLAG_NAME(NOWAIT),
 	CMD_FLAG_NAME(NOUNMAP),
 	CMD_FLAG_NAME(POLLED),
+	CMD_FLAG_NAME(ALLOC_CACHE),
+	CMD_FLAG_NAME(SWAP),
+	CMD_FLAG_NAME(DRV),
+	CMD_FLAG_NAME(FS_PRIVATE),
+	CMD_FLAG_NAME(NOUNMAP),
 };
 #undef CMD_FLAG_NAME
 
-- 
2.31.1

Re: [PATCH 07/11] block: Add missing entries from cmd_flag_name[]

[kernel test robot]

Hi John,

kernel test robot noticed the following build warnings:

[auto build test WARNING on axboe-block/for-next]
[also build test WARNING on next-20240709]
[cannot apply to hch-configfs/for-next linus/master v6.10-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Garry/block-remove-QUEUE_FLAG_STOPPED/20240709-191356
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
patch link:    https://lore.kernel.org/r/20240709110538.532896-8-john.g.garry%40oracle.com
patch subject: [PATCH 07/11] block: Add missing entries from cmd_flag_name[]
config: x86_64-randconfig-r132-20240710 (https://download.01.org/0day-ci/archive/20240710/202407100611.wrfknWDf-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240710/202407100611.wrfknWDf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202407100611.wrfknWDf-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> block/blk-mq-debugfs.c:231:9: sparse: sparse: Initializer entry defined twice
   block/blk-mq-debugfs.c:237:9: sparse:   also defined here

vim +231 block/blk-mq-debugfs.c

9abb2ad21e8b9b7 Omar Sandoval     2017-01-25  214  
1a435111f8eb30b Omar Sandoval     2017-05-04  215  #define CMD_FLAG_NAME(name) [__REQ_##name] = #name
8658dca8bd5666f Bart Van Assche   2017-04-26  216  static const char *const cmd_flag_name[] = {
1a435111f8eb30b Omar Sandoval     2017-05-04  217  	CMD_FLAG_NAME(FAILFAST_DEV),
1a435111f8eb30b Omar Sandoval     2017-05-04  218  	CMD_FLAG_NAME(FAILFAST_TRANSPORT),
1a435111f8eb30b Omar Sandoval     2017-05-04  219  	CMD_FLAG_NAME(FAILFAST_DRIVER),
1a435111f8eb30b Omar Sandoval     2017-05-04  220  	CMD_FLAG_NAME(SYNC),
1a435111f8eb30b Omar Sandoval     2017-05-04  221  	CMD_FLAG_NAME(META),
1a435111f8eb30b Omar Sandoval     2017-05-04  222  	CMD_FLAG_NAME(PRIO),
1a435111f8eb30b Omar Sandoval     2017-05-04  223  	CMD_FLAG_NAME(NOMERGE),
1a435111f8eb30b Omar Sandoval     2017-05-04  224  	CMD_FLAG_NAME(IDLE),
1a435111f8eb30b Omar Sandoval     2017-05-04  225  	CMD_FLAG_NAME(INTEGRITY),
1a435111f8eb30b Omar Sandoval     2017-05-04  226  	CMD_FLAG_NAME(FUA),
1a435111f8eb30b Omar Sandoval     2017-05-04  227  	CMD_FLAG_NAME(PREFLUSH),
1a435111f8eb30b Omar Sandoval     2017-05-04  228  	CMD_FLAG_NAME(RAHEAD),
1a435111f8eb30b Omar Sandoval     2017-05-04  229  	CMD_FLAG_NAME(BACKGROUND),
22d538213ec4fa6 Bart Van Assche   2017-08-18  230  	CMD_FLAG_NAME(NOWAIT),
1c26010c5e1b9ad Jianchao Wang     2019-01-24 @231  	CMD_FLAG_NAME(NOUNMAP),
6ce913fe3eee14f Christoph Hellwig 2021-10-12  232  	CMD_FLAG_NAME(POLLED),
6c56c597270e732 John Garry        2024-07-09  233  	CMD_FLAG_NAME(ALLOC_CACHE),
6c56c597270e732 John Garry        2024-07-09  234  	CMD_FLAG_NAME(SWAP),
6c56c597270e732 John Garry        2024-07-09  235  	CMD_FLAG_NAME(DRV),
6c56c597270e732 John Garry        2024-07-09  236  	CMD_FLAG_NAME(FS_PRIVATE),
6c56c597270e732 John Garry        2024-07-09  237  	CMD_FLAG_NAME(NOUNMAP),
8658dca8bd5666f Bart Van Assche   2017-04-26  238  };
1a435111f8eb30b Omar Sandoval     2017-05-04  239  #undef CMD_FLAG_NAME
8658dca8bd5666f Bart Van Assche   2017-04-26  240  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 08/11] block: Catch possible entries missing from cmd_flag_name[]

[John Garry]

Also add a BUILD_BUG_ON() call to ensure that we are not missing entries
in cmd_flag_name[].

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c    | 2 ++
 include/linux/blk_types.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 62ad3ddf0175..f764b86941c3 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -278,6 +278,8 @@ int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq)
 	const enum req_op op = req_op(rq);
 	const char *op_str = blk_op_str(op);
 
+	BUILD_BUG_ON(ARRAY_SIZE(cmd_flag_name) != __REQ_NR_BITS);
+
 	seq_printf(m, "%p {.op=", rq);
 	if (strcmp(op_str, "UNKNOWN") == 0)
 		seq_printf(m, "%u", op);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 632edd71f8c6..36ed96133217 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -354,6 +354,7 @@ enum req_op {
 	REQ_OP_LAST		= (__force blk_opf_t)36,
 };
 
+/* Keep cmd_flag_name[] in sync with the definitions below */
 enum req_flag_bits {
 	__REQ_FAILFAST_DEV =	/* no driver retries of device errors */
 		REQ_OP_BITS,
-- 
2.31.1

[PATCH 09/11] block: Make RQF_x as an enum

[John Garry]

Make RQF_x as an enum to better order and number members.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
Maybe we should also have an enum for bits, like:

RQF_STARTED_BIT = 0
...
RQF_STARTED = (1 << RQF_STARTED_BIT)

as RQF_MAX looks out of place in later patch
 include/linux/blk-mq.h | 66 ++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index aefbf93d431a..f3de4a0b5293 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -27,38 +27,40 @@ typedef enum rq_end_io_ret (rq_end_io_fn)(struct request *, blk_status_t);
  * request flags */
 typedef __u32 __bitwise req_flags_t;
 
-/* drive already may have started this one */
-#define RQF_STARTED		((__force req_flags_t)(1 << 1))
-/* request for flush sequence */
-#define RQF_FLUSH_SEQ		((__force req_flags_t)(1 << 4))
-/* merge of different types, fail separately */
-#define RQF_MIXED_MERGE		((__force req_flags_t)(1 << 5))
-/* don't call prep for this one */
-#define RQF_DONTPREP		((__force req_flags_t)(1 << 7))
-/* use hctx->sched_tags */
-#define RQF_SCHED_TAGS		((__force req_flags_t)(1 << 8))
-/* use an I/O scheduler for this request */
-#define RQF_USE_SCHED		((__force req_flags_t)(1 << 9))
-/* vaguely specified driver internal error.  Ignored by the block layer */
-#define RQF_FAILED		((__force req_flags_t)(1 << 10))
-/* don't warn about errors */
-#define RQF_QUIET		((__force req_flags_t)(1 << 11))
-/* account into disk and partition IO statistics */
-#define RQF_IO_STAT		((__force req_flags_t)(1 << 13))
-/* runtime pm request */
-#define RQF_PM			((__force req_flags_t)(1 << 15))
-/* on IO scheduler merge hash */
-#define RQF_HASHED		((__force req_flags_t)(1 << 16))
-/* track IO completion time */
-#define RQF_STATS		((__force req_flags_t)(1 << 17))
-/* Look at ->special_vec for the actual data payload instead of the
-   bio chain. */
-#define RQF_SPECIAL_PAYLOAD	((__force req_flags_t)(1 << 18))
-/* The request completion needs to be signaled to zone write pluging. */
-#define RQF_ZONE_WRITE_PLUGGING	((__force req_flags_t)(1 << 20))
-/* ->timeout has been called, don't expire again */
-#define RQF_TIMED_OUT		((__force req_flags_t)(1 << 21))
-#define RQF_RESV		((__force req_flags_t)(1 << 23))
+enum {
+	/* drive already may have started this one */
+	RQF_STARTED		=	((__force req_flags_t)(1 << 0)),
+	/* request for flush sequence */
+	RQF_FLUSH_SEQ		=	((__force req_flags_t)(1 << 1)),
+	/* merge of different types, fail separately */
+	RQF_MIXED_MERGE		=	((__force req_flags_t)(1 << 2)),
+	/* don't call prep for this one */
+	RQF_DONTPREP		=	((__force req_flags_t)(1 << 3)),
+	/* use hctx->sched_tags */
+	RQF_SCHED_TAGS		=	((__force req_flags_t)(1 << 4)),
+	/* use an I/O scheduler for this request */
+	RQF_USE_SCHED		=	((__force req_flags_t)(1 << 5)),
+	/* vaguely specified driver internal error.  Ignored by the block layer */
+	RQF_FAILED		=	((__force req_flags_t)(1 << 6)),
+	/* don't warn about errors */
+	RQF_QUIET		=	((__force req_flags_t)(1 << 7)),
+	/* account into disk and partition IO statistics */
+	RQF_IO_STAT		=	((__force req_flags_t)(1 << 8)),
+	/* runtime pm request */
+	RQF_PM			=	((__force req_flags_t)(1 << 9)),
+	/* on IO scheduler merge hash */
+	RQF_HASHED		=	((__force req_flags_t)(1 << 10)),
+	/* track IO completion time */
+	RQF_STATS		=	((__force req_flags_t)(1 << 11)),
+	/* Look at ->special_vec for the actual data payload instead of the
+	   bio chain. */
+	RQF_SPECIAL_PAYLOAD	=	((__force req_flags_t)(1 << 12)),
+	/* The request completion needs to be signaled to zone write pluging. */
+	RQF_ZONE_WRITE_PLUGGING	=	((__force req_flags_t)(1 << 13)),
+	/* ->timeout has been called, don't expire again */
+	RQF_TIMED_OUT		=	((__force req_flags_t)(1 << 14)),
+	RQF_RESV		=	((__force req_flags_t)(1 << 15)),
+};
 
 /* flags that prevent us from merging requests: */
 #define RQF_NOMERGE_FLAGS \
-- 
2.31.1

Re: [PATCH 09/11] block: Make RQF_x as an enum

[Christoph Hellwig]

> +enum {
> +	/* drive already may have started this one */
> +	RQF_STARTED		=	((__force req_flags_t)(1 << 0)),

Last time I tried to mix __bitwise and enums sparse was very unhappy.
Did this get fixed?

Re: [PATCH 09/11] block: Make RQF_x as an enum

[John Garry]

On 09/07/2024 12:23, Christoph Hellwig wrote:
>> +enum {
>> +	/* drive already may have started this one */
>> +	RQF_STARTED		=	((__force req_flags_t)(1 << 0)),
> 
> Last time I tried to mix __bitwise and enums sparse was very unhappy.
> Did this get fixed?

For me, sparse only complains about RQF_MAX, which is added later.

But, as I noted, I think that needs to change to a bit count from a flag 
anyway. I am not sure how that would look, maybe:

enum {

	RQF_STARTED_BIT = 0,

	...
	RQF_MAX_BITS

};

and then use a macro to define RQF_STARTED as ((__force 
req_flags_t)(RQF_STARTED_BIT  << 0))

But do you remember how you generated the sparse warning specifically?

> 

[PATCH 10/11] block: Add zone write plugging entry to rqf_name[]

[John Garry]

Add missing entry.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index f764b86941c3..97741996b5f2 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -253,6 +253,7 @@ static const char *const rqf_name[] = {
 	RQF_NAME(HASHED),
 	RQF_NAME(STATS),
 	RQF_NAME(SPECIAL_PAYLOAD),
+	RQF_NAME(ZONE_WRITE_PLUGGING),
 	RQF_NAME(TIMED_OUT),
 	RQF_NAME(RESV),
 };
-- 
2.31.1

[PATCH 11/11] block: Catch possible entries missing from rqf_name[]

[John Garry]

Also add a BUILD_BUG_ON() call to ensure that we are not missing entries
in rqf_name[].

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-mq-debugfs.c | 1 +
 include/linux/blk-mq.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 97741996b5f2..c829406579ee 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -280,6 +280,7 @@ int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq)
 	const char *op_str = blk_op_str(op);
 
 	BUILD_BUG_ON(ARRAY_SIZE(cmd_flag_name) != __REQ_NR_BITS);
+	BUILD_BUG_ON(ARRAY_SIZE(rqf_name) != RQF_MAX);
 
 	seq_printf(m, "%p {.op=", rq);
 	if (strcmp(op_str, "UNKNOWN") == 0)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index f3de4a0b5293..928674c026b3 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -27,6 +27,7 @@ typedef enum rq_end_io_ret (rq_end_io_fn)(struct request *, blk_status_t);
  * request flags */
 typedef __u32 __bitwise req_flags_t;
 
+/* Keep rqf_name[] in sync with the definitions below */
 enum {
 	/* drive already may have started this one */
 	RQF_STARTED		=	((__force req_flags_t)(1 << 0)),
@@ -60,6 +61,7 @@ enum {
 	/* ->timeout has been called, don't expire again */
 	RQF_TIMED_OUT		=	((__force req_flags_t)(1 << 14)),
 	RQF_RESV		=	((__force req_flags_t)(1 << 15)),
+	RQF_MAX			=	16
 };
 
 /* flags that prevent us from merging requests: */
-- 
2.31.1