[PATCH] block/mq-deadline: Replace DD_PRIO_MAX with DD_PRIO_COUNT

[PATCH] block/mq-deadline: Replace DD_PRIO_MAX with DD_PRIO_COUNT

[chengkaitao]

From: chengkaitao <chengkaitao@kylinos.cn>

Remove redundant DD_PRIO_MAX and enum types, Move DD_PRIO_COUNT
into enum dd_prio{}, and similarly for DD_DIR_COUNT.

Signed-off-by: chengkaitao <chengkaitao@kylinos.cn>
---
 block/mq-deadline.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index b9b7cdf1d3c9..1a031922c447 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -41,19 +41,16 @@ static const int fifo_batch = 16;       /* # of sequential requests treated as o
 enum dd_data_dir {
 	DD_READ		= READ,
 	DD_WRITE	= WRITE,
+	DD_DIR_COUNT	= 2
 };
 
-enum { DD_DIR_COUNT = 2 };
-
 enum dd_prio {
-	DD_RT_PRIO	= 0,
-	DD_BE_PRIO	= 1,
-	DD_IDLE_PRIO	= 2,
-	DD_PRIO_MAX	= 2,
+	DD_RT_PRIO,
+	DD_BE_PRIO,
+	DD_IDLE_PRIO,
+	DD_PRIO_COUNT
 };
 
-enum { DD_PRIO_COUNT = 3 };
-
 /*
  * I/O statistics per I/O priority. It is fine if these counters overflow.
  * What matters is that these counters are at least as wide as
@@ -441,7 +438,7 @@ static struct request *dd_dispatch_prio_aged_requests(struct deadline_data *dd,
 	if (prio_cnt < 2)
 		return NULL;
 
-	for (prio = DD_BE_PRIO; prio <= DD_PRIO_MAX; prio++) {
+	for (prio = DD_BE_PRIO; prio < DD_PRIO_COUNT; prio++) {
 		rq = __dd_dispatch_request(dd, &dd->per_prio[prio],
 					   now - dd->prio_aging_expire);
 		if (rq)
@@ -475,7 +472,7 @@ static struct request *dd_dispatch_request(struct blk_mq_hw_ctx *hctx)
 	 * Next, dispatch requests in priority order. Ignore lower priority
 	 * requests if any higher priority requests are pending.
 	 */
