diff mbox

[net-next-2.6] bnx2x: Remove two prefetch()

Message ID 1272406693.2343.26.camel@edumazet-laptop
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet April 27, 2010, 10:18 p.m. UTC 
Le mardi 27 avril 2010 à 15:08 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 23 Apr 2010 12:26:06 +0200
> 
> > Le vendredi 23 avril 2010 à 16:12 +0800, Changli Gao a écrit :
> >> batch skb dequeueing from softnet input_pkt_queue.
> >> 
> >> batch skb dequeueing from softnet input_pkt_queue to reduce potential lock
> >> contention when RPS is enabled.
> >> 
> >> Note: in the worst case, the number of packets in a softnet_data may be double
> >> of netdev_max_backlog.
> >> 
> >> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> >> ----
> > 
> > Oops, reading it again, I found process_backlog() was still taking the
> > lock twice, if only one packet is waiting in input_pkt_queue.
> > 
> > Possible fix, on top of your patch :
> 
> I've applied Changli's patch with this fixup added to it.
> 
> If there are any follow-on changes necessary after further analysis,
> please send patches on top of this work.
> 

Thanks David, I was about to resubmit the cumulative patch ;)

On my 'old' dev machine (two quad core), RPS is able to get a 300%
increase on udpsink test on 20 flows.

I yet have to make routing/firewalling tests as well.

I also noticed bnx2x driver has some strange prefetch() calls.

[PATCH net-next-2.6] bnx2x: Remove two prefetch()

1) Even on 64bit arches, sizeof(struct sk_buff) < 256
2) No need to prefetch same pointer twice.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Eilon Greenstein <eilong@broadcom.com>
---




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

Comments

David Miller April 27, 2010, 10:19 p.m. UTC | #1 
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 28 Apr 2010 00:18:13 +0200

> [PATCH net-next-2.6] bnx2x: Remove two prefetch()
> 
> 1) Even on 64bit arches, sizeof(struct sk_buff) < 256
> 2) No need to prefetch same pointer twice.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Eilon Greenstein <eilong@broadcom.com>

Eilon please review and ACK/NACK

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jamal April 28, 2010, 11:33 a.m. UTC | #2 
On Wed, 2010-04-28 at 00:18 +0200, Eric Dumazet wrote:

> Thanks David, I was about to resubmit the cumulative patch ;)

Hrm, i never got the email with your patch on top of Changlis
(the fscking ISP has creative ways of reordering, delaying and also
occassionaly loosing my emails). So all my tests from last
week did not include the extra patch. I will try to make time today
to test with latest net-next which seems to have some extra goodies.
If there is any other patch you want me to try let me know...

cheers,
jamal

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet April 28, 2010, 12:33 p.m. UTC | #3 
Le mercredi 28 avril 2010 à 07:33 -0400, jamal a écrit :
> On Wed, 2010-04-28 at 00:18 +0200, Eric Dumazet wrote:
> 
> > Thanks David, I was about to resubmit the cumulative patch ;)
> 
> Hrm, i never got the email with your patch on top of Changlis
> (the fscking ISP has creative ways of reordering, delaying and also
> occassionaly loosing my emails). So all my tests from last
> week did not include the extra patch. I will try to make time today
> to test with latest net-next which seems to have some extra goodies.
> If there is any other patch you want me to try let me know...
> 
> cheers,
> jamal

If you wait a bit, I have another patch to speedup udp receive path ;)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
jamal April 28, 2010, 12:36 p.m. UTC | #4 
On Wed, 2010-04-28 at 14:33 +0200, Eric Dumazet wrote:

> If you wait a bit, I have another patch to speedup udp receive path ;)

Shoot whenever you are ready ;-> I will test with and without your
patch..

cheers,
jamal

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eilon Greenstein April 28, 2010, 1:14 p.m. UTC | #5 
On Tue, 2010-04-27 at 15:19 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Wed, 28 Apr 2010 00:18:13 +0200
> 
> > [PATCH net-next-2.6] bnx2x: Remove two prefetch()
> > 
> > 1) Even on 64bit arches, sizeof(struct sk_buff) < 256
> > 2) No need to prefetch same pointer twice.
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > CC: Eilon Greenstein <eilong@broadcom.com>
> 
> Eilon please review and ACK/NACK

