Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.

2007-09-27 Thread David Miller
From: [EMAIL PROTECTED] (Eric W. Biederman)
Date: Thu, 27 Sep 2007 14:44:37 -0600

> David Miller <[EMAIL PROTECTED]> writes:
> 
> > From: [EMAIL PROTECTED] (Eric W. Biederman)
> > Date: Thu, 27 Sep 2007 01:48:00 -0600
> >
> >> I'm not doing get_cpu/put_cpu so does the comment make sense
> >> in relationship to per_cpu_ptr?
> >
> > It is possible.  But someone would need to go check for
> > sure.
> 
> Verified.
> 
> hard_start_xmit is called inside of a
> rcu_read_lock_bh(),rcu_read_unlock_bh() pair.  Which means
> the code will only run on one cpu.
> 
> Therefore we do not need get_cpu/put_cpu.
> 
> In addition per_cpu_ptr is valid.  As it is just a lookup
> into a NR_CPUS sized array by smp_processor_id() to return
> the address of the specific cpu.
> 
> The only difference between per_cpu_ptr and __get_cpu_var()
> are the implementation details between statically allocated
> and dynamically allocated per cpu state.
> 
> So the comment is still valid, and still interesting it just
> should say per_cpu_ptr instead of __get_cpu_var.
> 
> Signed-off-by: "Eric W. Biederman" <[EMAIL PROTECTED]>

I've already removed the comment, so you'll have to give
me a patch that adds it back with the new content :-)
-
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


Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.

2007-09-27 Thread Eric W. Biederman
David Miller <[EMAIL PROTECTED]> writes:

> From: [EMAIL PROTECTED] (Eric W. Biederman)
> Date: Thu, 27 Sep 2007 01:48:00 -0600
>
>> I'm not doing get_cpu/put_cpu so does the comment make sense
>> in relationship to per_cpu_ptr?
>
> It is possible.  But someone would need to go check for
> sure.

Verified.

hard_start_xmit is called inside of a
rcu_read_lock_bh(),rcu_read_unlock_bh() pair.  Which means
the code will only run on one cpu.

Therefore we do not need get_cpu/put_cpu.

In addition per_cpu_ptr is valid.  As it is just a lookup
into a NR_CPUS sized array by smp_processor_id() to return
the address of the specific cpu.

The only difference between per_cpu_ptr and __get_cpu_var()
are the implementation details between statically allocated
and dynamically allocated per cpu state.

So the comment is still valid, and still interesting it just
should say per_cpu_ptr instead of __get_cpu_var.

Signed-off-by: "Eric W. Biederman" <[EMAIL PROTECTED]>
---

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 0f9d8c6..756e267 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -154,7 +154,7 @@ static int loopback_xmit(struct sk_buff *skb, struct 
net_device *dev)
 #endif
dev->last_rx = jiffies;
 
-   /* it's OK to use __get_cpu_var() because BHs are off */
+   /* it's OK to use per_cpu_ptr() because BHs are off */
pcpu_lstats = netdev_priv(dev);
lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
lb_stats->bytes += skb->len;



-
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


Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.

2007-09-27 Thread David Miller
From: [EMAIL PROTECTED] (Eric W. Biederman)
Date: Thu, 27 Sep 2007 01:48:00 -0600

> I'm not doing get_cpu/put_cpu so does the comment make sense
> in relationship to per_cpu_ptr?

It is possible.  But someone would need to go check for
sure.
-
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


Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.

2007-09-27 Thread Eric W. Biederman
David Miller <[EMAIL PROTECTED]> writes:

> From: [EMAIL PROTECTED] (Eric W. Biederman)
> Date: Wed, 26 Sep 2007 17:53:40 -0600
>
>> 
>> This patch add support for dynamically allocating the statistics counters
>> for the loopback device and adds appropriate device methods for allocating
>> and freeing the loopback device.
>> 
>> This completes support for creating multiple instances of the loopback
>> device,  in preparation for creating per network namespace instances.
>> 
>> Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
>
> Applied to net-2.6.24, thanks.
>
>> @@ -155,7 +154,8 @@ static int loopback_xmit(struct sk_buff *skb, struct
> net_device *dev)
>>  dev->last_rx = jiffies;
>>  
>>  /* it's OK to use __get_cpu_var() because BHs are off */
>> -lb_stats = &__get_cpu_var(pcpu_lstats);
>> +pcpu_lstats = netdev_priv(dev);
>> +lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
>>  lb_stats->bytes += skb->len;
>>  lb_stats->packets++;
>>  
>
> I'm going to add a followon change that gets rid of that
> comment about __get_cpu_var() since it is no longer
> relevant.

Good point.

