Received-SPF: pass (google.com: domain of linux-kernel+bounces-66960-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <8aa809c0-585f-4750-98d4-e19165c6ff73@intel.com>
Date: Thu, 15 Feb 2024 14:12:41 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next] page_pool: disable direct recycling based on
 pool->cpuid on destroy
Content-Language: en-US
To: =?UTF-8?Q?Toke_H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>, "Lorenzo
 Bianconi" <lorenzo@kernel.org>
CC: "David S. Miller" <davem@davemloft.net>, Eric Dumazet
	<edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni
	<pabeni@redhat.com>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20240215113905.96817-1-aleksander.lobakin@intel.com>
 <87v86qc4qd.fsf@toke.dk>
From: Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <87v86qc4qd.fsf@toke.dk>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RjRvd09ocDcvRlVuWitiK21abExwYkJFWWErc2VqWjlOTE1iQkMrWnV2cWYw?=
 =?utf-8?B?S0FpQmZQQnc4Y3AxSVRETlhyUDZnMzYrYVBUY0xiTFNFUGxkQmV5SWJnU20w?=
 =?utf-8?B?akN4eWROczc2eHIzWVl0d3h1a0ZlSEFBZFcyWExRaGo1VndPem1YYUthR0J4?=
 =?utf-8?B?SGMvZUx0aWZnWDBsdnMxZCtVTk1Oc3hJNTZCNkh2Q1c3NGZwaDYzdkQzbnZB?=
 =?utf-8?B?RHEvMHZ4YXNHenhEL2NGc2duZEcwVXUrcDZoYjlLY0c0Wm5wNGpXQ2xxZ1RO?=
 =?utf-8?B?VmJBV3NuY2VhUVVmSW1ETjVYdzBlZHpFR1RVazNLM1hhNVpSbXVvNTVwYXJs?=
 =?utf-8?B?dFdSL1FBWWVxbS83YVJpVmlMZVcwRUsrT2kzL1lOTk12SUlBdUsvQlUzOFpn?=
 =?utf-8?B?cVRUL0hXN0crU0tHTjBPMUtRNE5GRHpER3k5OURnMzFLSzV1U0RkSjRJTnNN?=
 =?utf-8?B?NndCRkphNGlNV2dVb3BNdTZldWhGTDNjaERxWEdBL0ZsdklRSjloM1Y1QXls?=
 =?utf-8?B?MzR0T2xnMFoycjJ2VjFoWUVWd2FBQml1dnNHSU9jL3dEdmNwWmtyQWFNaGdj?=
 =?utf-8?B?b2FSZ3gvSGw1NFVFbWFCSmZqeStxL0RlZ0R2dlJoWEgyWkZkTmVnRzUxSnBm?=
 =?utf-8?B?bDNJd2pJRjdzVmd0dTA5ZUdhUEE3UU9RSE44SisrS3ZEVUpmakxzSHBWMXlD?=
 =?utf-8?B?SzBUbzdVSktmcGhaSTI4RDVYWVRWeTM4cG05Ylgzb01WQlNVTzVMbVI0dmU3?=
 =?utf-8?B?czFpbXpBN0lpcTlvU3pIamZ4N3JxRTl6ODlVbjIwNnFFdGZndEdIUFdjZlpH?=
 =?utf-8?B?N2pSamFEUkxtdEYrdElDV3VIZmkrMjMyMUlvWU1zQ2dQbUMzcWM3bFUxN3BP?=
 =?utf-8?B?aEo4Kzgwd3hvUjBYV3lKU0V1ditIZG1nSDF5b0VCSi94VzM0bTZRazVSVHJz?=
 =?utf-8?B?VjFWNTdRT0lKRk5jVk8zUG9HSC9jMEtjNUZkR2xWWjJMTTZTNTcwSUJzSVFa?=
 =?utf-8?B?UlEwL2VEQnF3aWlzQ3FSYTVlZzdkZys4bkNndEl3cGN2UmQ3STQxa2ZjcFVB?=
 =?utf-8?B?MWxqRXJrOGRlYURlREVGM3FpNG82MUM1Nll5a1Jzd3ltQTY3T3FIQ0NldTFB?=
 =?utf-8?B?YmgrNEx4Y29XdTUwVGtxaDV1czRuMWZBTjlmbUpIY3FrczJ3aHY2cmZLZjUr?=
 =?utf-8?B?Y2pCQ0dHNTd6bnZiR0NHc2k5U05ScGZjTkpLSUNRNU1HWjNOakJqZFVGMXhv?=
 =?utf-8?B?TFFjeVBvbXdsU2pCU1ovWGFZNG00b1B2bHhTVytFQlllcVRkU3hkMXp2Qmdh?=
 =?utf-8?B?a0lvNlNpSmZ4OSt0dXFOa3JJejZRVkJpOUNHaksxd1dmM1FTc1FGWG9EeXRJ?=
 =?utf-8?B?S1p4QWhkK0NGYzJOMDduaTFPN3oxQUZCSk1wWEoySEhNRDMrbEFMa2Z2SkR6?=
 =?utf-8?B?Qlg3M3RjbUJGY0JZa0NJNmNvWWdLbUppWnpKUWR4Vzh4YWZFb3BjbmIrb0J1?=
 =?utf-8?B?RlAyeU0rNEFuNWFGNVhyMWpWWTBmb3JjWG5QR0gzM3pIYjlnVDhZem9DQ0lv?=
 =?utf-8?B?ZCtpK0d4S0J6L0xjY21TaExSaGE2eWY1SlV4dXowa1dFaThMWE94bkxsTHh3?=
 =?utf-8?B?TThKNGFUVVVIc0xKQkNXc1JBZFR5c09kbzFBQURJODJFTEs5UW1HZ3RnTWlt?=
 =?utf-8?B?aTQxS1BWTEZpS0t1U1ZVNTh1QzQ5eW91c2lsV3A2MkFUcG9VMUEwZkQzajVl?=
 =?utf-8?B?MkdCbFI4WExYYjhibkgxQVdSYm5BQ1F1c2pGanJmQ1pJWmxncXlpRFZIblEz?=
 =?utf-8?B?djRuMERkZ3o0K3gzTFJHNVRVcVlrUjcrSGdKMFBiUWRRU2tjNVcwNFh2Ykxs?=
 =?utf-8?B?SE9pbGRiLzhBcWRINWxjYytJRFNJNmQyK2VGcThBdklpc3QwOHJNckhBbXMr?=
 =?utf-8?B?c3ZmSHBuZjA2Y0cveGM5VUZGQTJMK0hQaFhZYVFKanRsUVEvK0xsZ3pKSG9I?=
 =?utf-8?B?U1E5cE5FMUZWamlNTGVpb1dlODBmUXgyaDB3T1ZuSkljckg0MGIxbTRuR1Fx?=
 =?utf-8?B?RERsWE1LZS9zV1ltV2RrZ0x2QU5SMzFBanRILzNCTVo0S3hML2E5aHljUnhs?=
 =?utf-8?B?QjN4OUFjQUI2ZktlbVBvNnBRamRNMVFwcGxFb0xPQkdaQWZOc1N0SFlHUldx?=
 =?utf-8?B?QXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9c4a7720-2b7a-4e93-f088-08dc2e27ccdd
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB8718.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Feb 2024 13:12:45.7841
 (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: MBnXiZvclLxJyuLVMkqA3hIM5pZIXsTqwLX1VIMAnFGYxQQSm6ajd4UoxKSHWWxm0Ld2D30Z+mtLMi4gPj485oWZFWHhROR5zaH4dELBrb4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6727
X-OriginatorOrg: intel.com

From: Toke Høiland-Jørgensen <toke@redhat.com>
Date: Thu, 15 Feb 2024 13:05:30 +0100

> Alexander Lobakin <aleksander.lobakin@intel.com> writes:
> 
>> Now that direct recycling is performed basing on pool->cpuid when set,
>> memory leaks are possible:
>>
>> 1. A pool is destroyed.
>> 2. Alloc cache is emptied (it's done only once).
>> 3. pool->cpuid is still set.
>> 4. napi_pp_put_page() does direct recycling basing on pool->cpuid.
>> 5. Now alloc cache is not empty, but it won't ever be freed.
> 
> Did you actually manage to trigger this? pool->cpuid is only set for the
> system page pool instance which is never destroyed; so this seems a very
> theoretical concern?

To both Lorenzo and Toke:

Yes, system page pools are never destroyed, but we might latter use
cpuid in non-persistent PPs. Then there will be memory leaks.
I was able to trigger this by creating bpf/test_run page_pools with the
cpuid set to test direct recycling of live frames.

> 
> I guess we could still do this in case we find other uses for setting
> the cpuid; I don't think the addition of the READ_ONCE() will have any
> measurable overhead on the common arches?

READ_ONCE() is cheap, but I thought it's worth mentioning in the
commitmsg anyway :)

> 
> -Toke
> 

Thanks,
Olek