2019-01-24 10:30:29

by jianchao.wang

[permalink] [raw]
Subject: [PATCH] blk-mq: fix the cmd_flag_name array

Swap REQ_NOWAIT and REQ_NOUNMAP and add REQ_HIPRI.

Signed-off-by: Jianchao Wang <[email protected]>
---
block/blk-mq-debugfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 90d6876..f812083 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -308,8 +308,9 @@ static const char *const cmd_flag_name[] = {
CMD_FLAG_NAME(PREFLUSH),
CMD_FLAG_NAME(RAHEAD),
CMD_FLAG_NAME(BACKGROUND),
- CMD_FLAG_NAME(NOUNMAP),
CMD_FLAG_NAME(NOWAIT),
+ CMD_FLAG_NAME(NOUNMAP),
+ CMD_FLAG_NAME(HIPRI),
};
#undef CMD_FLAG_NAME

--
2.7.4



2019-01-24 15:09:54

by Jeff Moyer

[permalink] [raw]
Subject: Re: [PATCH] blk-mq: fix the cmd_flag_name array

Jianchao Wang <[email protected]> writes:

> Swap REQ_NOWAIT and REQ_NOUNMAP and add REQ_HIPRI.
>
> Signed-off-by: Jianchao Wang <[email protected]>
> ---
> block/blk-mq-debugfs.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 90d6876..f812083 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -308,8 +308,9 @@ static const char *const cmd_flag_name[] = {
> CMD_FLAG_NAME(PREFLUSH),
> CMD_FLAG_NAME(RAHEAD),
> CMD_FLAG_NAME(BACKGROUND),
> - CMD_FLAG_NAME(NOUNMAP),
> CMD_FLAG_NAME(NOWAIT),
> + CMD_FLAG_NAME(NOUNMAP),
> + CMD_FLAG_NAME(HIPRI),
> };
> #undef CMD_FLAG_NAME

Acked-by: Jeff Moyer <[email protected]>

You might consider also adding a comment above the req_flag_bits enum
noting that modifications also need to be propagated to cmd_flag_name.

2019-01-24 16:22:00

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] blk-mq: fix the cmd_flag_name array

On 1/24/19 3:28 AM, Jianchao Wang wrote:
> Swap REQ_NOWAIT and REQ_NOUNMAP and add REQ_HIPRI.

Applied, thanks.

--
Jens Axboe


2019-01-24 16:22:42

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] blk-mq: fix the cmd_flag_name array

On 1/24/19 8:09 AM, Jeff Moyer wrote:
> Jianchao Wang <[email protected]> writes:
>
>> Swap REQ_NOWAIT and REQ_NOUNMAP and add REQ_HIPRI.
>>
>> Signed-off-by: Jianchao Wang <[email protected]>
>> ---
>> block/blk-mq-debugfs.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
>> index 90d6876..f812083 100644
>> --- a/block/blk-mq-debugfs.c
>> +++ b/block/blk-mq-debugfs.c
>> @@ -308,8 +308,9 @@ static const char *const cmd_flag_name[] = {
>> CMD_FLAG_NAME(PREFLUSH),
>> CMD_FLAG_NAME(RAHEAD),
>> CMD_FLAG_NAME(BACKGROUND),
>> - CMD_FLAG_NAME(NOUNMAP),
>> CMD_FLAG_NAME(NOWAIT),
>> + CMD_FLAG_NAME(NOUNMAP),
>> + CMD_FLAG_NAME(HIPRI),
>> };
>> #undef CMD_FLAG_NAME
>
> Acked-by: Jeff Moyer <[email protected]>
>
> You might consider also adding a comment above the req_flag_bits enum
> noting that modifications also need to be propagated to cmd_flag_name.

Agree... These things are notoriously difficult to keep in sync, this
isn't the first time we've had a fixup. Not sure if that situation
is improvable, but at least a comment might help.

--
Jens Axboe


2019-01-24 16:29:41

by Bart Van Assche

[permalink] [raw]
Subject: Re: [PATCH] blk-mq: fix the cmd_flag_name array

