| Message ID | 20221207012959.2800421-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2,1/2] net: apple: mace: don't call dev_kfree_skb() under spin_lock_irqsave() | expand |
On Wed, Dec 07, 2022 at 09:29:58AM +0800, Yang Yingliang wrote: > It is not allowed to call consume_skb() from hardware interrupt context > or with interrupts being disabled. So replace dev_kfree_skb() with > dev_consume_skb_irq() under spin_lock_irqsave(). > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > v1 -> v2: > Add a fix tag. > --- > drivers/net/ethernet/apple/mace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/apple/mace.c b/drivers/net/ethernet/apple/mace.c > index d0a771b65e88..77b4ed05140b 100644 > --- a/drivers/net/ethernet/apple/mace.c > +++ b/drivers/net/ethernet/apple/mace.c > @@ -846,7 +846,7 @@ static void mace_tx_timeout(struct timer_list *t) > if (mp->tx_bad_runt) { > mp->tx_bad_runt = 0; > } else if (i != mp->tx_fill) { > - dev_kfree_skb(mp->tx_bufs[i]); > + dev_consume_skb_irq(mp->tx_bufs[i]); Same question, why did you chose dev_consume_skb_irq and not dev_kfree_skb_irq? Thanks > if (++i >= N_TX_RING) > i = 0; > mp->tx_empty = i; > -- > 2.25.1 >
On 2022/12/8 15:58, Leon Romanovsky wrote: > On Wed, Dec 07, 2022 at 09:29:58AM +0800, Yang Yingliang wrote: >> It is not allowed to call consume_skb() from hardware interrupt context >> or with interrupts being disabled. So replace dev_kfree_skb() with >> dev_consume_skb_irq() under spin_lock_irqsave(). >> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> v1 -> v2: >> Add a fix tag. >> --- >> drivers/net/ethernet/apple/mace.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/apple/mace.c b/drivers/net/ethernet/apple/mace.c >> index d0a771b65e88..77b4ed05140b 100644 >> --- a/drivers/net/ethernet/apple/mace.c >> +++ b/drivers/net/ethernet/apple/mace.c >> @@ -846,7 +846,7 @@ static void mace_tx_timeout(struct timer_list *t) >> if (mp->tx_bad_runt) { >> mp->tx_bad_runt = 0; >> } else if (i != mp->tx_fill) { >> - dev_kfree_skb(mp->tx_bufs[i]); >> + dev_consume_skb_irq(mp->tx_bufs[i]); > Same question, why did you chose dev_consume_skb_irq and not dev_kfree_skb_irq? I chose dev_consume_skb_irq(), because dev_kfree_skb() is consume_skb(). Thanks, Yang > > Thanks > >> if (++i >= N_TX_RING) >> i = 0; >> mp->tx_empty = i; >> -- >> 2.25.1 >> > .
On Thu, 8 Dec 2022 16:39:03 +0800 Yang Yingliang wrote: > >> @@ -846,7 +846,7 @@ static void mace_tx_timeout(struct timer_list *t) > >> if (mp->tx_bad_runt) { > >> mp->tx_bad_runt = 0; > >> } else if (i != mp->tx_fill) { > >> - dev_kfree_skb(mp->tx_bufs[i]); > >> + dev_consume_skb_irq(mp->tx_bufs[i]); > > Same question, why did you chose dev_consume_skb_irq and not dev_kfree_skb_irq? > I chose dev_consume_skb_irq(), because dev_kfree_skb() is consume_skb(). kfree_skb() should be used on error paths, when packet is dropped. consume_skb() on normal paths, when packet left the system successfully. dev_kfree* helpers probably default to consume_skb() to avoid spamming drop monitor, but switching to dev_consume explicitly is not right.
diff --git a/drivers/net/ethernet/apple/mace.c b/drivers/net/ethernet/apple/mace.c index d0a771b65e88..77b4ed05140b 100644 --- a/drivers/net/ethernet/apple/mace.c +++ b/drivers/net/ethernet/apple/mace.c @@ -846,7 +846,7 @@ static void mace_tx_timeout(struct timer_list *t) if (mp->tx_bad_runt) { mp->tx_bad_runt = 0; } else if (i != mp->tx_fill) { - dev_kfree_skb(mp->tx_bufs[i]); + dev_consume_skb_irq(mp->tx_bufs[i]); if (++i >= N_TX_RING) i = 0; mp->tx_empty = i;
It is not allowed to call consume_skb() from hardware interrupt context or with interrupts being disabled. So replace dev_kfree_skb() with dev_consume_skb_irq() under spin_lock_irqsave(). Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- v1 -> v2: Add a fix tag. --- drivers/net/ethernet/apple/mace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)