From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AF902233948 for ; Mon, 6 Jul 2026 02:51:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783306286; cv=none; b=qy9ilFVw62Cl+1bRpvSaQjJ/tFKrw6F+hAT2cD/tuqwh4O+BCfWcuq18gg/A+tSUWXLD5OVV00cp7bLk+u/40VNZ/oBlL3It6n0PyZ5HmxIcMedRm0mrzdo3CUxZHt7phIRQimXgWwJs5o6zWA2/w4Z6L6lNqB0VT7RWGDBL/34= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783306286; c=relaxed/simple; bh=+yAwIOhw7KV47T575yIBaKh7bCY2nTW9hlHRvmmMX98=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=ZvFYuTW1FyzPVRyHTpaAX4BQjbP+bOhP3G1t8hqyeDNXY/+qaSDEiVZoycY49inFcB7qN9aW9brcSY91qNBDOspgozrhqRDSw0ObGA8Afc6Qi3Z01EEFj5jgo2o3A0hcBnefKZ8vemvQbgphUEHwjUXdqQO3jmFWKj5HCHkS/PE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=W/IYSxhd; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="W/IYSxhd" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2B6671F000E9; Mon, 6 Jul 2026 02:51:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783306285; bh=Mwv5psBFonUnPcaVrRzWoLiOwCo4Fa1oDMIpZW78pFs=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=W/IYSxhd7T9rI+2RqBN3gFYAXrd8lEB5tRTHqdPKm0UPnsqN4b5UsZ03afzNRsZ4E Ut5P4SSVTLCw9W1e4G/2S4wpwmilXLONTxI1+PMsSYrVzXQ0obAb6m/5wG9R4wHgB9 swfcqH1UwabrUIuOMA2/PgSN2mxK3Sy0HozwAJh/tPpwhrKaTtz/f7lu415XN6dfrS +4YsBDK/XNI5tReqjyyWdpYt9NpC0sIg4hCBkCg7/JN/pBrz9ij74idTq12X1SVnJW YgzcBfLWkCv5KQy0W+KJefGR7kF/EefIpD+1Zha7ErGbSiOxp2fIroB8sKn8DHa02k DhE7xTtruxr6w== Message-ID: <026e7be6-fa97-46b0-8daf-57fb890db0b4@kernel.org> Date: Mon, 6 Jul 2026 11:51:18 +0900 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm/slub: fix lost local objects when bulk remote free batch fills To: hu.shengming@zte.com.cn Cc: vbabka@kernel.org, akpm@linux-foundation.org, hao.li@linux.dev, cl@gentwo.org, rientjes@google.com, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, zhang.run@zte.com.cn, cai.qu@zte.com.cn References: <20260705213758375Xf8iZzfLz92U9SASpvqTJ@zte.com.cn> Content-Language: en-US From: Harry Yoo In-Reply-To: <20260705213758375Xf8iZzfLz92U9SASpvqTJ@zte.com.cn> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------U3q03QC08IG6dyOa4TvhWlP0" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --------------U3q03QC08IG6dyOa4TvhWlP0 Content-Type: multipart/mixed; boundary="------------if0In2NWBbK2DDupQtZkI3Sc"; protected-headers="v1" From: Harry Yoo To: hu.shengming@zte.com.cn Cc: vbabka@kernel.org, akpm@linux-foundation.org, hao.li@linux.dev, cl@gentwo.org, rientjes@google.com, roman.gushchin@linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, zhang.run@zte.com.cn, cai.qu@zte.com.cn Message-ID: <026e7be6-fa97-46b0-8daf-57fb890db0b4@kernel.org> Subject: Re: [PATCH] mm/slub: fix lost local objects when bulk remote free batch fills References: <20260705213758375Xf8iZzfLz92U9SASpvqTJ@zte.com.cn> In-Reply-To: <20260705213758375Xf8iZzfLz92U9SASpvqTJ@zte.com.cn> --------------if0In2NWBbK2DDupQtZkI3Sc Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 7/5/26 10:37 PM, hu.shengming@zte.com.cn wrote: > Harry wrote: >> Hi Shengming, >=20 > Hi Harry, >=20 >> On 7/4/26 5:04 PM, hu.shengming@zte.com.cn wrote: >>> From: Shengming Hu >>> >>> In free_to_pcs_bulk(), when remote_objects[] fills to PCS_BATCH_MAX, >>> the code jumps to flush_remote to free the batch. If all remote entri= es >>> have already been compacted out of p[] via tail swaps while local obj= ects >>> remain, the flush_remote path returns early since `i < size` no longe= r >>> holds. The leftover local objects are then neither cached in the shea= f=20 >>> nor returned to the slab freelist, causing a memory leak. >> >> Ouch. >> >> Interestingly, we fixed a similar memory leak in free_to_pcs_bulk() >> during the v6.18 cycle; commit cbcff934fa7d ("mm/slub: fix memory leak= >> in free_to_pcs_bulk()"). >> >> Hmm, I feel is a signal that the control flow is quite complicated and= >> worth revisiting if we could simplify the logic there. I'm not saying = we >> should do that to fix the bug, just mentioning. >=20 > Yes, this is also a memory leak. I agree that the control flow in > free_to_pcs_bulk() is fairly subtle, especially considering the previou= s > leak fix. >=20 > My intention with this patch is only to remove this problematic path wi= th > a localized change. That's fair! >>> For illustration: >>> size =3D 64, local objects at p[0..31], remote objects at p[32..63] >>> After scanning all remotes: i =3D 32, size =3D 32 >>> p[0..31] local objects are dropped. >> >> I can see that could indeed happen. >> Interesting. How has it been undiscovered until today? >> >> Well, for this to trigger, at least PCS_BATCH_MAX (hardcoded to 32) >> objects in the sheaf should be from remote nodes. >> >> Looking at kmem_cache_free_bulk() users: >> - maple_node has sheaf_capacity =3D 32 >> - skbuff_head_cache has sheaf_capacity =3D 28 >> - panthor and msm drivers have sheaf_capacity =3D 4 >> >> The capacity is (at least for now) purely based on the object size >> (with user-requested capacity as a minimum) >> >> Only maple_node cache (out of four users) has >> sheaf_capacity >=3D PCS_BATCH_MAX. >> >> However, for this to trigger in maple_node cache, ALL objects in the >> sheaf should have been from remote nodes. And that means there is no >> local object to leak :) >> >> So, although the logic has a leak, you cannot trigger this in reality = yet. >> >=20 > Thanks for the detailed analysis! >=20 > Yes, this was found by code review rather than from a runtime report, a= nd > I can not verified it with an actual triggering workload. > > I agree that it does not seem triggerable with the current users. This = fix > is mainly intended to make the code robust against possible future user= s or > changes to PCS_BATCH_MAX :) Could we have these explanations/analysis above in the commit message please? :) But yeah, we should fix this. When fixing a bug usually I prefer to go through the process... 1) Confirm the bug exists by reproducing it, and 2) Confirm that the patch indeed fixes the bug But this one is impossible to reproduce, uh. In this case I think it's better not to Cc stable (which is already the case). In case there is an error in the analysis, it's still straightforward to backport later. I will double check if the patch is indeed correct. Thanks! >>> Replace the goto-based flush with an in-place free of full remote bat= ches >>> during the scan, then resume processing the compacted array. All loca= l >>> objects now go through the normal fast path, and the tail path only >>> handles any leftover partial remote batch. The redundant next_remote_= batch >>> jump label is removed as well. >>> >>> Fixes: <989b09b73978>("slab: skip percpu sheaves for remote object fr= eeing") >>> Signed-off-by: Shengming Hu