On Thu, 2019-01-24 at 09:22 -0700, Jens Axboe wrote:
+AD4 On 1/24/19 8:09 AM, Jeff Moyer wrote:
+AD4 +AD4 Jianchao Wang +ADw-jianchao.w.wang+AEA-oracle.com+AD4 writes:
+AD4 +AD4
+AD4 +AD4 +AD4 Swap REQ+AF8-NOWAIT and REQ+AF8-NOUNMAP and add REQ+AF8-HIPRI.
+AD4 +AD4 +AD4
+AD4 +AD4 +AD4 Signed-off-by: Jianchao Wang +ADw-jianchao.w.wang+AEA-oracle.com+AD4
+AD4 +AD4 +AD4 ---
+AD4 +AD4 +AD4 block/blk-mq-debugfs.c +AHw 3 +-+--
+AD4 +AD4 +AD4 1 file changed, 2 insertions(+-), 1 deletion(-)
+AD4 +AD4 +AD4
+AD4 +AD4 +AD4 diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
+AD4 +AD4 +AD4 index 90d6876..f812083 100644
+AD4 +AD4 +AD4 --- a/block/blk-mq-debugfs.c
+AD4 +AD4 +AD4 +-+-+- b/block/blk-mq-debugfs.c
+AD4 +AD4 +AD4 +AEAAQA -308,8 +-308,9 +AEAAQA static const char +ACo-const cmd+AF8-flag+AF8-name+AFsAXQ +AD0 +AHs
+AD4 +AD4 +AD4 CMD+AF8-FLAG+AF8-NAME(PREFLUSH),
+AD4 +AD4 +AD4 CMD+AF8-FLAG+AF8-NAME(RAHEAD),
+AD4 +AD4 +AD4 CMD+AF8-FLAG+AF8-NAME(BACKGROUND),
+AD4 +AD4 +AD4 - CMD+AF8-FLAG+AF8-NAME(NOUNMAP),
+AD4 +AD4 +AD4 CMD+AF8-FLAG+AF8-NAME(NOWAIT),
+AD4 +AD4 +AD4 +- CMD+AF8-FLAG+AF8-NAME(NOUNMAP),
+AD4 +AD4 +AD4 +- CMD+AF8-FLAG+AF8-NAME(HIPRI),
+AD4 +AD4 +AD4 +AH0AOw
+AD4 +AD4 +AD4 +ACM-undef CMD+AF8-FLAG+AF8-NAME
+AD4 +AD4
+AD4 +AD4 Acked-by: Jeff Moyer +ADw-jmoyer+AEA-redhat.com+AD4
+AD4 +AD4
+AD4 +AD4 You might consider also adding a comment above the req+AF8-flag+AF8-bits enum
+AD4 +AD4 noting that modifications also need to be propagated to cmd+AF8-flag+AF8-name.
+AD4
+AD4 Agree... These things are notoriously difficult to keep in sync, this
+AD4 isn't the first time we've had a fixup. Not sure if that situation
+AD4 is improvable, but at least a comment might help.

How about making the build system derive the CMD+AF8-FLAG+AF8-NAME() declarations
from the include/linux/blk+ACo header files? It's not that hard to do that e.g.
with the help of sed. The output of sed could be written into a .h file and
that file could be +ACM-included from blk-mq-debugfs.c.

Bart.

2019-01-24 16:36:29

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] blk-mq: fix the cmd_flag_name array

On 1/24/19 9:28 AM, Bart Van Assche wrote:
> On Thu, 2019-01-24 at 09:22 -0700, Jens Axboe wrote:
>> On 1/24/19 8:09 AM, Jeff Moyer wrote:
>>> Jianchao Wang <[email protected]> writes:
>>>
>>>> Swap REQ_NOWAIT and REQ_NOUNMAP and add REQ_HIPRI.
>>>>
>>>> Signed-off-by: Jianchao Wang <[email protected]>
>>>> ---
>>>> block/blk-mq-debugfs.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
>>>> index 90d6876..f812083 100644
>>>> --- a/block/blk-mq-debugfs.c
>>>> +++ b/block/blk-mq-debugfs.c
>>>> @@ -308,8 +308,9 @@ static const char *const cmd_flag_name[] = {
>>>> CMD_FLAG_NAME(PREFLUSH),
>>>> CMD_FLAG_NAME(RAHEAD),
>>>> CMD_FLAG_NAME(BACKGROUND),
>>>> - CMD_FLAG_NAME(NOUNMAP),
>>>> CMD_FLAG_NAME(NOWAIT),
>>>> + CMD_FLAG_NAME(NOUNMAP),
>>>> + CMD_FLAG_NAME(HIPRI),
>>>> };
>>>> #undef CMD_FLAG_NAME
>>>
>>> Acked-by: Jeff Moyer <[email protected]>
>>>
>>> You might consider also adding a comment above the req_flag_bits enum
>>> noting that modifications also need to be propagated to cmd_flag_name.
>>
>> Agree... These things are notoriously difficult to keep in sync, this
>> isn't the first time we've had a fixup. Not sure if that situation
>> is improvable, but at least a comment might help.
>
> How about making the build system derive the CMD_FLAG_NAME() declarations
> from the include/linux/blk* header files? It's not that hard to do that e.g.
> with the help of sed. The output of sed could be written into a .h file and
> that file could be #included from blk-mq-debugfs.c.

I'd be fine with that, if we already require sed for building other parts.
If it can't be handled with pre processor macros or similar.

--
Jens Axboe