From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 96828C433E6 for ; Sat, 13 Feb 2021 11:56:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5DEC764DEE for ; Sat, 13 Feb 2021 11:56:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229617AbhBMLyM (ORCPT ); Sat, 13 Feb 2021 06:54:12 -0500 Received: from mail-40131.protonmail.ch ([185.70.40.131]:51124 "EHLO mail-40131.protonmail.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229517AbhBMLyJ (ORCPT ); Sat, 13 Feb 2021 06:54:09 -0500 Date: Sat, 13 Feb 2021 11:53:13 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pm.me; s=protonmail; t=1613217205; bh=U9h7ecCA02bgXdLGxxosUvrTLcI+jIc/SZnrWbn3k90=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References:From; b=ZMq4DHhrTAzzviEoROqpXjW+tTJgetKNneTLYWBIlq1hjBTgqXgIdVKLHdwApgF7D v6L+0+0nn2UAdmV3DHCnIKrIcYo3rYVJfsas0IkWgFG1DFZ4ISVs+MUGiqZikiQ5cu zgqi1RJsyMOhvehrQ0f88EyM8+UGaK/FXcLBFOtbzEfdNQ50aP3QQzCevV27iIoNk3 lWwWv3QRAwYmKvAA823cwxis7+DpyMK6eB0dh9BfcGG0sZlxUwme2Y41HSFgbrci0Y +YCAKrVwxz45r9IAYqW+hXstZ1PBaISrHf+hxT5PA5XPhcCA3C91XpOaKUCTJBUl5y YyC7MAAwCewEg== To: Alexander Duyck From: Alexander Lobakin Cc: Alexander Lobakin , "David S. Miller" , Jakub Kicinski , Jonathan Lemon , Eric Dumazet , Dmitry Vyukov , Willem de Bruijn , Randy Dunlap , Kevin Hao , Pablo Neira Ayuso , Jakub Sitnicki , Marco Elver , Dexuan Cui , Paolo Abeni , Jesper Dangaard Brouer , Alexander Duyck , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Taehee Yoo , Cong Wang , =?utf-8?Q?Bj=C3=B6rn_T=C3=B6pel?= , Miaohe Lin , Guillaume Nault , Yonghong Song , zhudi , Michal Kubecek , Marcelo Ricardo Leitner , Dmitry Safonov <0x7f454c46@gmail.com>, Yang Yingliang , Florian Westphal , Edward Cree , LKML , Netdev Reply-To: Alexander Lobakin Subject: Re: [PATCH v5 net-next 09/11] skbuff: allow to optionally use NAPI cache from __alloc_skb() Message-ID: <20210213115229.2728-1-alobakin@pm.me> In-Reply-To: References: <20210211185220.9753-1-alobakin@pm.me> <20210211185220.9753-10-alobakin@pm.me> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Alexander Duyck Date: Thu, 11 Feb 2021 19:18:45 -0800 > On Thu, Feb 11, 2021 at 11:00 AM Alexander Lobakin wrote= : > > > > Reuse the old and forgotten SKB_ALLOC_NAPI to add an option to get > > an skbuff_head from the NAPI cache instead of inplace allocation > > inside __alloc_skb(). > > This implies that the function is called from softirq or BH-off > > context, not for allocating a clone or from a distant node. > > > > Signed-off-by: Alexander Lobakin > > --- > > net/core/skbuff.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > > index 9e1a8ded4acc..a0b457ae87c2 100644 > > --- a/net/core/skbuff.c > > +++ b/net/core/skbuff.c > > @@ -397,15 +397,20 @@ struct sk_buff *__alloc_skb(unsigned int size, gf= p_t gfp_mask, > > struct sk_buff *skb; > > u8 *data; > > bool pfmemalloc; > > + bool clone; > > > > - cache =3D (flags & SKB_ALLOC_FCLONE) > > - ? skbuff_fclone_cache : skbuff_head_cache; > > + clone =3D !!(flags & SKB_ALLOC_FCLONE); >=20 > The boolean conversion here is probably unnecessary. I would make > clone an int like flags and work with that. I suspect the compiler is > doing it already, but it is better to be explicit. >=20 > > + cache =3D clone ? skbuff_fclone_cache : skbuff_head_cache; > > > > if (sk_memalloc_socks() && (flags & SKB_ALLOC_RX)) > > gfp_mask |=3D __GFP_MEMALLOC; > > > > /* Get the HEAD */ > > - skb =3D kmem_cache_alloc_node(cache, gfp_mask & ~__GFP_DMA, nod= e); > > + if ((flags & SKB_ALLOC_NAPI) && !clone && >=20 > Rather than having to do two checks you could just check for > SKB_ALLOC_NAPI and SKB_ALLOC_FCLONE in a single check. You could just > do something like: > if ((flags & (SKB_ALLOC_FCLONE | SKB_ALLOC_NAPI) =3D=3D SKB_ALLOC_NAP= I) >=20 > That way you can avoid the extra conditional jumps and can start > computing the flags value sooner. I thought about combined check for two flags yesterday, so yeah, that probably should be better than the current version. > > + likely(node =3D=3D NUMA_NO_NODE || node =3D=3D numa_mem_id(= ))) > > + skb =3D napi_skb_cache_get(); > > + else > > + skb =3D kmem_cache_alloc_node(cache, gfp_mask & ~GFP_DM= A, node); > > if (unlikely(!skb)) > > return NULL; > > prefetchw(skb); > > @@ -436,7 +441,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_= t gfp_mask, > > __build_skb_around(skb, data, 0); > > skb->pfmemalloc =3D pfmemalloc; > > > > - if (flags & SKB_ALLOC_FCLONE) { > > + if (clone) { > > struct sk_buff_fclones *fclones; > > > > fclones =3D container_of(skb, struct sk_buff_fclones, s= kb1); > > -- > > 2.30.1 Thanks, Al