diff mbox series

[LEDE-DEV,1/3] ag71xx: Remove timestamp struct member and use trans_start instead

Message ID 1517956050-32412-1-git-send-email-rosenp@gmail.com
State Changes Requested
Headers show
Series [LEDE-DEV,1/3] ag71xx: Remove timestamp struct member and use trans_start instead | expand

Commit Message

Rosen Penev Feb. 6, 2018, 10:27 p.m. UTC 
Small speedup for TX.

Based on a Qualcomm commit. ag->timestamp = jiffies was not replaced with netif_trans_update(dev) because of this quote:

It should be noted that after this series several instances
of netif_trans_update() are useless (if they occur in
.ndo_start_xmit and driver doesn't set LLTX flag -- stack already
did an update).

From: http://lists.openwall.net/netdev/2016/05/03/87

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h | 2 --
 .../ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c     | 3 +--
 2 files changed, 1 insertion(+), 4 deletions(-)

Comments

Felix Fietkau Feb. 7, 2018, 1:54 p.m. UTC | #1 
On 2018-02-06 23:27, Rosen Penev wrote:
> Small speedup for TX.
> 
> Based on a Qualcomm commit. ag->timestamp = jiffies was not replaced with netif_trans_update(dev) because of this quote:
> 
> It should be noted that after this series several instances
> of netif_trans_update() are useless (if they occur in
> .ndo_start_xmit and driver doesn't set LLTX flag -- stack already
> did an update).
> 
> From: http://lists.openwall.net/netdev/2016/05/03/87
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> @@ -930,7 +929,7 @@ static bool ag71xx_check_dma_stuck(struct ag71xx *ag)
>  {
>  	u32 rx_sm, tx_sm, rx_fd;
>  
> -	if (likely(time_before(jiffies, ag->timestamp + HZ/10)))
> +	if (likely(time_before(jiffies, dev_trans_start(ag->dev) + HZ/10)))
dev_trans_start() does some extra checks that are unnecessary. You
should use netdev_get_tx_queue(dev, 0)->trans_start instead.

- Felix
Rosen Penev Feb. 7, 2018, 9:54 p.m. UTC | #2 
On Wed, Feb 7, 2018 at 5:54 AM, Felix Fietkau <nbd@nbd.name> wrote:
> On 2018-02-06 23:27, Rosen Penev wrote:
>> Small speedup for TX.
>>
>> Based on a Qualcomm commit. ag->timestamp = jiffies was not replaced with netif_trans_update(dev) because of this quote:
>>
>> It should be noted that after this series several instances
>> of netif_trans_update() are useless (if they occur in
>> .ndo_start_xmit and driver doesn't set LLTX flag -- stack already
>> did an update).
>>
>> From: http://lists.openwall.net/netdev/2016/05/03/87
>>
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>> @@ -930,7 +929,7 @@ static bool ag71xx_check_dma_stuck(struct ag71xx *ag)
>>  {
>>       u32 rx_sm, tx_sm, rx_fd;
>>
>> -     if (likely(time_before(jiffies, ag->timestamp + HZ/10)))
>> +     if (likely(time_before(jiffies, dev_trans_start(ag->dev) + HZ/10)))
> dev_trans_start() does some extra checks that are unnecessary. You
> should use netdev_get_tx_queue(dev, 0)->trans_start instead.
>
Hrm in the patches that drop trans_start from the regular netdev
struct, some drivers were modified to do that and others to use
dev_trans_start. I guess this is why...

Because that specific line got too long, I moved the timestamp
variable into the check_dma_stuck function. I also updated the commit
message.
> - Felix
diff mbox series

Patch

diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
index cea172e..d7897a9 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx.h
@@ -172,8 +172,6 @@  struct ag71xx {
 	struct napi_struct	napi;
 	u32			msg_enable;
 
-	unsigned long	timestamp;
-
 	/*
 	 * From this point onwards we're not looking at per-packet fields.
 	 */
diff --git a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
index 47b4c69..13f500b 100644
--- a/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
+++ b/target/linux/ar71xx/files/drivers/net/ethernet/atheros/ag71xx/ag71xx_main.c
@@ -816,7 +816,6 @@  static netdev_tx_t ag71xx_hard_start_xmit(struct sk_buff *skb,
 	i = (ring->curr + n - 1) & ring_mask;
 	ring->buf[i].len = skb->len;
 	ring->buf[i].skb = skb;
-	ag->timestamp = jiffies;
 
 	netdev_sent_queue(dev, skb->len);
 
@@ -930,7 +929,7 @@  static bool ag71xx_check_dma_stuck(struct ag71xx *ag)
 {
 	u32 rx_sm, tx_sm, rx_fd;
 
-	if (likely(time_before(jiffies, ag->timestamp + HZ/10)))
+	if (likely(time_before(jiffies, dev_trans_start(ag->dev) + HZ/10)))
 		return false;
 
 	if (!netif_carrier_ok(ag->dev))