* [PATCH] move skb->dev assignment into netdev_alloc_skb
@ 2006-08-05 13:01 Christoph Hellwig
2006-08-07 23:10 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2006-08-05 13:01 UTC (permalink / raw)
To: davem; +Cc: netdev
All caller of netdev_alloc_skb need to assign skb->dev shortly
afterwards. Move it into common code.
I also had to fixup a little bit of the surrounding control flow in
e1000 - it was just too convoluted.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c 2006-08-04 19:06:09.000000000 +0200
+++ linux-2.6/drivers/net/e1000/e1000_main.c 2006-08-05 14:18:32.000000000 +0200
@@ -3711,7 +3711,6 @@
netdev_alloc_skb(netdev, length + NET_IP_ALIGN);
if (new_skb) {
skb_reserve(new_skb, NET_IP_ALIGN);
- new_skb->dev = netdev;
memcpy(new_skb->data - NET_IP_ALIGN,
skb->data - NET_IP_ALIGN,
length + NET_IP_ALIGN);
@@ -3978,13 +3977,13 @@
buffer_info = &rx_ring->buffer_info[i];
while (cleaned_count--) {
- if (!(skb = buffer_info->skb))
- skb = netdev_alloc_skb(netdev, bufsz);
- else {
+ skb = buffer_info->skb;
+ if (skb) {
skb_trim(skb, 0);
goto map_skb;
}
+ skb = netdev_alloc_skb(netdev, bufsz);
if (unlikely(!skb)) {
/* Better luck next round */
adapter->alloc_rx_buff_failed++;
@@ -4009,10 +4008,10 @@
dev_kfree_skb(skb);
dev_kfree_skb(oldskb);
break; /* while !buffer_info->skb */
- } else {
- /* Use new allocation */
- dev_kfree_skb(oldskb);
}
+
+ /* Use new allocation */
+ dev_kfree_skb(oldskb);
}
/* Make buffer alignment 2 beyond a 16 byte boundary
* this will result in a 16 byte aligned IP header after
@@ -4020,8 +4019,6 @@
*/
skb_reserve(skb, NET_IP_ALIGN);
- skb->dev = netdev;
-
buffer_info->skb = skb;
buffer_info->length = adapter->rx_buffer_len;
map_skb:
@@ -4135,8 +4132,6 @@
*/
skb_reserve(skb, NET_IP_ALIGN);
- skb->dev = netdev;
-
buffer_info->skb = skb;
buffer_info->length = adapter->rx_ps_bsize0;
buffer_info->dma = pci_map_single(pdev, skb->data,
Index: linux-2.6/drivers/net/tg3.c
===================================================================
--- linux-2.6.orig/drivers/net/tg3.c 2006-08-04 19:06:11.000000000 +0200
+++ linux-2.6/drivers/net/tg3.c 2006-08-05 14:18:18.000000000 +0200
@@ -3101,7 +3101,6 @@
if (skb == NULL)
return -ENOMEM;
- skb->dev = tp->dev;
skb_reserve(skb, tp->rx_offset);
mapping = pci_map_single(tp->pdev, skb->data,
@@ -3274,7 +3273,6 @@
if (copy_skb == NULL)
goto drop_it_no_recycle;
- copy_skb->dev = tp->dev;
skb_reserve(copy_skb, 2);
skb_put(copy_skb, len);
pci_dma_sync_single_for_cpu(tp->pdev, dma_addr, len, PCI_DMA_FROMDEVICE);
Index: linux-2.6/net/core/skbuff.c
===================================================================
--- linux-2.6.orig/net/core/skbuff.c 2006-08-04 19:06:37.000000000 +0200
+++ linux-2.6/net/core/skbuff.c 2006-08-05 14:48:01.000000000 +0200
@@ -268,8 +268,10 @@
struct sk_buff *skb;
skb = alloc_skb(length + NET_SKB_PAD, gfp_mask);
- if (likely(skb))
+ if (likely(skb)) {
skb_reserve(skb, NET_SKB_PAD);
+ skb->dev = dev;
+ }
return skb;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] move skb->dev assignment into netdev_alloc_skb
2006-08-05 13:01 [PATCH] move skb->dev assignment into netdev_alloc_skb Christoph Hellwig
@ 2006-08-07 23:10 ` David Miller
2006-08-08 0:29 ` Jesse Brandeburg
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2006-08-07 23:10 UTC (permalink / raw)
To: hch; +Cc: netdev
From: Christoph Hellwig <hch@lst.de>
Date: Sat, 5 Aug 2006 15:01:09 +0200
> All caller of netdev_alloc_skb need to assign skb->dev shortly
> afterwards. Move it into common code.
>
> I also had to fixup a little bit of the surrounding control flow in
> e1000 - it was just too convoluted.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Since the e1000 change is non-trivial I'm not going to bypass
the driver author on it, sorry.
What I did do was put the netdev_alloc_skb() change into my
tree, and since I'm co-author of the tg3 driver I'll apply
that bit too.
The e1000 bit will need to go through the e1000 maintainers.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] move skb->dev assignment into netdev_alloc_skb
2006-08-07 23:10 ` David Miller
@ 2006-08-08 0:29 ` Jesse Brandeburg
2006-08-12 13:57 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Jesse Brandeburg @ 2006-08-08 0:29 UTC (permalink / raw)
To: David Miller; +Cc: hch, netdev, Brandeburg, Jesse
On 8/7/06, David Miller <davem@davemloft.net> wrote:
> From: Christoph Hellwig <hch@lst.de>
> Date: Sat, 5 Aug 2006 15:01:09 +0200
>
> > All caller of netdev_alloc_skb need to assign skb->dev shortly
> > afterwards. Move it into common code.
> >
> > I also had to fixup a little bit of the surrounding control flow in
> > e1000 - it was just too convoluted.
> >
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Since the e1000 change is non-trivial I'm not going to bypass
> the driver author on it, sorry.
>
> What I did do was put the netdev_alloc_skb() change into my
> tree, and since I'm co-author of the tg3 driver I'll apply
> that bit too.
>
> The e1000 bit will need to go through the e1000 maintainers.
Thank you, I'll take a look at this tomorrow, as I need to digest the
patch in context.
Jesse
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] move skb->dev assignment into netdev_alloc_skb
2006-08-08 0:29 ` Jesse Brandeburg
@ 2006-08-12 13:57 ` Christoph Hellwig
2006-08-15 1:00 ` Brandeburg, Jesse
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2006-08-12 13:57 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: David Miller, hch, netdev, Brandeburg, Jesse
On Mon, Aug 07, 2006 at 05:29:54PM -0700, Jesse Brandeburg wrote:
> On 8/7/06, David Miller <davem@davemloft.net> wrote:
> >From: Christoph Hellwig <hch@lst.de>
> >Date: Sat, 5 Aug 2006 15:01:09 +0200
> >
> >> All caller of netdev_alloc_skb need to assign skb->dev shortly
> >> afterwards. Move it into common code.
> >>
> >> I also had to fixup a little bit of the surrounding control flow in
> >> e1000 - it was just too convoluted.
> >>
> >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >
> >Since the e1000 change is non-trivial I'm not going to bypass
> >the driver author on it, sorry.
> >
> >What I did do was put the netdev_alloc_skb() change into my
> >tree, and since I'm co-author of the tg3 driver I'll apply
> >that bit too.
> >
> >The e1000 bit will need to go through the e1000 maintainers.
>
> Thank you, I'll take a look at this tomorrow, as I need to digest the
> patch in context.
Did you get a chance to look at it? For your conveniance here's just
the e1000 bits, without the hunks Dave has commited already:
Index: linux-2.6/drivers/net/e1000/e1000_main.c
===================================================================
--- linux-2.6.orig/drivers/net/e1000/e1000_main.c 2006-08-04 19:06:09.000000000 +0200
+++ linux-2.6/drivers/net/e1000/e1000_main.c 2006-08-05 14:18:32.000000000 +0200
@@ -3711,7 +3711,6 @@
netdev_alloc_skb(netdev, length + NET_IP_ALIGN);
if (new_skb) {
skb_reserve(new_skb, NET_IP_ALIGN);
- new_skb->dev = netdev;
memcpy(new_skb->data - NET_IP_ALIGN,
skb->data - NET_IP_ALIGN,
length + NET_IP_ALIGN);
@@ -3978,13 +3977,13 @@
buffer_info = &rx_ring->buffer_info[i];
while (cleaned_count--) {
- if (!(skb = buffer_info->skb))
- skb = netdev_alloc_skb(netdev, bufsz);
- else {
+ skb = buffer_info->skb;
+ if (skb) {
skb_trim(skb, 0);
goto map_skb;
}
+ skb = netdev_alloc_skb(netdev, bufsz);
if (unlikely(!skb)) {
/* Better luck next round */
adapter->alloc_rx_buff_failed++;
@@ -4009,10 +4008,10 @@
dev_kfree_skb(skb);
dev_kfree_skb(oldskb);
break; /* while !buffer_info->skb */
- } else {
- /* Use new allocation */
- dev_kfree_skb(oldskb);
}
+
+ /* Use new allocation */
+ dev_kfree_skb(oldskb);
}
/* Make buffer alignment 2 beyond a 16 byte boundary
* this will result in a 16 byte aligned IP header after
@@ -4020,8 +4019,6 @@
*/
skb_reserve(skb, NET_IP_ALIGN);
- skb->dev = netdev;
-
buffer_info->skb = skb;
buffer_info->length = adapter->rx_buffer_len;
map_skb:
@@ -4135,8 +4132,6 @@
*/
skb_reserve(skb, NET_IP_ALIGN);
- skb->dev = netdev;
-
buffer_info->skb = skb;
buffer_info->length = adapter->rx_ps_bsize0;
buffer_info->dma = pci_map_single(pdev, skb->data,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] move skb->dev assignment into netdev_alloc_skb
2006-08-12 13:57 ` Christoph Hellwig
@ 2006-08-15 1:00 ` Brandeburg, Jesse
2006-08-15 14:37 ` Auke Kok
0 siblings, 1 reply; 6+ messages in thread
From: Brandeburg, Jesse @ 2006-08-15 1:00 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jesse Brandeburg, David Miller, netdev, Brandeburg, Jesse,
auke-jan.h.kok
On Sat, 12 Aug 2006, Christoph Hellwig wrote:
> > >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> > >
> > >Since the e1000 change is non-trivial I'm not going to bypass
> > >the driver author on it, sorry.
> > >
> > >What I did do was put the netdev_alloc_skb() change into my
> > >tree, and since I'm co-author of the tg3 driver I'll apply
> > >that bit too.
> > >
> > >The e1000 bit will need to go through the e1000 maintainers.
> >
> > Thank you, I'll take a look at this tomorrow, as I need to digest the
> > patch in context.
>
> Did you get a chance to look at it? For your conveniance here's just
> the e1000 bits, without the hunks Dave has commited already:
>
I'm okay with this change. Auke will shortly be submitting a patch set
for 2.6.19 and it should include this.
Thanks for the feedback Christoph!
Jesse
for reference this is the patch, but like i said auke will enqueue it in
GIT and send it to Jeff for inclusion.
e1000: clean up skb allocation code, patch submitted by Christoph
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
---
drivers/net/e1000/e1000_main.c | 19 +++++++------------
1 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 627f224..66c0325 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -36,7 +36,7 @@ static char e1000_driver_string[] = "Int
#else
#define DRIVERNAPI "-NAPI"
#endif
-#define DRV_VERSION "7.1.9-k4"DRIVERNAPI
+#define DRV_VERSION "7.1.9-k6"DRIVERNAPI
char e1000_driver_version[] = DRV_VERSION;
static char e1000_copyright[] = "Copyright (c) 1999-2006 Intel Corporation.";
@@ -3711,7 +3711,6 @@ e1000_clean_rx_irq(struct e1000_adapter
netdev_alloc_skb(netdev, length + NET_IP_ALIGN);
if (new_skb) {
skb_reserve(new_skb, NET_IP_ALIGN);
- new_skb->dev = netdev;
memcpy(new_skb->data - NET_IP_ALIGN,
skb->data - NET_IP_ALIGN,
length + NET_IP_ALIGN);
@@ -3978,13 +3977,13 @@ e1000_alloc_rx_buffers(struct e1000_adap
buffer_info = &rx_ring->buffer_info[i];
while (cleaned_count--) {
- if (!(skb = buffer_info->skb))
- skb = netdev_alloc_skb(netdev, bufsz);
- else {
+ skb = buffer_info->skb;
+ if (skb) {
skb_trim(skb, 0);
goto map_skb;
}
+ skb = netdev_alloc_skb(netdev, bufsz);
if (unlikely(!skb)) {
/* Better luck next round */
adapter->alloc_rx_buff_failed++;
@@ -4009,10 +4008,10 @@ e1000_alloc_rx_buffers(struct e1000_adap
dev_kfree_skb(skb);
dev_kfree_skb(oldskb);
break; /* while !buffer_info->skb */
- } else {
- /* Use new allocation */
- dev_kfree_skb(oldskb);
}
+
+ /* Use new allocation */
+ dev_kfree_skb(oldskb);
}
/* Make buffer alignment 2 beyond a 16 byte boundary
* this will result in a 16 byte aligned IP header after
@@ -4020,8 +4019,6 @@ e1000_alloc_rx_buffers(struct e1000_adap
*/
skb_reserve(skb, NET_IP_ALIGN);
- skb->dev = netdev;
-
buffer_info->skb = skb;
buffer_info->length = adapter->rx_buffer_len;
map_skb:
@@ -4135,8 +4132,6 @@ e1000_alloc_rx_buffers_ps(struct e1000_a
*/
skb_reserve(skb, NET_IP_ALIGN);
- skb->dev = netdev;
-
buffer_info->skb = skb;
buffer_info->length = adapter->rx_ps_bsize0;
buffer_info->dma = pci_map_single(pdev, skb->data,
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] move skb->dev assignment into netdev_alloc_skb
2006-08-15 1:00 ` Brandeburg, Jesse
@ 2006-08-15 14:37 ` Auke Kok
0 siblings, 0 replies; 6+ messages in thread
From: Auke Kok @ 2006-08-15 14:37 UTC (permalink / raw)
To: Brandeburg, Jesse
Cc: Christoph Hellwig, Jesse Brandeburg, David Miller, netdev
Brandeburg, Jesse wrote:
> On Sat, 12 Aug 2006, Christoph Hellwig wrote:
>>>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>>> Since the e1000 change is non-trivial I'm not going to bypass
>>>> the driver author on it, sorry.
>>>>
>>>> What I did do was put the netdev_alloc_skb() change into my
>>>> tree, and since I'm co-author of the tg3 driver I'll apply
>>>> that bit too.
>>>>
>>>> The e1000 bit will need to go through the e1000 maintainers.
>>> Thank you, I'll take a look at this tomorrow, as I need to digest the
>>> patch in context.
>> Did you get a chance to look at it? For your conveniance here's just
>> the e1000 bits, without the hunks Dave has commited already:
>>
>
> I'm okay with this change. Auke will shortly be submitting a patch set
> for 2.6.19 and it should include this.
Great, I was already waiting for this change to drip in but hadn't checked for
it. I've queued it and it will be included in this weeks' patch series git pull
request.
Cheers,
Auke
> Thanks for the feedback Christoph!
>
> Jesse
>
> for reference this is the patch, but like i said auke will enqueue it in
> GIT and send it to Jeff for inclusion.
>
> e1000: clean up skb allocation code, patch submitted by Christoph
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> ---
>
> drivers/net/e1000/e1000_main.c | 19 +++++++------------
> 1 files changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index 627f224..66c0325 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -36,7 +36,7 @@ static char e1000_driver_string[] = "Int
> #else
> #define DRIVERNAPI "-NAPI"
> #endif
> -#define DRV_VERSION "7.1.9-k4"DRIVERNAPI
> +#define DRV_VERSION "7.1.9-k6"DRIVERNAPI
> char e1000_driver_version[] = DRV_VERSION;
> static char e1000_copyright[] = "Copyright (c) 1999-2006 Intel Corporation.";
>
> @@ -3711,7 +3711,6 @@ e1000_clean_rx_irq(struct e1000_adapter
> netdev_alloc_skb(netdev, length + NET_IP_ALIGN);
> if (new_skb) {
> skb_reserve(new_skb, NET_IP_ALIGN);
> - new_skb->dev = netdev;
> memcpy(new_skb->data - NET_IP_ALIGN,
> skb->data - NET_IP_ALIGN,
> length + NET_IP_ALIGN);
> @@ -3978,13 +3977,13 @@ e1000_alloc_rx_buffers(struct e1000_adap
> buffer_info = &rx_ring->buffer_info[i];
>
> while (cleaned_count--) {
> - if (!(skb = buffer_info->skb))
> - skb = netdev_alloc_skb(netdev, bufsz);
> - else {
> + skb = buffer_info->skb;
> + if (skb) {
> skb_trim(skb, 0);
> goto map_skb;
> }
>
> + skb = netdev_alloc_skb(netdev, bufsz);
> if (unlikely(!skb)) {
> /* Better luck next round */
> adapter->alloc_rx_buff_failed++;
> @@ -4009,10 +4008,10 @@ e1000_alloc_rx_buffers(struct e1000_adap
> dev_kfree_skb(skb);
> dev_kfree_skb(oldskb);
> break; /* while !buffer_info->skb */
> - } else {
> - /* Use new allocation */
> - dev_kfree_skb(oldskb);
> }
> +
> + /* Use new allocation */
> + dev_kfree_skb(oldskb);
> }
> /* Make buffer alignment 2 beyond a 16 byte boundary
> * this will result in a 16 byte aligned IP header after
> @@ -4020,8 +4019,6 @@ e1000_alloc_rx_buffers(struct e1000_adap
> */
> skb_reserve(skb, NET_IP_ALIGN);
>
> - skb->dev = netdev;
> -
> buffer_info->skb = skb;
> buffer_info->length = adapter->rx_buffer_len;
> map_skb:
> @@ -4135,8 +4132,6 @@ e1000_alloc_rx_buffers_ps(struct e1000_a
> */
> skb_reserve(skb, NET_IP_ALIGN);
>
> - skb->dev = netdev;
> -
> buffer_info->skb = skb;
> buffer_info->length = adapter->rx_ps_bsize0;
> buffer_info->dma = pci_map_single(pdev, skb->data,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-08-15 14:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-05 13:01 [PATCH] move skb->dev assignment into netdev_alloc_skb Christoph Hellwig
2006-08-07 23:10 ` David Miller
2006-08-08 0:29 ` Jesse Brandeburg
2006-08-12 13:57 ` Christoph Hellwig
2006-08-15 1:00 ` Brandeburg, Jesse
2006-08-15 14:37 ` Auke Kok
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).