From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=none Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8D0EA19A4; Tue, 28 Nov 2023 18:55:03 -0800 (PST) Received: from dggpemm500005.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Sg3nf1VjFzvRHv; Wed, 29 Nov 2023 10:54:30 +0800 (CST) Received: from [10.69.30.204] (10.69.30.204) by dggpemm500005.china.huawei.com (7.185.36.74) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Wed, 29 Nov 2023 10:55:01 +0800 Subject: Re: [PATCH net-next v5 01/14] page_pool: make sure frag API fields don't span between cachelines To: Alexander Lobakin CC: Maciej Fijalkowski , Michal Kubiak , Larysa Zaremba , Alexander Duyck , David Christensen , Jesper Dangaard Brouer , Ilias Apalodimas , Paul Menzel , , , , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni References: <20231124154732.1623518-1-aleksander.lobakin@intel.com> <20231124154732.1623518-2-aleksander.lobakin@intel.com> <9902d1c4-5e51-551a-3b66-c078c217c5ad@huawei.com> <5e6859d3-d3e7-44c1-acee-2c4ec568615d@intel.com> From: Yunsheng Lin Message-ID: Date: Wed, 29 Nov 2023 10:55:00 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 In-Reply-To: <5e6859d3-d3e7-44c1-acee-2c4ec568615d@intel.com> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpemm500005.china.huawei.com (7.185.36.74) X-CFilter-Loop: Reflected On 2023/11/27 22:08, Alexander Lobakin wrote: > From: Yunsheng Lin > Date: Sat, 25 Nov 2023 20:29:22 +0800 > >> On 2023/11/24 23:47, Alexander Lobakin wrote: >>> After commit 5027ec19f104 ("net: page_pool: split the page_pool_params >>> into fast and slow") that made &page_pool contain only "hot" params at >>> the start, cacheline boundary chops frag API fields group in the middle >>> again. >>> To not bother with this each time fast params get expanded or shrunk, >>> let's just align them to `4 * sizeof(long)`, the closest upper pow-2 to >>> their actual size (2 longs + 2 ints). This ensures 16-byte alignment for >>> the 32-bit architectures and 32-byte alignment for the 64-bit ones, >>> excluding unnecessary false-sharing. >>> >>> Signed-off-by: Alexander Lobakin >>> --- >>> include/net/page_pool/types.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h >>> index e1bb92c192de..989d07b831fc 100644 >>> --- a/include/net/page_pool/types.h >>> +++ b/include/net/page_pool/types.h >>> @@ -127,7 +127,7 @@ struct page_pool { >>> >>> bool has_init_callback; >> >> It seems odd to have only a slow field between tow fast >> field group, isn't it better to move it to the end of >> page_pool or where is more appropriate? > > 1. There will be more in the subsequent patches. > 2. ::has_init_callback happens each new page allocation, it's not slow. > Jakub did put it here for purpose. > >> >>> >>> - long frag_users; >>> + long frag_users __aligned(4 * sizeof(long)); >> >> If we need that, why not just use '____cacheline_aligned_in_smp'? > > It can be an overkill. We don't need a full cacheline, but only these > fields to stay within one, no matter whether they are in the beginning > of it or at the end. I am still a little lost here, A comment explaining why using '4' in the above would be really helpful here.