* [PATCH] net/xdp: remove unused macro REG_STATE_NEW @ 2020-11-06 5:43 Alex Shi 2020-11-06 16:13 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 5+ messages in thread From: Alex Shi @ 2020-11-06 5:43 UTC (permalink / raw) To: davem Cc: Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, netdev, bpf, linux-kernel 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(-) 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 -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net/xdp: remove unused macro REG_STATE_NEW 2020-11-06 5:43 [PATCH] net/xdp: remove unused macro REG_STATE_NEW Alex Shi @ 2020-11-06 16:13 ` Jesper Dangaard Brouer 2020-11-07 13:08 ` Alex Shi 0 siblings, 1 reply; 5+ messages in thread From: Jesper Dangaard Brouer @ 2020-11-06 16:13 UTC (permalink / raw) To: Alex Shi Cc: davem, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, netdev, bpf, linux-kernel 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 -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net/xdp: remove unused macro REG_STATE_NEW 2020-11-06 16:13 ` Jesper Dangaard Brouer @ 2020-11-07 13:08 ` Alex Shi 2020-11-09 21:44 ` John Fastabend 0 siblings, 1 reply; 5+ messages in thread From: Alex Shi @ 2020-11-07 13:08 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: davem, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, netdev, bpf, linux-kernel 在 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; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net/xdp: remove unused macro REG_STATE_NEW 2020-11-07 13:08 ` Alex Shi @ 2020-11-09 21:44 ` John Fastabend 2020-11-10 11:43 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 5+ messages in thread From: John Fastabend @ 2020-11-09 21:44 UTC (permalink / raw) To: Alex Shi, Jesper Dangaard Brouer Cc: davem, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend, netdev, bpf, linux-kernel 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net/xdp: remove unused macro REG_STATE_NEW 2020-11-09 21:44 ` John Fastabend @ 2020-11-10 11:43 ` Jesper Dangaard Brouer 0 siblings, 0 replies; 5+ messages in thread From: Jesper Dangaard Brouer @ 2020-11-10 11:43 UTC (permalink / raw) To: John Fastabend Cc: Alex Shi, davem, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer, netdev, bpf, linux-kernel 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 > > > > -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-10 11:43 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-06 5:43 [PATCH] net/xdp: remove unused macro REG_STATE_NEW Alex Shi 2020-11-06 16:13 ` Jesper Dangaard Brouer 2020-11-07 13:08 ` Alex Shi 2020-11-09 21:44 ` John Fastabend 2020-11-10 11:43 ` Jesper Dangaard Brouer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).