I'm not doing get_cpu/put_cpu so does the comment make sense
in relationship to per_cpu_ptr?

Eric

-
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


Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.

2007-09-26 Thread David Miller
From: [EMAIL PROTECTED] (Eric W. Biederman)
Date: Wed, 26 Sep 2007 17:53:40 -0600

> 
> This patch add support for dynamically allocating the statistics counters
> for the loopback device and adds appropriate device methods for allocating
> and freeing the loopback device.
> 
> This completes support for creating multiple instances of the loopback
> device,  in preparation for creating per network namespace instances.
> 
> Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>

Applied to net-2.6.24, thanks.

> @@ -155,7 +154,8 @@ static int loopback_xmit(struct sk_buff *skb, struct 
> net_device *dev)
>   dev->last_rx = jiffies;
>  
>   /* it's OK to use __get_cpu_var() because BHs are off */
> - lb_stats = &__get_cpu_var(pcpu_lstats);
> + pcpu_lstats = netdev_priv(dev);
> + lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
>   lb_stats->bytes += skb->len;
>   lb_stats->packets++;
>  

I'm going to add a followon change that gets rid of that
comment about __get_cpu_var() since it is no longer
relevant.
-
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 1/4] net: Dynamically allocate the per cpu counters for the loopback device.

2007-09-26 Thread Eric W. Biederman

This patch add support for dynamically allocating the statistics counters
for the loopback device and adds appropriate device methods for allocating
and freeing the loopback device.

This completes support for creating multiple instances of the loopback
device,  in preparation for creating per network namespace instances.

Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
---
 drivers/net/loopback.c |   32 
 1 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 4b6f7b2..f3018bb 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -62,7 +62,6 @@ struct pcpu_lstats {
unsigned long packets;
unsigned long bytes;
 };
-static DEFINE_PER_CPU(struct pcpu_lstats, pcpu_lstats);
 
 #define LOOPBACK_OVERHEAD (128 + MAX_HEADER + 16 + 16)
 
@@ -134,7 +133,7 @@ static void emulate_large_send_offload(struct sk_buff *skb)
  */
 static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-   struct pcpu_lstats *lb_stats;
+   struct pcpu_lstats *pcpu_lstats, *lb_stats;
 
skb_orphan(skb);
 
@@ -155,7 +154,8 @@ static int loopback_xmit(struct sk_buff *skb, struct 
net_device *dev)
dev->last_rx = jiffies;
 
/* it's OK to use __get_cpu_var() because BHs are off */
-   lb_stats = &__get_cpu_var(pcpu_lstats);
+   pcpu_lstats = netdev_priv(dev);
+   lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
lb_stats->bytes += skb->len;
lb_stats->packets++;
 
@@ -166,15 +166,17 @@ static int loopback_xmit(struct sk_buff *skb, struct 
net_device *dev)
 
 static struct net_device_stats *get_stats(struct net_device *dev)
 {
+   const struct pcpu_lstats *pcpu_lstats;
struct net_device_stats *stats = &dev->stats;
unsigned long bytes = 0;
unsigned long packets = 0;
int i;
 
+   pcpu_lstats = netdev_priv(dev);
for_each_possible_cpu(i) {
const struct pcpu_lstats *lb_stats;
 
-   lb_stats = &per_cpu(pcpu_lstats, i);
+   lb_stats = per_cpu_ptr(pcpu_lstats, i);
bytes   += lb_stats->bytes;
packets += lb_stats->packets;
}
@@ -198,6 +200,26 @@ static const struct ethtool_ops loopback_ethtool_ops = {
.get_rx_csum= always_on,
 };
 
+static int loopback_dev_init(struct net_device *dev)
+{
+   struct pcpu_lstats *lstats;
+
+   lstats = alloc_percpu(struct pcpu_lstats);
+   if (!lstats)
+   return -ENOMEM;
+
+   dev->priv = lstats;
+   return 0;
+}
+
+static void loopback_dev_free(struct net_device *dev)
+{
+   struct pcpu_lstats *lstats = netdev_priv(dev);
+
+   free_percpu(lstats);
+   free_netdev(dev);
+}
+
 /*
  * The loopback device is special. There is only one instance and
  * it is statically allocated. Don't do this for other devices.
@@ -225,6 +247,8 @@ static void loopback_setup(struct net_device *dev)
| NETIF_F_LLTX
| NETIF_F_NETNS_LOCAL,
dev->ethtool_ops= &loopback_ethtool_ops;
+   dev->init = loopback_dev_init;
+   dev->destructor = loopback_dev_free;
 }
 
 /* Setup and register the loopback device. */
-- 
1.5.3.rc6.17.g1911

-
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