From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-178.mta1.migadu.com (out-178.mta1.migadu.com [95.215.58.178]) (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 B8CFE1F4634 for ; Fri, 10 Jul 2026 05:18:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783660708; cv=none; b=oceSimrgHs2wBHWRFIqcsmEtXFLRh2UWCJv4HG+6z32iivni20/x78F4KpvXXl4aaeq6gOEMh906PTADr2FkSZsiyDXtNoL9ooMfVkpQekOJUShKPc56h1iBbEqXFL2fL49Xz/TB7PcpP+r622wYVz4fKn6Tt2DpmXWCiYXKjv4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783660708; c=relaxed/simple; bh=nt2ucK3eYfbv9y4oN0lNeZeFbKsB6C5yZJ3cvawiYlI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JNXaT2VynlcVXfp3qu39Om7OofmTZ5uhdrQPBNy1gVHecuiTMaJzSnMMkvBsOeZAfoaM5cfi3HAUiOww80/6LB7EtDrh8CAwcP5DiappvRaEX2mmVBbeVZMydbY7CbjBcz2oCYr8XzgI1oWB5XH1XAumh+/WUnms++JRLjjUw3c= 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=R1q9pxij; arc=none smtp.client-ip=95.215.58.178 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="R1q9pxij" Date: Fri, 10 Jul 2026 13:18:16 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1783660705; 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=PhKD875aUeJrB1VfoaFiq3+jQv2LC7XAlpRGxkj4Sxg=; b=R1q9pxij0lYyjsgLHagWWARkOhYP2VklVTrV5AT+tVG6+/9jGFhprKn+6kQFDOgFtbktTe IH6T1MuVwoCtL0hP9Ix2PiLFGPY1PGcdqlm6rmaa++M05WaET8MJPXD+N1FyVjBKzngeQy EuS1TX9NJHrLjoXyWDa2nIy+MTkQzyc= 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: <20260707-slab-simplify-bulk-pcs-v1-1-4850dbe0d904@kernel.org> X-Migadu-Flow: FLOW_OUT 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! :) > > -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