| Message ID | 20240816220604.2688389-2-stephen.s.brennan@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | SUNRPC: convert RPC_TASK_* constants to enum | expand |
On Sat, 17 Aug 2024, Stephen Brennan wrote: > The RPC_TASK_* constants are defined as macros, which means that most > kernel builds will not contain their definitions in the debuginfo. > However, it's quite useful for debuggers to be able to view the task > state constant and interpret it correctly. Conversion to an enum will > ensure the constants are present in debuginfo and can be interpreted by > debuggers without needing to hard-code them and track their changes. > > Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> > --- > include/linux/sunrpc/sched.h | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h > index 0c77ba488bbae..177220524eb5d 100644 > --- a/include/linux/sunrpc/sched.h > +++ b/include/linux/sunrpc/sched.h > @@ -151,13 +151,15 @@ struct rpc_task_setup { > #define RPC_WAS_SENT(t) ((t)->tk_flags & RPC_TASK_SENT) > #define RPC_IS_MOVEABLE(t) ((t)->tk_flags & RPC_TASK_MOVEABLE) > > -#define RPC_TASK_RUNNING 0 > -#define RPC_TASK_QUEUED 1 > -#define RPC_TASK_ACTIVE 2 > -#define RPC_TASK_NEED_XMIT 3 > -#define RPC_TASK_NEED_RECV 4 > -#define RPC_TASK_MSG_PIN_WAIT 5 > -#define RPC_TASK_SIGNALLED 6 > +enum { > + RPC_TASK_RUNNING = 0, > + RPC_TASK_QUEUED = 1, > + RPC_TASK_ACTIVE = 2, > + RPC_TASK_NEED_XMIT = 3, > + RPC_TASK_NEED_RECV = 4, > + RPC_TASK_MSG_PIN_WAIT = 5, > + RPC_TASK_SIGNALLED = 6, > +}; I am strongly in favour of converting these #defines to an enum, but having the explicit assignments in the enum is pure noise adding no value at all. Would you consider resubmiting as a simple enum that uses the default values? Thanks, NeilBrown > > #define rpc_test_and_set_running(t) \ > test_and_set_bit(RPC_TASK_RUNNING, &(t)->tk_runstate) > -- > 2.43.5 > >
"NeilBrown" <neilb@suse.de> writes: > On Sat, 17 Aug 2024, Stephen Brennan wrote: >> The RPC_TASK_* constants are defined as macros, which means that most >> kernel builds will not contain their definitions in the debuginfo. >> However, it's quite useful for debuggers to be able to view the task >> state constant and interpret it correctly. Conversion to an enum will >> ensure the constants are present in debuginfo and can be interpreted by >> debuggers without needing to hard-code them and track their changes. >> >> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> >> --- >> include/linux/sunrpc/sched.h | 16 +++++++++------- >> 1 file changed, 9 insertions(+), 7 deletions(-) >> >> diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h >> index 0c77ba488bbae..177220524eb5d 100644 >> --- a/include/linux/sunrpc/sched.h >> +++ b/include/linux/sunrpc/sched.h >> @@ -151,13 +151,15 @@ struct rpc_task_setup { >> #define RPC_WAS_SENT(t) ((t)->tk_flags & RPC_TASK_SENT) >> #define RPC_IS_MOVEABLE(t) ((t)->tk_flags & RPC_TASK_MOVEABLE) >> >> -#define RPC_TASK_RUNNING 0 >> -#define RPC_TASK_QUEUED 1 >> -#define RPC_TASK_ACTIVE 2 >> -#define RPC_TASK_NEED_XMIT 3 >> -#define RPC_TASK_NEED_RECV 4 >> -#define RPC_TASK_MSG_PIN_WAIT 5 >> -#define RPC_TASK_SIGNALLED 6 >> +enum { >> + RPC_TASK_RUNNING = 0, >> + RPC_TASK_QUEUED = 1, >> + RPC_TASK_ACTIVE = 2, >> + RPC_TASK_NEED_XMIT = 3, >> + RPC_TASK_NEED_RECV = 4, >> + RPC_TASK_MSG_PIN_WAIT = 5, >> + RPC_TASK_SIGNALLED = 6, >> +}; > > I am strongly in favour of converting these #defines to an enum, but > having the explicit assignments in the enum is pure noise adding no > value at all. I agree, I only included it in case reviewers would prefer to be able to see the values, as they were with the #defines. But I think it's common knowledge that enums start at 0 and increment by 1. > Would you consider resubmiting as a simple enum that uses the default > values? Definitely! Thanks, Stephen
diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h index 0c77ba488bbae..177220524eb5d 100644 --- a/include/linux/sunrpc/sched.h +++ b/include/linux/sunrpc/sched.h @@ -151,13 +151,15 @@ struct rpc_task_setup { #define RPC_WAS_SENT(t) ((t)->tk_flags & RPC_TASK_SENT) #define RPC_IS_MOVEABLE(t) ((t)->tk_flags & RPC_TASK_MOVEABLE) -#define RPC_TASK_RUNNING 0 -#define RPC_TASK_QUEUED 1 -#define RPC_TASK_ACTIVE 2 -#define RPC_TASK_NEED_XMIT 3 -#define RPC_TASK_NEED_RECV 4 -#define RPC_TASK_MSG_PIN_WAIT 5 -#define RPC_TASK_SIGNALLED 6 +enum { + RPC_TASK_RUNNING = 0, + RPC_TASK_QUEUED = 1, + RPC_TASK_ACTIVE = 2, + RPC_TASK_NEED_XMIT = 3, + RPC_TASK_NEED_RECV = 4, + RPC_TASK_MSG_PIN_WAIT = 5, + RPC_TASK_SIGNALLED = 6, +}; #define rpc_test_and_set_running(t) \ test_and_set_bit(RPC_TASK_RUNNING, &(t)->tk_runstate)
The RPC_TASK_* constants are defined as macros, which means that most kernel builds will not contain their definitions in the debuginfo. However, it's quite useful for debuggers to be able to view the task state constant and interpret it correctly. Conversion to an enum will ensure the constants are present in debuginfo and can be interpreted by debuggers without needing to hard-code them and track their changes. Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> --- include/linux/sunrpc/sched.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-)