18 messages, 4 authors, 2011-07-04 · open the first message on its own page
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 01:09:03
cc: WANG Cong <redacted> Joe Perches (2): netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev netpoll: Remove trivial wrapper function netpoll_poll include/linux/netpoll.h | 2 -- net/core/netpoll.c | 13 +++---------- 2 files changed, 3 insertions(+), 12 deletions(-) -- 1.7.6.rc1
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 01:09:06
Too trivial to live. cc: WANG Cong <redacted> Signed-off-by: Joe Perches <joe@perches.com> --- net/core/netpoll.c | 9 ++------- 1 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 4ce595e..adf84dd 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c@@ -209,11 +209,6 @@ static void netpoll_poll_dev(struct net_device *dev) zap_completion_queue(); } -static void netpoll_poll(struct netpoll *np) -{ - netpoll_poll_dev(np->dev); -} - static void refill_skbs(void) { struct sk_buff *skb;
@@ -273,7 +268,7 @@ repeat: if (!skb) { if (++count < 10) { - netpoll_poll(np); + netpoll_poll_dev(np->dev); goto repeat; } return NULL;
@@ -334,7 +329,7 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, } /* tickle device maybe there is some cleanup */ - netpoll_poll(np); + netpoll_poll_dev(np->dev); udelay(USEC_PER_POLL); }
--
1.7.6.rc1
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 01:09:29
Unused symbols waste space. Commit 0e34e93177fb "(netpoll: add generic support for bridge and bonding devices)" added the symbol more than a year ago with the promise of "future use". Because it is so far unused, remove it for now. It can be easily readded if or when it actually needs to be used. cc: WANG Cong <redacted> Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/netpoll.h | 2 -- net/core/netpoll.c | 6 ++---- 2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 79358bb..5dfa091 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h@@ -40,8 +40,6 @@ struct netpoll_info { struct netpoll *netpoll; }; -void netpoll_poll_dev(struct net_device *dev); -void netpoll_poll(struct netpoll *np); void netpoll_send_udp(struct netpoll *np, const char *msg, int len); void netpoll_print_options(struct netpoll *np); int netpoll_parse_options(struct netpoll *np, char *opt);
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 18d9cbd..4ce595e 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c@@ -177,7 +177,7 @@ static void service_arp_queue(struct netpoll_info *npi) } } -void netpoll_poll_dev(struct net_device *dev) +static void netpoll_poll_dev(struct net_device *dev) { const struct net_device_ops *ops;
@@ -208,13 +208,11 @@ void netpoll_poll_dev(struct net_device *dev) zap_completion_queue(); } -EXPORT_SYMBOL(netpoll_poll_dev); -void netpoll_poll(struct netpoll *np) +static void netpoll_poll(struct netpoll *np) { netpoll_poll_dev(np->dev); } -EXPORT_SYMBOL(netpoll_poll); static void refill_skbs(void) {
--
1.7.6.rc1
From: Cong Wang <hidden>
Date: 2011-07-01 03:27:02
于 2011年07月01日 09:08, Joe Perches 写道:
cc: WANG Cong<redacted> Joe Perches (2): netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev netpoll: Remove trivial wrapper function netpoll_poll include/linux/netpoll.h | 2 -- net/core/netpoll.c | 13 +++---------- 2 files changed, 3 insertions(+), 12 deletions(-)
I am fine with patch 1/2, but not 2/2, since it is trivial, why touch it? With netpoll_poll(), we don't need to explore the details of struct netpoll, I think it is nice to have it. BTW, you can kill the export of netpoll_send_skb_on_dev() too. Thanks.
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 03:35:51
On Fri, 2011-07-01 at 11:26 +0800, Cong Wang wrote:
于 2011年07月01日 09:08, Joe Perches 写道:quoted
cc: WANG Cong<redacted> Joe Perches (2): netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev netpoll: Remove trivial wrapper function netpoll_poll include/linux/netpoll.h | 2 -- net/core/netpoll.c | 13 +++---------- 2 files changed, 3 insertions(+), 12 deletions(-)I am fine with patch 1/2, but not 2/2, since it is trivial, why touch it? With netpoll_poll(), we don't need to explore the details of struct netpoll, I think it is nice to have it.
Shrug. Neither is used. Keep things minimal.
BTW, you can kill the export of netpoll_send_skb_on_dev() too.
Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h. cheers, Joe
From: Cong Wang <hidden>
Date: 2011-07-01 03:49:30
于 2011年07月01日 11:35, Joe Perches 写道:
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.
% git grep netpoll_send_skb_on_dev . include/linux/netpoll.h:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, include/linux/netpoll.h: netpoll_send_skb_on_dev(np, skb, np->dev); net/core/netpoll.c:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, net/core/netpoll.c:EXPORT_SYMBOL(netpoll_send_skb_on_dev); No modules use it...
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 03:55:25
On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:
于 2011年07月01日 11:35, Joe Perches 写道:quoted
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.% git grep netpoll_send_skb_on_dev . include/linux/netpoll.h:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, include/linux/netpoll.h: netpoll_send_skb_on_dev(np, skb, np->dev); net/core/netpoll.c:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, net/core/netpoll.c:EXPORT_SYMBOL(netpoll_send_skb_on_dev); No modules use it...
Read what I wrote. Look up one level at where it's used by netpoll.h.
From: Cong Wang <hidden>
Date: 2011-07-01 04:00:23
于 2011年07月01日 11:55, Joe Perches 写道:
On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:quoted
于 2011年07月01日 11:35, Joe Perches 写道:quoted
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.% git grep netpoll_send_skb_on_dev . include/linux/netpoll.h:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, include/linux/netpoll.h: netpoll_send_skb_on_dev(np, skb, np->dev); net/core/netpoll.c:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, net/core/netpoll.c:EXPORT_SYMBOL(netpoll_send_skb_on_dev); No modules use it...Read what I wrote. Look up one level at where it's used by netpoll.h.
Then move netpoll_send_skb(), export it instead of netpoll_send_skb_on_dev()...
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 04:04:03
On Fri, 2011-07-01 at 12:00 +0800, Cong Wang wrote:
于 2011年07月01日 11:55, Joe Perches 写道:quoted
On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:quoted
于 2011年07月01日 11:35, Joe Perches 写道:quoted
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.Read what I wrote. Look up one level at where it's used by netpoll.h.Then move netpoll_send_skb(), export it instead of netpoll_send_skb_on_dev()...
Submit that if you choose.
From: Cong Wang <hidden>
Date: 2011-07-01 04:05:51
于 2011年07月01日 12:03, Joe Perches 写道:
On Fri, 2011-07-01 at 12:00 +0800, Cong Wang wrote:quoted
于 2011年07月01日 11:55, Joe Perches 写道:quoted
On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:quoted
于 2011年07月01日 11:35, Joe Perches 写道:quoted
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.Read what I wrote. Look up one level at where it's used by netpoll.h.Then move netpoll_send_skb(), export it instead of netpoll_send_skb_on_dev()...Submit that if you choose.
Well, you are on it, I am fine to leave as it is, as you agree these changes are all trivial. :)
From: David Miller <davem@davemloft.net>
Date: 2011-07-01 07:06:51
From: Cong Wang <redacted> Date: Fri, 01 Jul 2011 11:49:23 +0800
于 2011年07月01日 11:35, Joe Perches 写道:quoted
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.% git grep netpoll_send_skb_on_dev . include/linux/netpoll.h:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, include/linux/netpoll.h: netpoll_send_skb_on_dev(np, skb, np->dev); net/core/netpoll.c:void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb, net/core/netpoll.c:EXPORT_SYMBOL(netpoll_send_skb_on_dev); No modules use it...
Right, this is a relic that bonding was using at one point in the past but it no longer does.
From: Joe Perches <joe@perches.com>
Date: 2011-07-01 22:18:26
On Fri, 2011-07-01 at 00:06 -0700, David Miller wrote:
From: Cong Wang <redacted>quoted
quoted
quoted
BTW, you can kill the export of netpoll_send_skb_on_dev() too.Not too sure about that. It's used in netpoll.h by netpoll_send_skb. It could be called from anywhere. It's currently called/used by bonding.h.% git grep netpoll_send_skb_on_dev .
[]
quoted
No modules use it...Right, this is a relic that bonding was using at one point in the past but it no longer does.
Are you sure?
netpoll.h:
void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
struct net_device *dev);
static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
{
netpoll_send_skb_on_dev(np, skb, np->dev);
}
$ grep -rP --include=*.[ch] -w netpoll_send_skb *
drivers/net/bonding/bonding.h: netpoll_send_skb(np, skb);
include/linux/netpoll.h:static inline void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
net/bridge/br_private.h: netpoll_send_skb(np, skb);
net/core/netpoll.c: "netpoll_send_skb(): %s enabled interrupts in poll (%pF)\n",
net/core/netpoll.c: netpoll_send_skb(np, skb);
net/core/netpoll.c: netpoll_send_skb(np, send_skb);
from bonding.h
#ifdef CONFIG_NET_POLL_CONTROLLER
static inline void bond_netpoll_send_skb(const struct slave *slave,
struct sk_buff *skb)
{
struct netpoll *np = slave->np;
if (np)
netpoll_send_skb(np, skb);
}
and br_private.h
static inline void br_netpoll_send_skb(const struct net_bridge_port *p,
struct sk_buff *skb)
{
struct netpoll *np = p->np;
if (np)
netpoll_send_skb(np, skb);
}
These are inlined and dependent on CONFIG_NET_POLL_CONTROLLER
in drivers/net/Kconfig so doesn't netpoll_send_skb_on_dev need
to be SYMBOL_EXPORTed?
cheers, Joe
From: David Miller <davem@davemloft.net>
Date: 2011-07-01 22:31:15
From: Joe Perches <joe@perches.com> Date: Fri, 01 Jul 2011 15:18:23 -0700
These are inlined and dependent on CONFIG_NET_POLL_CONTROLLER in drivers/net/Kconfig so doesn't netpoll_send_skb_on_dev need to be SYMBOL_EXPORTed?
Right I noticed this when I read the rest of the thread where this was discussed.
From: David Miller <davem@davemloft.net>
Date: 2011-07-04 03:02:33
From: Joe Perches <joe@perches.com> Date: Thu, 30 Jun 2011 18:08:57 -0700
Unused symbols waste space. Commit 0e34e93177fb "(netpoll: add generic support for bridge and bonding devices)" added the symbol more than a year ago with the promise of "future use". Because it is so far unused, remove it for now. It can be easily readded if or when it actually needs to be used. cc: WANG Cong <redacted> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
From: David Miller <davem@davemloft.net>
Date: 2011-07-04 03:02:40
From: Joe Perches <joe@perches.com> Date: Thu, 30 Jun 2011 18:08:58 -0700
Too trivial to live. cc: WANG Cong <redacted> Signed-off-by: Joe Perches <joe@perches.com>
Applied.
From: Harry Wei <hidden>
Date: 2011-07-04 06:53:44
On Thu, Jun 30, 2011 at 06:08:57PM -0700, Joe Perches wrote:
Unused symbols waste space. Commit 0e34e93177fb "(netpoll: add generic support for bridge and bonding devices)" added the symbol more than a year ago with the promise of "future use". Because it is so far unused, remove it for now. It can be easily readded if or when it actually needs to be used. cc: WANG Cong <redacted> Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/netpoll.h | 2 -- net/core/netpoll.c | 6 ++---- 2 files changed, 2 insertions(+), 6 deletions(-)diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index 79358bb..5dfa091 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h@@ -40,8 +40,6 @@ struct netpoll_info { struct netpoll *netpoll; }; -void netpoll_poll_dev(struct net_device *dev); -void netpoll_poll(struct netpoll *np); void netpoll_send_udp(struct netpoll *np, const char *msg, int len); void netpoll_print_options(struct netpoll *np); int netpoll_parse_options(struct netpoll *np, char *opt);diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 18d9cbd..4ce595e 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c@@ -177,7 +177,7 @@ static void service_arp_queue(struct netpoll_info *npi) } } -void netpoll_poll_dev(struct net_device *dev) +static void netpoll_poll_dev(struct net_device *dev)
Hi Joe,
You have removed the function declaration in include/linux/netpoll.h.
And you changed the function like
static void netpoll_poll_dev(...)
{
...
}
But i don't know its usage.
The 'static' before this function just says the active area of 'netpoll_poll_dev'
is in the file net/core/netpoll.c. What is the purpose? Could you please tell me?
Thanks in advance.
{ const struct net_device_ops *ops;@@ -208,13 +208,11 @@ void netpoll_poll_dev(struct net_device *dev) zap_completion_queue(); } -EXPORT_SYMBOL(netpoll_poll_dev); -void netpoll_poll(struct netpoll *np) +static void netpoll_poll(struct netpoll *np) { netpoll_poll_dev(np->dev); } -EXPORT_SYMBOL(netpoll_poll); static void refill_skbs(void) {-- 1.7.6.rc1
From: Joe Perches <joe@perches.com>
Date: 2011-07-04 07:46:33
On Mon, 2011-07-04 at 14:53 +0800, Harry Wei wrote:
On Thu, Jun 30, 2011 at 06:08:57PM -0700, Joe Perches wrote:quoted
Unused symbols waste space. Commit 0e34e93177fb "(netpoll: add generic support for bridge and bonding devices)" added the symbol more than a year ago with the promise of "future use". Because it is so far unused, remove it for now. It can be easily readded if or when it actually needs to be used. cc: WANG Cong <redacted> Signed-off-by: Joe Perches <joe@perches.com> --- include/linux/netpoll.h | 2 -- net/core/netpoll.c | 6 ++---- 2 files changed, 2 insertions(+), 6 deletions(-)diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
[]
quoted
@@ -40,8 +40,6 @@ struct netpoll_info { struct netpoll *netpoll; }; -void netpoll_poll_dev(struct net_device *dev); -void netpoll_poll(struct netpoll *np); void netpoll_send_udp(struct netpoll *np, const char *msg, int len); void netpoll_print_options(struct netpoll *np); int netpoll_parse_options(struct netpoll *np, char *opt);diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 18d9cbd..4ce595e 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c@@ -177,7 +177,7 @@ static void service_arp_queue(struct netpoll_info *npi) } } -void netpoll_poll_dev(struct net_device *dev) +static void netpoll_poll_dev(struct net_device *dev)You have removed the function declaration in include/linux/netpoll.h.
Because it's unused by anything in the tree but netpoll.c
And you changed the function like
static void netpoll_poll_dev(...)
{
...
}
But i don't know its usage.It's now a local function to netpoll.c
The 'static' before this function just says the active area of 'netpoll_poll_dev' is in the file net/core/netpoll.c. What is the purpose? Could you please tell me?
It prods the device to perform network work. /* Process pending work on NIC */ ops->ndo_poll_controller(dev); poll_napi(dev);
From: Harry Wei <hidden>
Date: 2011-07-04 09:27:57
On Mon, Jul 04, 2011 at 12:46:29AM -0700, Joe Perches wrote: [...]
quoted
You have removed the function declaration in include/linux/netpoll.h.Because it's unused by anything in the tree but netpoll.c
Hmmmm..., so sorry. I misunderstood your 'unused', which i thought there was also no use in netpoll.c :-/
quoted
And you changed the function like static void netpoll_poll_dev(...) { ... } But i don't know its usage.It's now a local function to netpoll.c
Yeah, it's true.
quoted
The 'static' before this function just says the active area of 'netpoll_poll_dev' is in the file net/core/netpoll.c. What is the purpose? Could you please tell me?It prods the device to perform network work. /* Process pending work on NIC */ ops->ndo_poll_controller(dev); poll_napi(dev);
Yeah, i will read the codes. Thanks so much Harry Wei