From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-172.mta0.migadu.com (out-172.mta0.migadu.com [91.218.175.172]) (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 A36EB1A6831 for ; Fri, 10 Jul 2026 05:21:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783660898; cv=none; b=dweuE0v514VTpnaokH/tWxEMTnLyz0moCXIWSl6kN+VqQ6ReYkplGL6WCR1RFbsZQxtDuWNBoAvTDesINVY74XLt5X0gZRYsKYlmeJbvnODInwIA9xHG4HD45fSn3i+NmEyi06EF9uRMm6D9tychvbNDBhdOe24EgPdAr31zqtk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783660898; c=relaxed/simple; bh=ZxYqNg/5GW6zbLnnu3AMS5xBGYhN9mppm7ZZ9WsnBFk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QK/7xVOT13/lGM4A1E+d2cEYudilLSILQUPeGy6dx8FV0tKh38DAw2p3KeJqWavotEYPeU7IS+vl0zkt2XDzPi/a+AmRgyZasML87DPFhS6AOiLU4V/dYksFKpv2zodUFWvwHEZqKg8kFsZgxCdhkeXovGG0mi/Ht+0QvdXwNfI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=WBis7U+m; arc=none smtp.client-ip=91.218.175.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="WBis7U+m" Date: Fri, 10 Jul 2026 13:21:27 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1783660894; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=S/41ri3xvxu3vsNe/Fvn4VJ+MCvfys4SQAyvquWjtKE=; b=WBis7U+mrG+jKIVHHy7VOdGsiYMs8W1kD3FRqsNzcKblgGrsyqu8lkXT5S+3PJk6+ocl3Y 14XG6CVzz32dolugPBP9M4+IGmX6/YMC5sZnk/e1LkumrE4imWr3sGPX9V8Td6mNwDQdlT rf/g6ySQ79tY2TnXF1ihz61A6FbUi7E= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Hao Li To: "Vlastimil Babka (SUSE)" Cc: Harry Yoo , Andrew Morton , Shengming Hu , Christoph Lameter , David Rientjes , Roman Gushchin , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/slab: extract __free_to_pcs_batch() from free_to_pcs_bulk() Message-ID: References: <20260707-slab-simplify-bulk-pcs-v1-1-4850dbe0d904@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT On Fri, Jul 10, 2026 at 01:18:25PM +0800, Hao Li wrote: > On Tue, Jul 07, 2026 at 02:16:00PM +0200, Vlastimil Babka (SUSE) wrote: > > It has been noted that free_to_pcs_bulk() is difficult to follow, with a > > number of goto labels, and this has contributed to two memory leak bugs > > in there. > > > > Extract part of the code to __free_to_pcs_batch(), which focuses only on > > freeing free-hook-processed local objects to a percpu sheaf, and > > returning how many were freed. Zero means a trylock failure or no empty > > sheaf available, and thus the caller should fallback to > > __kmem_cache_free_bulk(). > > > > Make free_to_pcs_bulk() call this in a while loop, removing all goto > > labels from the function. __free_to_pcs_batch() retains two rather > > straightforward ones. > > > > Signed-off-by: Vlastimil Babka (SUSE) > > Nice simplification! > > [...] > > +/* > > + * Bulk free objects to the percpu sheaves. > > + * Unlike free_to_pcs() this includes the calls to all necessary hooks > > + * and the fallback to freeing to slab pages. > > + */ > > +static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p) > > +{ > > + bool init = slab_want_init_on_free(s); > > + void *remote_objects[PCS_BATCH_MAX]; > > + unsigned int remote_nr = 0; > > + > > + for (unsigned int i = 0; i < size;) { > > + struct slab *slab = virt_to_slab(p[i]); > > + > > + memcg_slab_free_hook(s, slab, p + i, 1); > > + alloc_tagging_slab_free_hook(s, slab, p + i, 1); > > + > > + if (unlikely(!slab_free_hook(s, p[i], init, false))) { > > + p[i] = p[--size]; > > + continue; > > + } > > + > > + if (unlikely(!can_free_to_pcs(slab))) { > > + remote_objects[remote_nr] = p[i]; > > + p[i] = p[--size]; > > + if (++remote_nr >= PCS_BATCH_MAX) { > > + __kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]); > > + stat_add(s, FREE_SLOWPATH, remote_nr); > > + remote_nr = 0; > > + } > > + continue; > > + } > > + > > + i++; > > } > > > > - if (remote_nr) > > - goto flush_remote; > > + while (size) { > > + unsigned int batch_freed = __free_to_pcs_batch(s, size, p); > > > > - return; > > + if (!batch_freed) > > + break; > > > > -no_empty: > > - local_unlock(&s->cpu_sheaves->lock); > > + p += batch_freed; > > + size -= batch_freed; > > + } > > > > - /* > > - * if we depleted all empty sheaves in the barn or there are too > > - * many full sheaves, free the rest to slab pages > > - */ > > -fallback: > > - __kmem_cache_free_bulk(s, size, p); > > - stat_add(s, FREE_SLOWPATH, size); > > + if (size) { > > + __kmem_cache_free_bulk(s, size, p); > > + stat_add(s, FREE_SLOWPATH, size); > > + } > > By the way, could we move this directly into the loop inside the > `if (!batch_freed)` block to make it a bit more concise? but it's totally fine > to leave it as is! :) oops, forgot to add the r-b tag just now :P Reviewed-by: Hao Li > > > > > -flush_remote: > > if (remote_nr) { > > __kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]); > > stat_add(s, FREE_SLOWPATH, remote_nr); > > > > --- > > base-commit: 72bb229f9161a1efcd5df32141b69fcc6ae81a13 > > change-id: 20260707-slab-simplify-bulk-pcs-a8d0478feef6 > > > > Best regards, > > -- > > Vlastimil Babka (SUSE) > > -- Thanks, Hao