[PATCH 0/2] netpoll: Trivial updates

[PATCH 0/2] netpoll: Trivial updates

[Joe Perches]

cc: WANG Cong <amwang@redhat.com>

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

[PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev

[Joe Perches]

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 <amwang@redhat.com>
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

Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev

[David Miller]

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 <amwang@redhat.com>
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev

[Harry Wei]

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 <amwang@redhat.com>
> 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

Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev

[Joe Perches]

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:
> > 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 <amwang@redhat.com>
> > 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
[]
> > @@ -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);

Re: [PATCH 1/2] netpoll: Remove unused EXPORT_SYMBOLs of netpoll_poll and netpoll_poll_dev

[Harry Wei]

On Mon, Jul 04, 2011 at 12:46:29AM -0700, Joe Perches wrote:
[...]
> > 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 :-/
> 
> > 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.
> 
> > 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

[PATCH 2/2] netpoll: Remove wrapper function netpoll_poll

[Joe Perches]

Too trivial to live.

cc: WANG Cong <amwang@redhat.com>
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

Re: [PATCH 2/2] netpoll: Remove wrapper function netpoll_poll

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Thu, 30 Jun 2011 18:08:58 -0700

> Too trivial to live.
> 
> cc: WANG Cong <amwang@redhat.com>
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

Re: [PATCH 0/2] netpoll: Trivial updates

[Cong Wang]

于 2011年07月01日 09:08, Joe Perches 写道:
> cc: WANG Cong<amwang@redhat.com>
>
> 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.

Re: [PATCH 0/2] netpoll: Trivial updates

[Joe Perches]

On Fri, 2011-07-01 at 11:26 +0800, Cong Wang wrote:
> 于 2011年07月01日 09:08, Joe Perches 写道:
> > cc: WANG Cong<amwang@redhat.com>
> > 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

Re: [PATCH 0/2] netpoll: Trivial updates

[Cong Wang]

于 2011年07月01日 11:35, Joe Perches 写道:
>
>> 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...

Re: [PATCH 0/2] netpoll: Trivial updates

[Joe Perches]

On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:
> 于 2011年07月01日 11:35, Joe Perches 写道:
> >> 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.

Re: [PATCH 0/2] netpoll: Trivial updates

[Cong Wang]

于 2011年07月01日 11:55, Joe Perches 写道:
> On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:
>> 于 2011年07月01日 11:35, Joe Perches 写道:
>>>> 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()...

Re: [PATCH 0/2] netpoll: Trivial updates

[Joe Perches]

On Fri, 2011-07-01 at 12:00 +0800, Cong Wang wrote:
> 于 2011年07月01日 11:55, Joe Perches 写道:
> > On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:
> >> 于 2011年07月01日 11:35, Joe Perches 写道:
> >>>> 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.

Re: [PATCH 0/2] netpoll: Trivial updates

[Cong Wang]

于 2011年07月01日 12:03, Joe Perches 写道:
> On Fri, 2011-07-01 at 12:00 +0800, Cong Wang wrote:
>> 于 2011年07月01日 11:55, Joe Perches 写道:
>>> On Fri, 2011-07-01 at 11:49 +0800, Cong Wang wrote:
>>>> 于 2011年07月01日 11:35, Joe Perches 写道:
>>>>>> 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. :)

Re: [PATCH 0/2] netpoll: Trivial updates

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Fri, 01 Jul 2011 11:49:23 +0800

> 于 2011年07月01日 11:35, Joe Perches 写道:
>>
>>> 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.

Re: [PATCH 0/2] netpoll: Trivial updates

[Joe Perches]

On Fri, 2011-07-01 at 00:06 -0700, David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> >>> 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 .
[]
> > 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

Re: [PATCH 0/2] netpoll: Trivial updates

[David Miller]

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.