Re: [PATCH 02/11] netpoll: netpoll_poll cleanup

[David Miller]

From: Stephen Hemminger <[EMAIL PROTECTED]>
Date: Sat, 03 Nov 2007 11:43:16 -0700

> Restructure code slightly to improve readability:
>   * dereference device once
>   * change obvious while() loop
>   * let poll_napi() handle null list itself
> 
> Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>

Applied to net-2.6.25, but I made some coding style fixups, one of
which is a huge pet peeve of mine.

When declaring local variables for a function I always list the
longest lines down gradually to the shortest lines. It is nicer to the
eye and naturally it means that all the complicated structure
assignments and dereferences sit at the top and the simpler iterators
and counters like 'i' end up at the bottom making local variable lists
that much easier to read and search when learning how a function works.

You explicitly changed this one I had set up:

>  {
> - struct netpoll_info *npinfo = np->dev->npinfo;
> - struct napi_struct *napi;
>   int budget = 16;
> + struct napi_struct *napi;

And I thus reverted it back to the correct order:

struct napi_struct *napi;
int budget = 16;

I also got rid of the mid-parens spaces in:

> + while ( (skb = skb_dequeue(&npi->arp_tx)) )

Thanks.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/11] netpoll: netpoll_poll cleanup

[Stephen Hemminger]

Restructure code slightly to improve readability:
  * dereference device once
  * change obvious while() loop
  * let poll_napi() handle null list itself

Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>


--- a/net/core/netpoll.c2007-11-03 10:02:50.0 -0700
+++ b/net/core/netpoll.c2007-11-03 11:40:27.0 -0700
@@ -139,16 +139,15 @@ static int poll_one_napi(struct netpoll_
return budget - work;
 }
 
-static void poll_napi(struct netpoll *np)
+static void poll_napi(struct net_device *dev)
 {
-   struct netpoll_info *npinfo = np->dev->npinfo;
-   struct napi_struct *napi;
int budget = 16;
+   struct napi_struct *napi;
 
-   list_for_each_entry(napi, &np->dev->napi_list, dev_list) {
+   list_for_each_entry(napi, &dev->napi_list, dev_list) {
if (napi->poll_owner != smp_processor_id() &&
spin_trylock(&napi->poll_lock)) {
-   budget = poll_one_napi(npinfo, napi, budget);
+   budget = poll_one_napi(dev->npinfo, napi, budget);
spin_unlock(&napi->poll_lock);
 
if (!budget)
@@ -159,30 +158,27 @@ static void poll_napi(struct netpoll *np
 
 static void service_arp_queue(struct netpoll_info *npi)
 {
-   struct sk_buff *skb;
+   if (npi) {
+   struct sk_buff *skb;
 
-   if (unlikely(!npi))
-   return;
-
-   skb = skb_dequeue(&npi->arp_tx);
-
-   while (skb != NULL) {
-   arp_reply(skb);
-   skb = skb_dequeue(&npi->arp_tx);
+   while ( (skb = skb_dequeue(&npi->arp_tx)) )
+   arp_reply(skb);
}
 }
 
 void netpoll_poll(struct netpoll *np)
 {
-   if (!np->dev || !netif_running(np->dev) || !np->dev->poll_controller)
+   struct net_device *dev = np->dev;
+
+   if (!dev || !netif_running(dev) || !dev->poll_controller)
return;
 
/* Process pending work on NIC */
-   np->dev->poll_controller(np->dev);
-   if (!list_empty(&np->dev->napi_list))
-   poll_napi(np);
+   dev->poll_controller(dev);
+
+   poll_napi(dev);
 
-   service_arp_queue(np->dev->npinfo);
+   service_arp_queue(dev->npinfo);
 
zap_completion_queue();
 }

-- 
Stephen Hemminger <[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html