| Message ID | 20240911135828.378317-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: ethernet: ag71xx: Remove dead code | expand |
On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote: > The err variable isn't being used anywhere other than getting > initialized to 0 and then it is being checked in if condition. The > condition can never be true. Remove the err and deadcode. > > Move the rx_dropped counter above when skb isn't found. > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On 11/09/2024 14:58, Muhammad Usama Anjum wrote: > The err variable isn't being used anywhere other than getting > initialized to 0 and then it is being checked in if condition. The > condition can never be true. Remove the err and deadcode. > > Move the rx_dropped counter above when skb isn't found. > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - Move the rx_dropped counter above when skb isn't found. > --- > drivers/net/ethernet/atheros/ag71xx.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c > index db2a8ade62055..2effceeb191db 100644 > --- a/drivers/net/ethernet/atheros/ag71xx.c > +++ b/drivers/net/ethernet/atheros/ag71xx.c > @@ -1619,7 +1619,6 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit) > unsigned int i = ring->curr & ring_mask; > struct ag71xx_desc *desc = ag71xx_ring_desc(ring, i); > int pktlen; > - int err = 0; > > if (ag71xx_desc_empty(desc)) > break; > @@ -1643,20 +1642,16 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit) > skb = napi_build_skb(ring->buf[i].rx.rx_buf, ag71xx_buffer_size(ag)); > if (!skb) { > skb_free_frag(ring->buf[i].rx.rx_buf); > + ndev->stats.rx_dropped++; > goto next; > } > > skb_reserve(skb, offset); > skb_put(skb, pktlen); > > - if (err) { > - ndev->stats.rx_dropped++; > - kfree_skb(skb); > - } else { > - skb->dev = ndev; > - skb->ip_summed = CHECKSUM_NONE; > - list_add_tail(&skb->list, &rx_list); > - } > + skb->dev = ndev; > + skb->ip_summed = CHECKSUM_NONE; > + list_add_tail(&skb->list, &rx_list); > > next: > ring->buf[i].rx.rx_buf = NULL; Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote: > The err variable isn't being used anywhere other than getting > initialized to 0 and then it is being checked in if condition. The > condition can never be true. Remove the err and deadcode. > > Move the rx_dropped counter above when skb isn't found. > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> Thank you! Regards, Oleksij
On Wed, Sep 11, 2024 at 8:20 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote: > > The err variable isn't being used anywhere other than getting > > initialized to 0 and then it is being checked in if condition. The > > condition can never be true. Remove the err and deadcode. > > > > Move the rx_dropped counter above when skb isn't found. > > > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> > > Thank you! > > Regards, > Oleksij I do not see any credits given to Qianqiang Liu, who is desperate to get his first linux patch... https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/
Hi Eric, > I do not see any credits given to Qianqiang Liu, who is desperate to get his > first linux patch... > > https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/ Yes, you are right! I'm a kernel newbie. But Linux is a FOSS software, and anyone can contribute, right? Actually, I have two patches that were merged into the linux-next branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=93497752dfed196b41d2804503e80b9a04318adb https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=cd0920ebab6bce93ac5054d621c0633f6a4d640b
On Thu, 12 Sep 2024 17:56:11 +0200 Eric Dumazet wrote: > On Wed, Sep 11, 2024 at 8:20 PM Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > On Wed, Sep 11, 2024 at 06:58:27PM +0500, Muhammad Usama Anjum wrote: > > > The err variable isn't being used anywhere other than getting > > > initialized to 0 and then it is being checked in if condition. The > > > condition can never be true. Remove the err and deadcode. > > > > > > Move the rx_dropped counter above when skb isn't found. > > > > > > Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") > > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > > > > Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> > > I do not see any credits given to Qianqiang Liu, who is desperate to get his > first linux patch... > > https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/ Right, odd, is there a reason you took over from Qianqiang Liu? Otherwise I'd prefer if they could send the next version. Last thing we need is arguments about ownership of trivial patches. This v2 has an unnecessary Fixes tag, this is not a fix.
On Thu, 12 Sep 2024 17:42:22 -0700 Jakub Kicinski wrote: > On Thu, 12 Sep 2024 17:56:11 +0200 Eric Dumazet wrote: > > I do not see any credits given to Qianqiang Liu, who is desperate to get his > > first linux patch... > > > > https://lore.kernel.org/netdev/20240910152254.21238-1-qianqiang.liu@163.com/ > > Right, odd, is there a reason you took over from Qianqiang Liu? > Otherwise I'd prefer if they could send the next version. > Last thing we need is arguments about ownership of trivial > patches. > > This v2 has an unnecessary Fixes tag, this is not a fix. Oh, I guess it may be a v2 of your own change: https://lore.kernel.org/all/20240911105924.4028423-1-usama.anjum@collabora.com/ Are you both using the same tool? 5 year old code and suddenly we get the same fix two times.
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c index db2a8ade62055..2effceeb191db 100644 --- a/drivers/net/ethernet/atheros/ag71xx.c +++ b/drivers/net/ethernet/atheros/ag71xx.c @@ -1619,7 +1619,6 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit) unsigned int i = ring->curr & ring_mask; struct ag71xx_desc *desc = ag71xx_ring_desc(ring, i); int pktlen; - int err = 0; if (ag71xx_desc_empty(desc)) break; @@ -1643,20 +1642,16 @@ static int ag71xx_rx_packets(struct ag71xx *ag, int limit) skb = napi_build_skb(ring->buf[i].rx.rx_buf, ag71xx_buffer_size(ag)); if (!skb) { skb_free_frag(ring->buf[i].rx.rx_buf); + ndev->stats.rx_dropped++; goto next; } skb_reserve(skb, offset); skb_put(skb, pktlen); - if (err) { - ndev->stats.rx_dropped++; - kfree_skb(skb); - } else { - skb->dev = ndev; - skb->ip_summed = CHECKSUM_NONE; - list_add_tail(&skb->list, &rx_list); - } + skb->dev = ndev; + skb->ip_summed = CHECKSUM_NONE; + list_add_tail(&skb->list, &rx_list); next: ring->buf[i].rx.rx_buf = NULL;
The err variable isn't being used anywhere other than getting initialized to 0 and then it is being checked in if condition. The condition can never be true. Remove the err and deadcode. Move the rx_dropped counter above when skb isn't found. Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver") Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- Changes since v1: - Move the rx_dropped counter above when skb isn't found. --- drivers/net/ethernet/atheros/ag71xx.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)