Vlad ran few benchmarks, and we couldn't find any justification for
those prefetch calls. After consulting with Eliezer Tamir (the original
author) we are glad to Ack this patch.

Thanks Eric!
Acked-by: <eilong@broadcom.com>



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eliezer Tamir April 28, 2010, 3:44 p.m. UTC | #6 
On Wed, Apr 28, 2010 at 4:14 PM, Eilon Greenstein <eilong@broadcom.com> wrote:
>
> On Tue, 2010-04-27 at 15:19 -0700, David Miller wrote:
> > From: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Wed, 28 Apr 2010 00:18:13 +0200
> >
> > > [PATCH net-next-2.6] bnx2x: Remove two prefetch()
> > >
> > > 1) Even on 64bit arches, sizeof(struct sk_buff) < 256
> > > 2) No need to prefetch same pointer twice.
> > >
> > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > > CC: Eilon Greenstein <eilong@broadcom.com>
> >
> > Eilon please review and ACK/NACK
>
> Vlad ran few benchmarks, and we couldn't find any justification for
> those prefetch calls. After consulting with Eliezer Tamir (the original
> author) we are glad to Ack this patch.
>
> Thanks Eric!
> Acked-by: <eilong@broadcom.com>
>
>
Normally, I would not have said anything but since Eilon asked.
Acked-by: <eliezer@tamir.org.il>
(this time in plain text)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 28, 2010, 4:53 p.m. UTC | #7 
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Wed, 28 Apr 2010 16:14:15 +0300

> On Tue, 2010-04-27 at 15:19 -0700, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Wed, 28 Apr 2010 00:18:13 +0200
>> 
>> > [PATCH net-next-2.6] bnx2x: Remove two prefetch()
>> > 
>> > 1) Even on 64bit arches, sizeof(struct sk_buff) < 256
>> > 2) No need to prefetch same pointer twice.
>> > 
>> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> > CC: Eilon Greenstein <eilong@broadcom.com>
>> 
>> Eilon please review and ACK/NACK
> 
> Vlad ran few benchmarks, and we couldn't find any justification for
> those prefetch calls. After consulting with Eliezer Tamir (the original
> author) we are glad to Ack this patch.
> 
> Thanks Eric!
> Acked-by: <eilong@broadcom.com>

Thanks, applied.

Please put your full name as well as your email address in Acked-by:
tags, just like you do for Signed-off-by: tags.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller April 28, 2010, 4:55 p.m. UTC | #8 
From: Eliezer Tamir <eliezer@tamir.org.il>
Date: Wed, 28 Apr 2010 18:42:37 +0300

> Acked-by: <eliezer@tamir.org.il>

Like I told Eilon, please specify your full name in future Acked-by: tags,
just as you would for a Signed-off-by: tag.

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

Patch

diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
index 613f727..f706ed1 100644
--- a/drivers/net/bnx2x_main.c
+++ b/drivers/net/bnx2x_main.c
@@ -1617,7 +1617,6 @@  static int bnx2x_rx_int(struct bnx2x_fastpath *fp, int budget)
 			rx_buf = &fp->rx_buf_ring[bd_cons];
 			skb = rx_buf->skb;
 			prefetch(skb);
-			prefetch((u8 *)skb + 256);
 			len = le16_to_cpu(cqe->fast_path_cqe.pkt_len);
 			pad = cqe->fast_path_cqe.placement_offset;
 
@@ -1668,7 +1667,6 @@  static int bnx2x_rx_int(struct bnx2x_fastpath *fp, int budget)
 					dma_unmap_addr(rx_buf, mapping),
 						   pad + RX_COPY_THRESH,
 						   DMA_FROM_DEVICE);
-			prefetch(skb);
 			prefetch(((char *)(skb)) + 128);
 
 			/* is this an error packet? */