Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <5e6859d3-d3e7-44c1-acee-2c4ec568615d@intel.com>
Date:   Mon, 27 Nov 2023 15:08:07 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v5 01/14] page_pool: make sure frag API fields
 don't span between cachelines
Content-Language: en-US
To:     Yunsheng Lin <linyunsheng@huawei.com>
CC:     Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Michal Kubiak <michal.kubiak@intel.com>,
        Larysa Zaremba <larysa.zaremba@intel.com>,
        Alexander Duyck <alexanderduyck@fb.com>,
        David Christensen <drc@linux.vnet.ibm.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        "Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
        Paul Menzel <pmenzel@molgen.mpg.de>, <netdev@vger.kernel.org>,
        <intel-wired-lan@lists.osuosl.org>, <linux-kernel@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        "Jakub Kicinski" <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
References: <20231124154732.1623518-1-aleksander.lobakin@intel.com>
 <20231124154732.1623518-2-aleksander.lobakin@intel.com>
 <9902d1c4-5e51-551a-3b66-c078c217c5ad@huawei.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <9902d1c4-5e51-551a-3b66-c078c217c5ad@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bG04Si9TdUZhVjZua1hXanNpdG9ZL2ROaThxUVNnQlJhVmlUQkNXVE83aXEv?=
 =?utf-8?B?a3I2UjFhMHNDSTFFYnY5ZnAxZHhCUFo0dDVHVVFldWdCMHN3NUw1MG80T3hD?=
 =?utf-8?B?cE9RSmlxTlM0R09kM2w1YlFUblNlUWFLeFlkV1hITWk1OHJ0RmpvZDQ2QXlX?=
 =?utf-8?B?eXNFMUcxNHpJNGJERzg0OGpkT2k0U2l3T3RsV3hZcUs0SWxQV2o1ZTVLbHAr?=
 =?utf-8?B?dXpVSzFhbitqcERRbFpNOVBoUFNTSFo0ZW1leTk2Z09pWG1OZlhlRjFPZkJH?=
 =?utf-8?B?Y25SV0lsWHdKcnc3MUxPT3V4ZDNnK0ZRQmJXVUFRNDBFdVJ3UkZ2NnJUS3Vk?=
 =?utf-8?B?T3E0VUs3SUhITWVCaytyNGMzdlNkaVRwZHYrRkljSno2UlgrR1NRampNYStW?=
 =?utf-8?B?dHJ1TFVKR1dzMkt2UEdnQXdpNkR0d0I1dlZWdkxTVE4xbWxGVzZRNW85VE1u?=
 =?utf-8?B?TFVxWmlXK0hvUjBUdnBlZXdBTTBvZTF5eGNJWFpmR0NrN1Q3eGwrQm81MEdu?=
 =?utf-8?B?ZThPdVVjY2Z4MVcrUzI1eldBTFlzOTlDek5TdVNVb01NK1hkTnRqL3BjVktK?=
 =?utf-8?B?ck91WUlOMTFnYTgwc1c5dGlON3Y3aXFyUlVjYnc3VGhhaXNmZyt2ZUFsSlJ4?=
 =?utf-8?B?NjJKZHkyVFRQL2hxczBCYTJDSm45elcxc3NzZDR6NlFlOHRpRXBLK2xBaUg2?=
 =?utf-8?B?c2ZyVlI4TjNjcGNVSE9xS2h6ejlvOGJONmVtamUraVZMbGczMXJXV2Vjb3VD?=
 =?utf-8?B?dStuT3YxUXFQS2VSaGVqRjMxVE8xZzIxQm1UUlQwZUsyU05nZVdaOUhOOGZJ?=
 =?utf-8?B?cEtkQ2lmOFNlZFQyVzJxV29ZTHNDVmNFczlpNjNDWGZDc0U3eDNpMkd6Y1k0?=
 =?utf-8?B?ckpZZlEzbzFlcmdxS1owVm9WekVPbFFIbHZrcU5QVjBpTVBydERhbXJGbFJO?=
 =?utf-8?B?dGtCdk51cU41KzBSUmZNWWNGdXdycVNHc0RwdzRjZ3lUdEowWldTUDVyeWVq?=
 =?utf-8?B?dzd1citIM1VvTzRCZ0pONmd4eTRJbHNjYXBPVGZsSkdINlk0N0xRUXRONHhO?=
 =?utf-8?B?a0dJeVZGU0xBeEswNjlOQTJ2M25HN2dtclpSUG9EanhpbVpLUU44NGZTbDUz?=
 =?utf-8?B?NXJQcTF2VTRNL0lScTJXaXNNVzB3WHRkR2g0WHBxb2hBN3JzcjNHdGdWUHRO?=
 =?utf-8?B?d1ZEMU8wSjNxamVqVGRqZXIvamtVVVNvMUkyWGNJdXlVNUd2NmdQSEhTc2Fy?=
 =?utf-8?B?KzE5amdXeHk5TGJnMFphbDJiaCtra2U3UmV2dkxNQytoSmx6aVE5VzgwMVRr?=
 =?utf-8?B?VjFEbnpZRncwWGdMZHdNd2FjTmdpLzh1Mld0alZBMlB0b0FKUmVIYUM1S1gx?=
 =?utf-8?B?VytheitNV0NLdmxKOFFGaFF1Wlp2Rm5GQU1XbU5GU3VsMVdRZldEV3dBaFRG?=
 =?utf-8?B?T1h1VFN1bDZvL2xiRHl1azVxSldIc0FCWWFHYzloSkJFOHZrMEYvTmJuMjcr?=
 =?utf-8?B?UmlIeFRuTGNiMmhNQ2pOOFhZUVg1VjZQOE9Zdjc0MnFtUGdkRStQWjlPRDhj?=
 =?utf-8?B?S2U4T0RtU0dzY2c3NmlxbGNHNUZic3R2NXBiN0hPTENpV3BhcUdGTmVJVDgy?=
 =?utf-8?B?M3p3K3M0K1kvL1RxRG42RVl3NkNvWDNnWk4zZitIblFhVUpabHpMTUlwa0Ev?=
 =?utf-8?B?dFhiQlYxUzNrRkRSRDlyUFRpdUNjMVphT0hPVTZySERSalo0MTM1UENSdnFN?=
 =?utf-8?B?ZnNxSlVQODlWS1lHdzNhRUhZakJ4aU11VnZzNXdXbmgrS3hMUzk1TnE0akVH?=
 =?utf-8?B?SzV1blBuYnh3SjZmNVVHSGZpTldHb0l2NHFzQWpyWjBFVWFDK3lLYldwQ1Ix?=
 =?utf-8?B?dlViY2xTdjdQNDFXdlVrdkd6RExEaTNyVU5EWGVGUXpUZjFMUk84SU54cmdl?=
 =?utf-8?B?ZlZ4L3NwcGpMdHdtZ21uUGp3dmlKT2hZdVV4ME8wZHo1VkpMenA5S3ZvQ1Mz?=
 =?utf-8?B?aGMyQTVvSWJpU0k4a3hHOTlvL0RFRUJITXBZc2EwdFV2ZWpaeU5HQnY0Zitn?=
 =?utf-8?B?RXlwQVU4bnpaUWU5NzNicmFKYldoaTZRc0RXNlZWZjFMTkRVV29oc0h4WmRT?=
 =?utf-8?B?b3JoZkpCbit4UlFQS2drSldkTmE0ZGx3WVgyM2REOHkrZWFqTTVjUVdZTzVl?=
 =?utf-8?B?MVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: dd4d57c9-b2ec-4d14-cd18-08dbef525891
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Nov 2023 14:08:35.7464
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: g6fm9WiaZnO6D9/oxdl21trncAURBFH/O8L69yXfETkw8xBZBSUb22Tge6jNyuw+EJx7tP/DjbsC/TKLtJBtMs+l63dwaO5F0p3DS93tv0s=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR11MB5570
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Mon, 27 Nov 2023 06:18:01 -0800 (PST)

From: Yunsheng Lin <linyunsheng@huawei.com>
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 <aleksander.lobakin@intel.com>
>> ---
>>  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.

> 
>>  	struct page *frag_page;
>>  	unsigned int frag_offset;
>>  	u32 pages_state_hold_cnt;
>>

Thanks,
Olek