| Message ID | 1604641431-6295-1-git-send-email-alex.shi@linux.alibaba.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | net/xdp: remove unused macro REG_STATE_NEW | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Guessed tree name to be net-next |
| jkicinski/subject_prefix | warning | Target tree name not specified in the subject |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On Fri, 6 Nov 2020 13:43:51 +0800 Alex Shi <alex.shi@linux.alibaba.com> wrote: > To tame gcc warning on it: > net/core/xdp.c:20:0: warning: macro "REG_STATE_NEW" is not used > [-Wunused-macros] Hmm... REG_STATE_NEW is zero, so it is implicitly set via memset zero. But it is true that it is technically not directly used or referenced. It is mentioned in a comment, so please send V2 with this additional change: $ git diff diff --git a/net/core/xdp.c b/net/core/xdp.c index 48aba933a5a8..6e1430971ac2 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -175,7 +175,7 @@ int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq, return -ENODEV; } - /* State either UNREGISTERED or NEW */ + /* State either UNREGISTERED or zero */ xdp_rxq_info_init(xdp_rxq); xdp_rxq->dev = dev; xdp_rxq->queue_index = queue_index; > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Jesper Dangaard Brouer <hawk@kernel.org> > Cc: John Fastabend <john.fastabend@gmail.com> > Cc: netdev@vger.kernel.org > Cc: bpf@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > net/core/xdp.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/net/core/xdp.c b/net/core/xdp.c > index 48aba933a5a8..3d88aab19c89 100644 > --- a/net/core/xdp.c > +++ b/net/core/xdp.c > @@ -19,7 +19,6 @@ > #include <trace/events/xdp.h> > #include <net/xdp_sock_drv.h> > > -#define REG_STATE_NEW 0x0 > #define REG_STATE_REGISTERED 0x1 > #define REG_STATE_UNREGISTERED 0x2 > #define REG_STATE_UNUSED 0x3
在 2020/11/7 上午12:13, Jesper Dangaard Brouer 写道: > Hmm... REG_STATE_NEW is zero, so it is implicitly set via memset zero. > But it is true that it is technically not directly used or referenced. > > It is mentioned in a comment, so please send V2 with this additional change: Hi Jesper, Thanks a lot for comments. here is the v2: From 2908d25bf2e1c90ad71a83ba056743f45da283e8 Mon Sep 17 00:00:00 2001 From: Alex Shi <alex.shi@linux.alibaba.com> Date: Fri, 6 Nov 2020 13:41:58 +0800 Subject: [PATCH v2] net/xdp: remove unused macro REG_STATE_NEW To tame gcc warning on it: net/core/xdp.c:20:0: warning: macro "REG_STATE_NEW" is not used [-Wunused-macros] And change related comments as Jesper Dangaard Brouer suggested. Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> --- net/core/xdp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/core/xdp.c b/net/core/xdp.c index 48aba933a5a8..0df5ee5682d9 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -19,7 +19,6 @@ #include <trace/events/xdp.h> #include <net/xdp_sock_drv.h> -#define REG_STATE_NEW 0x0 #define REG_STATE_REGISTERED 0x1 #define REG_STATE_UNREGISTERED 0x2 #define REG_STATE_UNUSED 0x3 @@ -175,7 +174,7 @@ int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq, return -ENODEV; } - /* State either UNREGISTERED or NEW */ + /* State either UNREGISTERED or zero */ xdp_rxq_info_init(xdp_rxq); xdp_rxq->dev = dev; xdp_rxq->queue_index = queue_index;
Alex Shi wrote: > > > 在 2020/11/7 上午12:13, Jesper Dangaard Brouer 写道: > > Hmm... REG_STATE_NEW is zero, so it is implicitly set via memset zero. > > But it is true that it is technically not directly used or referenced. > > > > It is mentioned in a comment, so please send V2 with this additional change: > > Hi Jesper, > > Thanks a lot for comments. here is the v2: > > From 2908d25bf2e1c90ad71a83ba056743f45da283e8 Mon Sep 17 00:00:00 2001 > From: Alex Shi <alex.shi@linux.alibaba.com> > Date: Fri, 6 Nov 2020 13:41:58 +0800 > Subject: [PATCH v2] net/xdp: remove unused macro REG_STATE_NEW > > To tame gcc warning on it: > net/core/xdp.c:20:0: warning: macro "REG_STATE_NEW" is not used > [-Wunused-macros] > And change related comments as Jesper Dangaard Brouer suggested. > > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> > --- > net/core/xdp.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/core/xdp.c b/net/core/xdp.c > index 48aba933a5a8..0df5ee5682d9 100644 > --- a/net/core/xdp.c > +++ b/net/core/xdp.c > @@ -19,7 +19,6 @@ > #include <trace/events/xdp.h> > #include <net/xdp_sock_drv.h> > > -#define REG_STATE_NEW 0x0 > #define REG_STATE_REGISTERED 0x1 > #define REG_STATE_UNREGISTERED 0x2 > #define REG_STATE_UNUSED 0x3 I think having the define there makes it more readable and clear what the zero state is. But if we run with unused-macros I guess its even uglier to try and mark it with unused attribute. Acked-by: John Fastabend <john.fastabend@gmail.com> > @@ -175,7 +174,7 @@ int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq, > return -ENODEV; > } > > - /* State either UNREGISTERED or NEW */ > + /* State either UNREGISTERED or zero */ > xdp_rxq_info_init(xdp_rxq); > xdp_rxq->dev = dev; > xdp_rxq->queue_index = queue_index; > -- > 1.8.3.1 >
On Mon, 09 Nov 2020 13:44:48 -0800 John Fastabend <john.fastabend@gmail.com> wrote: > Alex Shi wrote: > > > > > > 在 2020/11/7 上午12:13, Jesper Dangaard Brouer 写道: > > > Hmm... REG_STATE_NEW is zero, so it is implicitly set via memset zero. > > > But it is true that it is technically not directly used or referenced. > > > > > > It is mentioned in a comment, so please send V2 with this additional change: > > > > Hi Jesper, > > > > Thanks a lot for comments. here is the v2: > > > > From 2908d25bf2e1c90ad71a83ba056743f45da283e8 Mon Sep 17 00:00:00 2001 > > From: Alex Shi <alex.shi@linux.alibaba.com> > > Date: Fri, 6 Nov 2020 13:41:58 +0800 > > Subject: [PATCH v2] net/xdp: remove unused macro REG_STATE_NEW > > > > To tame gcc warning on it: > > net/core/xdp.c:20:0: warning: macro "REG_STATE_NEW" is not used > > [-Wunused-macros] > > And change related comments as Jesper Dangaard Brouer suggested. > > > > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> > > --- > > > net/core/xdp.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/net/core/xdp.c b/net/core/xdp.c > > index 48aba933a5a8..0df5ee5682d9 100644 > > --- a/net/core/xdp.c > > +++ b/net/core/xdp.c > > @@ -19,7 +19,6 @@ > > #include <trace/events/xdp.h> > > #include <net/xdp_sock_drv.h> > > > > -#define REG_STATE_NEW 0x0 > > #define REG_STATE_REGISTERED 0x1 > > #define REG_STATE_UNREGISTERED 0x2 > > #define REG_STATE_UNUSED 0x3 > > I think having the define there makes it more readable and clear what > the zero state is. But if we run with unused-macros I guess its even > uglier to try and mark it with unused attribute. I agree having the define there makes it more readable and clear what the zero state is. We can also add code that replace the comment, that check/use these defines. It is slow-path code, so it doesn't hurt to add this extra code. Generally I find it strange to "fix" these kind of warnings, but also don't care that we do fix them if it helps someone else spot code where it actually matters. > Acked-by: John Fastabend <john.fastabend@gmail.com> > > > @@ -175,7 +174,7 @@ int xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq, > > return -ENODEV; > > } > > > > - /* State either UNREGISTERED or NEW */ > > + /* State either UNREGISTERED or zero */ > > xdp_rxq_info_init(xdp_rxq); > > xdp_rxq->dev = dev; > > xdp_rxq->queue_index = queue_index; > > -- > > 1.8.3.1 > > > >
diff --git a/net/core/xdp.c b/net/core/xdp.c index 48aba933a5a8..3d88aab19c89 100644 --- a/net/core/xdp.c +++ b/net/core/xdp.c @@ -19,7 +19,6 @@ #include <trace/events/xdp.h> #include <net/xdp_sock_drv.h> -#define REG_STATE_NEW 0x0 #define REG_STATE_REGISTERED 0x1 #define REG_STATE_UNREGISTERED 0x2 #define REG_STATE_UNUSED 0x3
To tame gcc warning on it: net/core/xdp.c:20:0: warning: macro "REG_STATE_NEW" is not used [-Wunused-macros] Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Jesper Dangaard Brouer <hawk@kernel.org> Cc: John Fastabend <john.fastabend@gmail.com> Cc: netdev@vger.kernel.org Cc: bpf@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- net/core/xdp.c | 1 - 1 file changed, 1 deletion(-)