From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3E819C28B30 for ; Wed, 12 Mar 2025 14:58:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A0C34280002; Wed, 12 Mar 2025 10:58:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9BD21280001; Wed, 12 Mar 2025 10:58:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 81031280002; Wed, 12 Mar 2025 10:58:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 59F4C280001 for ; Wed, 12 Mar 2025 10:58:05 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id C97698102E for ; Wed, 12 Mar 2025 14:58:05 +0000 (UTC) X-FDA: 83213204130.24.352273F Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131]) by imf18.hostedemail.com (Postfix) with ESMTP id 465851C000A for ; Wed, 12 Mar 2025 14:58:03 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=bUAecovx; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=SMYvXAmW; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=pXsFZoM5; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=scuwjW2o; spf=pass (imf18.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741791483; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=x7WD2XsTpzTFKoZOc3nhZIXrM4cIYiYfZH3pyvx6hMI=; b=Knz3IWV1XPNUBHsKHXuxS66H0yDzVE9mFTSzGI5avEva1X29ZCfo+4vCo6C7tCxA1NnhGU IHu0e25p2aLxTVTJMa0STbWCqRyAO5sIgfCdWHvOufuy7D8TIlgXaO3f6I0aytHqRujSZN ohkxOgepyjU0IaV8+OGNETHxb78/+VQ= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741791483; a=rsa-sha256; cv=none; b=HIeEt5ltpRXkZAq50SLNTBYk8fvGXRzUMaPg0wlwWnbSp7D17WT/FbLQMK/ZIxRsK+Sh4d 8aAkeujM/+0T2GVF3X4ONjUApteSji1WpqGb95L6P/Z7upYYd95GRBOZIRHq1Cfkwd561a RrU40KiUO+yxWsVOHtfrv4blSkQfe7A= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=bUAecovx; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=SMYvXAmW; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=pXsFZoM5; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=scuwjW2o; spf=pass (imf18.hostedemail.com: domain of vbabka@suse.cz designates 195.135.223.131 as permitted sender) smtp.mailfrom=vbabka@suse.cz; dmarc=none Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 410451F388; Wed, 12 Mar 2025 14:58:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1741791481; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=x7WD2XsTpzTFKoZOc3nhZIXrM4cIYiYfZH3pyvx6hMI=; b=bUAecovxufFsvQxggDCCx8Y3DyZk+rNGotjoDo6HIb0ITUEkWAQgEMt8zcVqvu8fuj8/Kx Bt0wotsAJFBMx0hr8DyJXQk/rRZ+aZYybsiqR26MIO4F78tNSn4Bzn2VvJyFr1+wpol0AT cT0AzVn23PX7NG2Qj3QI0fxlg2KF9W8= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1741791481; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=x7WD2XsTpzTFKoZOc3nhZIXrM4cIYiYfZH3pyvx6hMI=; b=SMYvXAmW1g3TNwwTc4XM8onhkEEi2xT9rKWJCM6PiUASWWYirgzwKlOVhyBF+1ulO+1Y0k 0p7IEvNggRmYj3CA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1741791480; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=x7WD2XsTpzTFKoZOc3nhZIXrM4cIYiYfZH3pyvx6hMI=; b=pXsFZoM5XCoHDOisVf0wbfSWsX/ZkkbLTWW1KnH2cqkTVDWQvkeoQWAMZujvwWRqFhVwx+ Nkql1e2hcyKK+tnZmyLCCfoZBWcYijaQpiK3UXe4vm3j4kAElqhKQLplCGpqKMOAruhKgX Ul13dIjsNLL0L6QzMztH6UgDIObajcc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1741791480; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=x7WD2XsTpzTFKoZOc3nhZIXrM4cIYiYfZH3pyvx6hMI=; b=scuwjW2o9N+zQ6gvHgqJ11+GAWNBVKBW1YbwQpB1ElgyO9+YR4CoOtVwGcYKy1IqpdzasI yqjYVUlIzLgE4OCA== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 26061132CB; Wed, 12 Mar 2025 14:58:00 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id bmnNCPig0WdOAgAAD6G6ig (envelope-from ); Wed, 12 Mar 2025 14:58:00 +0000 Message-ID: Date: Wed, 12 Mar 2025 15:57:59 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RFC v2 01/10] slab: add opt-in caching layer of percpu sheaves Content-Language: en-US To: Suren Baghdasaryan Cc: "Liam R. Howlett" , Christoph Lameter , David Rientjes , Roman Gushchin , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Uladzislau Rezki , linux-mm@kvack.org, linux-kernel@vger.kernel.org, rcu@vger.kernel.org, maple-tree@lists.infradead.org References: <20250214-slub-percpu-caches-v2-0-88592ee0966a@suse.cz> <20250214-slub-percpu-caches-v2-1-88592ee0966a@suse.cz> From: Vlastimil Babka In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Action: no action X-Rspam-User: X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: 465851C000A X-Stat-Signature: trairptp9cajs3wzh8a16zwo535p7369 X-HE-Tag: 1741791483-193836 X-HE-Meta: U2FsdGVkX1+N/SI2ymEFXfspzDJd3m/2mhvllEUCQrcbVl7HSoXSYpbanvReKVtxQUVNu74UqL4f5990wqncRffYkOm0x23XovpSFQESGRXcs86sVh5+5QRjb/V3f25e/HOxOJVEAhuw8RmeNcG8hkO+aaQ5jHGR6ZIQ7ztl1L8J7xXtHovbhHQ9kNux/sVaALpOdLdTWCP4KUDZfAzWnKZRs0fvhuMdkexcH97yHMAe/k5bRVTiZXc9zmi9I8SzciQ/f66UwcvkD1TsAMgxo5kdbQCvIS0Zq5CuVQKLSAoTBkE4hFWeDtg9fc4FW1KMMQDAknjkaGRs96kafJaswCVmn1EpuR5zJxeSlZqL/EEDAvbGxY8yTODsSaqtSJ+WQwiAcUcyJfbDVsiJIfLxnz4sQm2uX4MDCjfxIXeWqiOsPDXRR5+mDlC4pdHI64OL9TvMH+8cypTKumtnCd6Mjsv/jlyXoOTV/dskdd2RU9bwqOrIngjGDSqpYkwXHddPs02iUYSynVxFO145H6WsMFEk4sTTk3145FhW+Lwf05hH1T1++3ks5hiCU5h8Epf1NPfey5MFo3qTMMhVp3UpIUkH8gsMo3Zj74u393psJaO9oluHnIMq6j87DcLEmr+nqGeH1nEqdMVHmN+QPXntgjMvG0SXR6f6ZJNRpHL3WpqHg2LmkXQ6hyRixHcobQYiZPpJA8uVlTSxhhUUBzuHLrZ54mKJiC0noOguB2cLXxG8pIq7CNy7GnNTJhUyJZWuOqN99zzy2cflk+RK1JMIBAH+JAKBTj4swLAwBZ4gOHpkAaY1KWdiHcPvJhD+1Uj25jP5n7jFqPJG0fWfz/8SHmOUBtEFXrmBasgvbEOHL5pveO8Hz3srBvIRC7bdqSuIcPpw79OqkS/qeOTlrJfLtT6aerf9FKVeTs3lGrCZ/gNibclPulVBgS71bZSUVkmhId7JZY3WgnKnbdYI1IY pB8knJGD dn0apWh7Lw2uBrF0Dmuriepj0kqDrDMdvP35diKGZaEwT8ShOUlJy5PHM3ZlzPiPF4jbEH4lEoNnuzibfxZg0aeEPSgDrmbdiefKxXrhxSIDL/wbXrCigR3YpF3vNGqdLB+2WRvIPxm6dqkpT6n1ZrVsKZEvmcxoXyDL5R8Oz2a9PjQ4v7/06IaD9BqXAe+sc8QV2DpFj6DonxF569roBeGINOuDHuz0Wopb9tXQ9haolHrDB62z9wEbaVfoaEZionxfgqajyN+OJyfJSDKXw5SurWwGM+9gs6JMw7OoCpC847f0+Kv7Kn932HE9SJHXpFSnaDQPAhTPvtdQuIxwIv+GjxUy9nyclsyJFYSNobWlNAEQT25ClPd4GGt4pfJMoBc9D X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2/22/25 23:46, Suren Baghdasaryan wrote: > On Fri, Feb 14, 2025 at 8:27 AM Vlastimil Babka wrote: >> >> Specifying a non-zero value for a new struct kmem_cache_args field >> sheaf_capacity will setup a caching layer of percpu arrays called >> sheaves of given capacity for the created cache. >> >> Allocations from the cache will allocate via the percpu sheaves (main or >> spare) as long as they have no NUMA node preference. Frees will also >> refill one of the sheaves. >> >> When both percpu sheaves are found empty during an allocation, an empty >> sheaf may be replaced with a full one from the per-node barn. If none >> are available and the allocation is allowed to block, an empty sheaf is >> refilled from slab(s) by an internal bulk alloc operation. When both >> percpu sheaves are full during freeing, the barn can replace a full one >> with an empty one, unless over a full sheaves limit. In that case a >> sheaf is flushed to slab(s) by an internal bulk free operation. Flushing >> sheaves and barns is also wired to the existing cpu flushing and cache >> shrinking operations. >> >> The sheaves do not distinguish NUMA locality of the cached objects. If >> an allocation is requested with kmem_cache_alloc_node() with a specific >> node (not NUMA_NO_NODE), sheaves are bypassed. >> >> The bulk operations exposed to slab users also try to utilize the >> sheaves as long as the necessary (full or empty) sheaves are available >> on the cpu or in the barn. Once depleted, they will fallback to bulk >> alloc/free to slabs directly to avoid double copying. >> >> Sysfs stat counters alloc_cpu_sheaf and free_cpu_sheaf count objects >> allocated or freed using the sheaves. Counters sheaf_refill, >> sheaf_flush_main and sheaf_flush_other count objects filled or flushed >> from or to slab pages, and can be used to assess how effective the >> caching is. The refill and flush operations will also count towards the >> usual alloc_fastpath/slowpath, free_fastpath/slowpath and other >> counters. >> >> Access to the percpu sheaves is protected by local_lock_irqsave() >> operations, each per-NUMA-node barn has a spin_lock. >> >> A current limitation is that when slub_debug is enabled for a cache with >> percpu sheaves, the objects in the array are considered as allocated from >> the slub_debug perspective, and the alloc/free debugging hooks occur >> when moving the objects between the array and slab pages. This means >> that e.g. an use-after-free that occurs for an object cached in the >> array is undetected. Collected alloc/free stacktraces might also be less >> useful. This limitation could be changed in the future. >> >> On the other hand, KASAN, kmemcg and other hooks are executed on actual >> allocations and frees by kmem_cache users even if those use the array, >> so their debugging or accounting accuracy should be unaffected. >> >> Signed-off-by: Vlastimil Babka > > Only one possible issue in __pcs_flush_all_cpu(), all other comments > are nits and suggestions. Thanks. >> + * Limitations: when slub_debug is enabled for the cache, all relevant >> + * actions (i.e. poisoning, obtaining stacktraces) and checks happen >> + * when objects move between sheaves and slab pages, which may result in >> + * e.g. not detecting a use-after-free while the object is in the array >> + * cache, and the stacktraces may be less useful. > > I would also love to see a short comparison of sheaves (when objects > are freed using kfree_rcu()) vs SLAB_TYPESAFE_BY_RCU. I think both > mechanisms rcu-free objects in bulk but sheaves would not reuse an > object before RCU grace period is passed. Is that right? I don't think that's right. SLAB_TYPESAFE_BY_RCU doesn't rcu-free objects in bulk, the objects are freed immediately. It only rcu-delays freeing the slab folio once all objects are freed. >> +struct slub_percpu_sheaves { >> + local_lock_t lock; >> + struct slab_sheaf *main; /* never NULL when unlocked */ >> + struct slab_sheaf *spare; /* empty or full, may be NULL */ >> + struct slab_sheaf *rcu_free; > > Would be nice to have a short comment for rcu_free as well. I could > guess what main and spare are but for rcu_free had to look further. Added. >> +static int __kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, >> + size_t size, void **p); >> + >> + >> +static int refill_sheaf(struct kmem_cache *s, struct slab_sheaf *sheaf, >> + gfp_t gfp) >> +{ >> + int to_fill = s->sheaf_capacity - sheaf->size; >> + int filled; >> + >> + if (!to_fill) >> + return 0; >> + >> + filled = __kmem_cache_alloc_bulk(s, gfp, to_fill, >> + &sheaf->objects[sheaf->size]); >> + >> + if (!filled) >> + return -ENOMEM; >> + >> + sheaf->size = s->sheaf_capacity; > > nit: __kmem_cache_alloc_bulk() either allocates requested number of > objects or returns 0, so the current code is fine but if at some point > the implementation changes so that it can return smaller number of > objects than requested (filled < to_fill) then the above assignment > will become invalid. I think a safer thing here would be to just: > > sheaf->size += filled; > > which also makes logical sense. Alternatively you could add > VM_BUG_ON(filled != to_fill) but the increment I think would be > better. It's useful to indicate the refill was not successful, for patch 6. So I'm changing this to: sheaf->size += filled; stat_add(s, SHEAF_REFILL, filled); if (filled < to_fill) return -ENOMEM; return 0; >> + >> + stat_add(s, SHEAF_REFILL, filled); >> + >> + return 0; >> +} >> + >> + >> +static struct slab_sheaf *alloc_full_sheaf(struct kmem_cache *s, gfp_t gfp) >> +{ >> + struct slab_sheaf *sheaf = alloc_empty_sheaf(s, gfp); >> + >> + if (!sheaf) >> + return NULL; >> + >> + if (refill_sheaf(s, sheaf, gfp)) { >> + free_empty_sheaf(s, sheaf); >> + return NULL; >> + } >> + >> + return sheaf; >> +} >> + >> +/* >> + * Maximum number of objects freed during a single flush of main pcs sheaf. >> + * Translates directly to an on-stack array size. >> + */ >> +#define PCS_BATCH_MAX 32U >> + > .> +static void __kmem_cache_free_bulk(struct kmem_cache *s, size_t > size, void **p); >> + > > A comment clarifying why you are freeing in PCS_BATCH_MAX batches here > would be helpful. My understanding is that you do that to free objects > outside of the cpu_sheaves->lock, so you isolate a batch, release the > lock and then free the batch. OK. >> +static void sheaf_flush_main(struct kmem_cache *s) >> +{ >> + struct slub_percpu_sheaves *pcs; >> + unsigned int batch, remaining; >> + void *objects[PCS_BATCH_MAX]; >> + struct slab_sheaf *sheaf; >> + unsigned long flags; >> + >> +next_batch: >> + local_lock_irqsave(&s->cpu_sheaves->lock, flags); >> + pcs = this_cpu_ptr(s->cpu_sheaves); >> + sheaf = pcs->main; >> + >> + batch = min(PCS_BATCH_MAX, sheaf->size); >> + >> + sheaf->size -= batch; >> + memcpy(objects, sheaf->objects + sheaf->size, batch * sizeof(void *)); >> + >> + remaining = sheaf->size; >> + >> + local_unlock_irqrestore(&s->cpu_sheaves->lock, flags); >> + >> + __kmem_cache_free_bulk(s, batch, &objects[0]); >> + >> + stat_add(s, SHEAF_FLUSH_MAIN, batch); >> + >> + if (remaining) >> + goto next_batch; >> +} >> + > > This function seems to be used against either isolated sheaves or in > slub_cpu_dead() --> __pcs_flush_all_cpu() path where we hold > slab_mutex and I think that guarantees that the sheaf is unused. Maybe > a short comment clarifying this requirement or rename the function to > reflect that? Something like flush_unused_sheaf()? It's not slab_mutex, but the fact slub_cpu_dead() is executed in a hotplug phase when the given cpu is already not executing anymore and thus cannot be manipulating its percpu sheaves, so we are the only one that does. So I will clarify and rename to sheaf_flush_unused(). >> + >> +static void __pcs_flush_all_cpu(struct kmem_cache *s, unsigned int cpu) >> +{ >> + struct slub_percpu_sheaves *pcs; >> + >> + pcs = per_cpu_ptr(s->cpu_sheaves, cpu); >> + >> + if (pcs->spare) { >> + sheaf_flush(s, pcs->spare); >> + free_empty_sheaf(s, pcs->spare); >> + pcs->spare = NULL; >> + } >> + >> + // TODO: handle rcu_free >> + BUG_ON(pcs->rcu_free); >> + >> + sheaf_flush_main(s); > > Hmm. sheaf_flush_main() always flushes for this_cpu only, so IIUC this > call will not necessarily flush the main sheaf for the cpu passed to > __pcs_flush_all_cpu(). Thanks, yes I need to call sheaf_flush_unused(pcs->main). It's ok to do given my reply above. >> +/* >> + * Free an object to the percpu sheaves. >> + * The object is expected to have passed slab_free_hook() already. >> + */ >> +static __fastpath_inline >> +void free_to_pcs(struct kmem_cache *s, void *object) >> +{ >> + struct slub_percpu_sheaves *pcs; >> + unsigned long flags; >> + >> +restart: >> + local_lock_irqsave(&s->cpu_sheaves->lock, flags); >> + pcs = this_cpu_ptr(s->cpu_sheaves); >> + >> + if (unlikely(pcs->main->size == s->sheaf_capacity)) { >> + >> + struct slab_sheaf *empty; >> + >> + if (!pcs->spare) { >> + empty = barn_get_empty_sheaf(pcs->barn); >> + if (empty) { >> + pcs->spare = pcs->main; >> + pcs->main = empty; >> + goto do_free; >> + } >> + goto alloc_empty; >> + } >> + >> + if (pcs->spare->size < s->sheaf_capacity) { >> + stat(s, SHEAF_SWAP); >> + swap(pcs->main, pcs->spare); >> + goto do_free; >> + } >> + >> + empty = barn_replace_full_sheaf(pcs->barn, pcs->main); >> + >> + if (!IS_ERR(empty)) { >> + pcs->main = empty; >> + goto do_free; >> + } >> + >> + if (PTR_ERR(empty) == -E2BIG) { >> + /* Since we got here, spare exists and is full */ >> + struct slab_sheaf *to_flush = pcs->spare; >> + >> + pcs->spare = NULL; >> + local_unlock_irqrestore(&s->cpu_sheaves->lock, flags); >> + >> + sheaf_flush(s, to_flush); >> + empty = to_flush; >> + goto got_empty; >> + } >> + >> +alloc_empty: >> + local_unlock_irqrestore(&s->cpu_sheaves->lock, flags); >> + >> + empty = alloc_empty_sheaf(s, GFP_NOWAIT); >> + >> + if (!empty) { >> + sheaf_flush_main(s); >> + goto restart; >> + } >> + >> +got_empty: >> + local_lock_irqsave(&s->cpu_sheaves->lock, flags); >> + pcs = this_cpu_ptr(s->cpu_sheaves); >> + >> + /* >> + * if we put any sheaf to barn here, it's because we raced or >> + * have been migrated to a different cpu, which should be rare >> + * enough so just ignore the barn's limits to simplify >> + */ >> + if (unlikely(pcs->main->size < s->sheaf_capacity)) { >> + if (!pcs->spare) >> + pcs->spare = empty; >> + else >> + barn_put_empty_sheaf(pcs->barn, empty, true); >> + goto do_free; >> + } >> + >> + if (!pcs->spare) { >> + pcs->spare = pcs->main; >> + pcs->main = empty; >> + goto do_free; >> + } >> + >> + barn_put_full_sheaf(pcs->barn, pcs->main, true); >> + pcs->main = empty; > > I find the program flow in this function quite complex and hard to > follow. I think refactoring the above block starting from "pcs = > this_cpu_ptr(s->cpu_sheaves)" would somewhat simplify it. That > eliminates the need for the "got_empty" label and makes the > locking/unlocking sequence of s->cpu_sheaves->lock a bit more clear. I'm a bit lost, refactoring how exactly? >> + } >> + >> +do_free: >> + pcs->main->objects[pcs->main->size++] = object; >> + >> + local_unlock_irqrestore(&s->cpu_sheaves->lock, flags); >> + >> + stat(s, FREE_PCS); >> +}