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 |
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
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 --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))