* [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
@ 2010-03-16 15:42 Yegor Yefremov
2010-03-16 15:52 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Yegor Yefremov @ 2010-03-16 15:42 UTC (permalink / raw)
To: netdev; +Cc: davem
KS8695: update ksp->next_rx_desc_read at the end of rx loop
There is no need to adjust the next rx descriptor after each packet,
so do it only once at the end of the routine.
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
===================================================================
--- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
+++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
@@ -538,12 +538,13 @@ rx_finished:
*/
last_rx_processed = buff_n;
buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
- /*And note which RX descriptor we last did */
- if (likely(last_rx_processed != -1))
- ksp->next_rx_desc_read =
- (last_rx_processed + 1) &
- MAX_RX_DESC_MASK;
}
+
+ /*And note which RX descriptor we last did */
+ if (likely(last_rx_processed != -1))
+ ksp->next_rx_desc_read =
+ (last_rx_processed + 1) & MAX_RX_DESC_MASK;
+
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
2010-03-16 15:42 [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop Yegor Yefremov
@ 2010-03-16 15:52 ` Eric Dumazet
2010-03-17 9:39 ` Yegor Yefremov
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2010-03-16 15:52 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: netdev, davem
Le mardi 16 mars 2010 à 16:42 +0100, Yegor Yefremov a écrit :
> KS8695: update ksp->next_rx_desc_read at the end of rx loop
>
> There is no need to adjust the next rx descriptor after each packet,
> so do it only once at the end of the routine.
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>
> Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
> ===================================================================
> --- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
> +++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
> @@ -538,12 +538,13 @@ rx_finished:
> */
> last_rx_processed = buff_n;
> buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
> - /*And note which RX descriptor we last did */
> - if (likely(last_rx_processed != -1))
> - ksp->next_rx_desc_read =
> - (last_rx_processed + 1) &
> - MAX_RX_DESC_MASK;
> }
> +
> + /*And note which RX descriptor we last did */
> + if (likely(last_rx_processed != -1))
> + ksp->next_rx_desc_read =
> + (last_rx_processed + 1) & MAX_RX_DESC_MASK;
> +
Very strange to see all these computations...
At this point, why not use
if (likely(last_rx_processed != -1))
ksp->next_rx_desc_read = buff_n;
or even get rid of last_rx_processed completely and do :
diff --git a/drivers/net/arm/ks8695net.c b/drivers/net/arm/ks8695net.c
index a1d4188..ac6ab04 100644
--- a/drivers/net/arm/ks8695net.c
+++ b/drivers/net/arm/ks8695net.c
@@ -461,7 +461,6 @@ static int ks8695_rx(struct ks8695_priv *ksp, int budget)
int buff_n;
u32 flags;
int pktlen;
- int last_rx_processed = -1;
int received = 0;
buff_n = ksp->next_rx_desc_read;
@@ -533,17 +532,9 @@ rx_failure:
ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
rx_finished:
received++;
- /* And note this as processed so we can start
- * from here next time
- */
- last_rx_processed = buff_n;
buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
- /*And note which RX descriptor we last did */
- if (likely(last_rx_processed != -1))
- ksp->next_rx_desc_read =
- (last_rx_processed + 1) &
- MAX_RX_DESC_MASK;
}
+ ksp->next_rx_desc_read = buff_n;
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
2010-03-16 15:52 ` Eric Dumazet
@ 2010-03-17 9:39 ` Yegor Yefremov
2010-03-17 9:40 ` Yegor Yefremov
0 siblings, 1 reply; 5+ messages in thread
From: Yegor Yefremov @ 2010-03-17 9:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
On Tue, Mar 16, 2010 at 4:52 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 16 mars 2010 à 16:42 +0100, Yegor Yefremov a écrit :
>> KS8695: update ksp->next_rx_desc_read at the end of rx loop
>>
>> There is no need to adjust the next rx descriptor after each packet,
>> so do it only once at the end of the routine.
>>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>>
>> Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
>> ===================================================================
>> --- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
>> +++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
>> @@ -538,12 +538,13 @@ rx_finished:
>> */
>> last_rx_processed = buff_n;
>> buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
>> - /*And note which RX descriptor we last did */
>> - if (likely(last_rx_processed != -1))
>> - ksp->next_rx_desc_read =
>> - (last_rx_processed + 1) &
>> - MAX_RX_DESC_MASK;
>> }
>> +
>> + /*And note which RX descriptor we last did */
>> + if (likely(last_rx_processed != -1))
>> + ksp->next_rx_desc_read =
>> + (last_rx_processed + 1) & MAX_RX_DESC_MASK;
>> +
>
> Very strange to see all these computations...
>
> At this point, why not use
>
> if (likely(last_rx_processed != -1))
> ksp->next_rx_desc_read = buff_n;
>
> or even get rid of last_rx_processed completely and do :
>
>
> diff --git a/drivers/net/arm/ks8695net.c b/drivers/net/arm/ks8695net.c
> index a1d4188..ac6ab04 100644
> --- a/drivers/net/arm/ks8695net.c
> +++ b/drivers/net/arm/ks8695net.c
> @@ -461,7 +461,6 @@ static int ks8695_rx(struct ks8695_priv *ksp, int budget)
> int buff_n;
> u32 flags;
> int pktlen;
> - int last_rx_processed = -1;
> int received = 0;
>
> buff_n = ksp->next_rx_desc_read;
> @@ -533,17 +532,9 @@ rx_failure:
> ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
> rx_finished:
> received++;
> - /* And note this as processed so we can start
> - * from here next time
> - */
> - last_rx_processed = buff_n;
> buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
> - /*And note which RX descriptor we last did */
> - if (likely(last_rx_processed != -1))
> - ksp->next_rx_desc_read =
> - (last_rx_processed + 1) &
> - MAX_RX_DESC_MASK;
> }
> + ksp->next_rx_desc_read = buff_n;
> /* And refill the buffers */
> ks8695_refill_rxbuffers(ksp);
Thank you for reviewing. Great idea. This simplifies the driver a
little. I tested your patch and everything is working as before. I
also added your signed-off-by and made some beatifications.
Yegor
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
2010-03-17 9:39 ` Yegor Yefremov
@ 2010-03-17 9:40 ` Yegor Yefremov
2010-03-20 5:44 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Yegor Yefremov @ 2010-03-17 9:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, davem
KS8695: update ksp->next_rx_desc_read at the end of rx loop
There is no need to adjust the next rx descriptor after each packet,
so do it only once at the end of the routine.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Index: linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
===================================================================
--- linux-2.6.34-rc1.orig/drivers/net/arm/ks8695net.c
+++ linux-2.6.34-rc1/drivers/net/arm/ks8695net.c
@@ -449,11 +449,10 @@ ks8695_rx_irq(int irq, void *dev_id)
}
/**
- * ks8695_rx - Receive packets called by NAPI poll method
+ * ks8695_rx - Receive packets called by NAPI poll method
* @ksp: Private data for the KS8695 Ethernet
- * @budget: The max packets would be receive
+ * @budget: Number of packets allowed to process
*/
-
static int ks8695_rx(struct ks8695_priv *ksp, int budget)
{
struct net_device *ndev = ksp->ndev;
@@ -461,7 +460,6 @@ static int ks8695_rx(struct ks8695_priv
int buff_n;
u32 flags;
int pktlen;
- int last_rx_processed = -1;
int received = 0;
buff_n = ksp->next_rx_desc_read;
@@ -471,6 +469,7 @@ static int ks8695_rx(struct ks8695_priv
cpu_to_le32(RDES_OWN)))) {
rmb();
flags = le32_to_cpu(ksp->rx_ring[buff_n].status);
+
/* Found an SKB which we own, this means we
* received a packet
*/
@@ -533,23 +532,18 @@ rx_failure:
ksp->rx_ring[buff_n].status = cpu_to_le32(RDES_OWN);
rx_finished:
received++;
- /* And note this as processed so we can start
- * from here next time
- */
- last_rx_processed = buff_n;
buff_n = (buff_n + 1) & MAX_RX_DESC_MASK;
- /*And note which RX descriptor we last did */
- if (likely(last_rx_processed != -1))
- ksp->next_rx_desc_read =
- (last_rx_processed + 1) &
- MAX_RX_DESC_MASK;
}
+
+ /* And note which RX descriptor we last did */
+ ksp->next_rx_desc_read = buff_n;
+
/* And refill the buffers */
ks8695_refill_rxbuffers(ksp);
- /* Kick the RX DMA engine, in case it became
- * suspended */
+ /* Kick the RX DMA engine, in case it became suspended */
ks8695_writereg(ksp, KS8695_DRSC, 0);
+
return received;
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop
2010-03-17 9:40 ` Yegor Yefremov
@ 2010-03-20 5:44 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2010-03-20 5:44 UTC (permalink / raw)
To: yegorslists; +Cc: eric.dumazet, netdev
From: Yegor Yefremov <yegorslists@googlemail.com>
Date: Wed, 17 Mar 2010 10:40:10 +0100
> KS8695: update ksp->next_rx_desc_read at the end of rx loop
>
> There is no need to adjust the next rx descriptor after each packet,
> so do it only once at the end of the routine.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
Applied.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-03-20 5:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-16 15:42 [PATCH 1/1] KS8695: update ksp->next_rx_desc_read at the end of rx loop Yegor Yefremov
2010-03-16 15:52 ` Eric Dumazet
2010-03-17 9:39 ` Yegor Yefremov
2010-03-17 9:40 ` Yegor Yefremov
2010-03-20 5:44 ` David Miller
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).