-	for (prio = 0; prio <= DD_PRIO_MAX; prio++) {
+	for (prio = 0; prio < DD_PRIO_COUNT; prio++) {
 		rq = __dd_dispatch_request(dd, &dd->per_prio[prio], now);
 		if (rq || dd_queued(dd, prio))
 			break;
@@ -530,7 +527,7 @@ static void dd_exit_sched(struct elevator_queue *e)
 	struct deadline_data *dd = e->elevator_data;
 	enum dd_prio prio;
 
-	for (prio = 0; prio <= DD_PRIO_MAX; prio++) {
+	for (prio = 0; prio < DD_PRIO_COUNT; prio++) {
 		struct dd_per_prio *per_prio = &dd->per_prio[prio];
 		const struct io_stats_per_prio *stats = &per_prio->stats;
 		uint32_t queued;
@@ -565,7 +562,7 @@ static int dd_init_sched(struct request_queue *q, struct elevator_queue *eq)
 
 	eq->elevator_data = dd;
 
-	for (prio = 0; prio <= DD_PRIO_MAX; prio++) {
+	for (prio = 0; prio < DD_PRIO_COUNT; prio++) {
 		struct dd_per_prio *per_prio = &dd->per_prio[prio];
 
 		INIT_LIST_HEAD(&per_prio->dispatch);
@@ -748,7 +745,7 @@ static bool dd_has_work(struct blk_mq_hw_ctx *hctx)
 	struct deadline_data *dd = hctx->queue->elevator->elevator_data;
 	enum dd_prio prio;
 
-	for (prio = 0; prio <= DD_PRIO_MAX; prio++)
+	for (prio = 0; prio < DD_PRIO_COUNT; prio++)
 		if (dd_has_work_for_prio(&dd->per_prio[prio]))
 			return true;
 
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] block/mq-deadline: Replace DD_PRIO_MAX with DD_PRIO_COUNT

[Bart Van Assche]

On 8/28/25 6:54 PM, chengkaitao wrote:
> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> index b9b7cdf1d3c9..1a031922c447 100644
> --- a/block/mq-deadline.c
> +++ b/block/mq-deadline.c
> @@ -41,19 +41,16 @@ static const int fifo_batch = 16;       /* # of sequential requests treated as o
>   enum dd_data_dir {
>   	DD_READ		= READ,
>   	DD_WRITE	= WRITE,
> +	DD_DIR_COUNT	= 2
>   };
>   
> -enum { DD_DIR_COUNT = 2 };
> -

This change is not an improvement in my opinion because it makes it
less clear what the role of DD_DIR_COUNT is.

>   enum dd_prio {
> -	DD_RT_PRIO	= 0,
> -	DD_BE_PRIO	= 1,
> -	DD_IDLE_PRIO	= 2,
> -	DD_PRIO_MAX	= 2,
> +	DD_RT_PRIO,
> +	DD_BE_PRIO,
> +	DD_IDLE_PRIO,

There is code that depends on DD_RT_PRIO < DD_BE_PRIO < DD_IDLE_PRIO so 
I'd like to keep the explicit enum values.

> +	DD_PRIO_COUNT
>   };
>   
> -enum { DD_PRIO_COUNT = 3 };

I see the above change as a step backwards because it makes the role of
DD_PRIO_COUNT less clear.

> -	for (prio = DD_BE_PRIO; prio <= DD_PRIO_MAX; prio++) {
> +	for (prio = DD_BE_PRIO; prio < DD_PRIO_COUNT; prio++) {

The current code is easier to read IMHO than the new code.

Thanks,

Bart.

Re: [PATCH] block/mq-deadline: Replace DD_PRIO_MAX with DD_PRIO_COUNT

[Tao pilgrim]

On Fri, Aug 29, 2025 at 12:38 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 8/28/25 6:54 PM, chengkaitao wrote:
> > diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> > index b9b7cdf1d3c9..1a031922c447 100644
> > --- a/block/mq-deadline.c
> > +++ b/block/mq-deadline.c
> > @@ -41,19 +41,16 @@ static const int fifo_batch = 16;       /* # of sequential requests treated as o
> >   enum dd_data_dir {
> >       DD_READ         = READ,
> >       DD_WRITE        = WRITE,
> > +     DD_DIR_COUNT    = 2
> >   };
> >
> > -enum { DD_DIR_COUNT = 2 };
> > -
>
> This change is not an improvement in my opinion because it makes it
> less clear what the role of DD_DIR_COUNT is.

DD_DIR_COUNT is used to count the number of members in the
enum dd_data_dir{}, and should be placed inside the enum dd_data_dir.
There are many such examples in the kernel, such as:
KFENCE_COUNTER_COUNT,__MTHP_STAT_COUNT,CHANNELMSG_COUNT,
ETH_RSS_HASH_FUNCS_COUNT,DEPOT_COUNTER_COUNT....
>
> >   enum dd_prio {
> > -     DD_RT_PRIO      = 0,
> > -     DD_BE_PRIO      = 1,
> > -     DD_IDLE_PRIO    = 2,
> > -     DD_PRIO_MAX     = 2,
> > +     DD_RT_PRIO,
> > +     DD_BE_PRIO,
> > +     DD_IDLE_PRIO,
>
> There is code that depends on DD_RT_PRIO < DD_BE_PRIO < DD_IDLE_PRIO so
> I'd like to keep the explicit enum values.

Wouldn't it be better to simply add a comment for this purpose?
>
> > +     DD_PRIO_COUNT
> >   };
> >
> > -enum { DD_PRIO_COUNT = 3 };
>
> I see the above change as a step backwards because it makes the role of
> DD_PRIO_COUNT less clear.

Defining DD_PRIO_COUNT within the enum dd_prio{} clearly indicates
that DD_PRIO_COUNT serves solely for the enum dd_prio{}. If a new
member is added to enum dd_prio{} in the future, DD_PRIO_COUNT
would not need to be modified separately.
>
> > -     for (prio = DD_BE_PRIO; prio <= DD_PRIO_MAX; prio++) {
> > +     for (prio = DD_BE_PRIO; prio < DD_PRIO_COUNT; prio++) {
>
> The current code is easier to read IMHO than the new code.

I believe programmers are intelligent enough to understand the meanings
of *_COUNT and *_MAX without needing to define them independently,
especially in cases like *_COUNT = *_MAX + 1, where _MAX seems rather
redundant.
>
The above are my personal thoughts. The purpose of this patch is to make
the code more concise, and not merging it into the mainline won't have any
significant impact.

Looking forward to your reply.
-- 
Yours,
Kaitao Cheng