Messages in this thread | | | From | Eric Dumazet <> | Date | Wed, 16 Sep 2020 10:38:54 +0200 | Subject | Re: [PATCH net-next 6/6] net: hns3: use napi_consume_skb() when cleaning tx desc |
| |
On Wed, Sep 16, 2020 at 10:33 AM Saeed Mahameed <saeed@kernel.org> wrote: > > On Tue, 2020-09-15 at 15:04 +0800, Yunsheng Lin wrote: > > On 2020/9/15 13:09, Saeed Mahameed wrote: > > > On Mon, 2020-09-14 at 20:06 +0800, Huazhong Tan wrote: > > > > From: Yunsheng Lin <linyunsheng@huawei.com> > > > > > > > > Use napi_consume_skb() to batch consuming skb when cleaning > > > > tx desc in NAPI polling. > > > > > > > > Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com> > > > > Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com> > > > > --- > > > > drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 27 > > > > +++++++++++- > > > > ---------- > > > > drivers/net/ethernet/hisilicon/hns3/hns3_enet.h | 2 +- > > > > drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 4 ++-- > > > > 3 files changed, 17 insertions(+), 16 deletions(-) > > > > > > > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c > > > > b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c > > > > index 4a49a76..feeaf75 100644 > > > > --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c > > > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c > > > > @@ -2333,10 +2333,10 @@ static int hns3_alloc_buffer(struct > > > > hns3_enet_ring *ring, > > > > } > > > > > > > > static void hns3_free_buffer(struct hns3_enet_ring *ring, > > > > - struct hns3_desc_cb *cb) > > > > + struct hns3_desc_cb *cb, int budget) > > > > { > > > > if (cb->type == DESC_TYPE_SKB) > > > > - dev_kfree_skb_any((struct sk_buff *)cb->priv); > > > > + napi_consume_skb(cb->priv, budget); > > > > > > This code can be reached from hns3_lb_clear_tx_ring() below which > > > is > > > your loopback test and called with non-zero budget, I am not sure > > > you > > > are allowed to call napi_consume_skb() with non-zero budget outside > > > napi context, perhaps the cb->type for loopback test is different > > > in lb > > > test case ? Idk.. , please double check other code paths. > > > > Yes, loopback test may call napi_consume_skb() with non-zero budget > > outside > > napi context. Thanks for pointing out this case. > > > > How about add the below WARN_ONCE() in napi_consume_skb() to catch > > this > > kind of error? > > > > WARN_ONCE(!in_serving_softirq(), "napi_consume_skb() is called with > > non-zero budget outside napi context"); > > > > Cc: Eric > > I don't know, need to check performance impact. > And in_serving_softirq() doesn't necessarily mean in napi > but looking at _kfree_skb_defer(), i think it shouldn't care if napi or > not as long as it runs in soft irq it will push the skb to that > particular cpu napi_alloc_cache, which should be fine. > > Maybe instead of the WARN_ONCE just remove the budget condition and > replace it with > > if (!in_serving_softirq()) > dev_consume_skb_any(skb); >
I think we need to keep costs small.
So lets add a CONFIG_DEBUG_NET option so that developers can add various DEBUG_NET() clauses.
| |