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 09819C77B7A for ; Wed, 24 May 2023 11:58:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5787B900004; Wed, 24 May 2023 07:58:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 50149900003; Wed, 24 May 2023 07:58:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 37B75900004; Wed, 24 May 2023 07:58:30 -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 21F8D900003 for ; Wed, 24 May 2023 07:58:30 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E09EF80967 for ; Wed, 24 May 2023 11:58:29 +0000 (UTC) X-FDA: 80825001138.10.9356DA0 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf28.hostedemail.com (Postfix) with ESMTP id 2870DC0004 for ; Wed, 24 May 2023 11:58:26 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=WMTRqdgW; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5OLIE73r; dmarc=none; spf=pass (imf28.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684929507; 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=rH18iBwWnn9zYEQ2bRQgcgXCwVXM3ls6ER1KLLT/r60=; b=53AQhQxsrcTWu++NyFcOImJ6w0o6f9KimTd/JpaA4DELXwkQzo0lEKHbHjuv7Dgg4oEqgT 6AOstTNsoTiJttdQia0MYmLi9UN1xp5zKY+odBMIiAyVFyFW6dy4XYtdNyr07OOUubMwsw J525jTcPYgPIxDML02zpddV4KaoPjjI= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=WMTRqdgW; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=5OLIE73r; dmarc=none; spf=pass (imf28.hostedemail.com: domain of vbabka@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=vbabka@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684929507; a=rsa-sha256; cv=none; b=iuD7cq1xPQxsbDIdc+6duMtYBUPJ3AejGRfF34OKfrSLiFDyOcdk3QB1IOG1niZWrlD4ZO u146OUnhZK9gaLa53h72ZpPRbaq5SJncMCNVqSEZ3zcDieY8f3Q0fY+sv08W9u/V0aTXie ulqkBPWWkBJoKvkbctMbyFfmZ6THpKg= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 802D622168; Wed, 24 May 2023 11:58:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1684929505; 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=rH18iBwWnn9zYEQ2bRQgcgXCwVXM3ls6ER1KLLT/r60=; b=WMTRqdgWgo5Xv1JGWXGXa/4leJ+kA3Fr2EvuxoJ+Aqe6NU5zbCpx8cdBkJRqivz3zlmCI8 fYweNr+iUlPQUznzgqAE9D1FG4L+KMokWqHKNGTedF59FJDkPiQa49qcPJoEz7QSNuY/TE 11uKzpPFgkG44/ySFC88vJeQKALxzNA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1684929505; 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=rH18iBwWnn9zYEQ2bRQgcgXCwVXM3ls6ER1KLLT/r60=; b=5OLIE73rOKr/k1ojPtkzX/HPu5nXWbAb/WgNuvY9KLod1eci7r+KTUnNLWeyZDjI8HJPtV go+jXuaQCYw1m7Dw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id F3D0D13425; Wed, 24 May 2023 11:58:24 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id cu/HOuD7bWSZVgAAMHmgww (envelope-from ); Wed, 24 May 2023 11:58:24 +0000 Message-ID: <18c33bf0-0c7e-7584-5149-33cf77b50b8a@suse.cz> Date: Wed, 24 May 2023 13:58:24 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v3 09/11] mm/slub: Fold slab_update_freelist() Content-Language: en-US To: Peter Zijlstra , torvalds@linux-foundation.org Cc: corbet@lwn.net, will@kernel.org, boqun.feng@gmail.com, mark.rutland@arm.com, catalin.marinas@arm.com, dennis@kernel.org, tj@kernel.org, cl@linux.com, hca@linux.ibm.com, gor@linux.ibm.com, agordeev@linux.ibm.com, borntraeger@linux.ibm.com, svens@linux.ibm.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com, joro@8bytes.org, suravee.suthikulpanit@amd.com, robin.murphy@arm.com, dwmw2@infradead.org, baolu.lu@linux.intel.com, Arnd Bergmann , Herbert Xu , davem@davemloft.net, penberg@kernel.org, rientjes@google.com, iamjoonsoo.kim@lge.com, Andrew Morton , roman.gushchin@linux.dev, 42.hyeyoo@gmail.com, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-s390@vger.kernel.org, iommu@lists.linux.dev, linux-arch@vger.kernel.org, linux-crypto@vger.kernel.org References: <20230515075659.118447996@infradead.org> <20230515080554.520976397@infradead.org> From: Vlastimil Babka In-Reply-To: <20230515080554.520976397@infradead.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 2870DC0004 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: aartseu7rne7c9dsxuzd7mjnk4kbrkh7 X-HE-Tag: 1684929506-462977 X-HE-Meta: U2FsdGVkX1+v40+WW/rcKnUv+gEIEUQjsncVTt/xAAi0QacOXIc99E5YA4WNjkEecR75SrFtJUQlIdHMLFnX3OCKRj4C+KlcAY2Yr3uOHkuEy5vvDlFGVHor++T0xdvxT9S0rD5K7WjUe0g59huE7VVf21bgzz5pQf9ccdAkcqozje3bqjIJgN8n/5+eaNYKMcW39G25VcID33/RzVKsyBw5vyrrOh9cNotiyalLp7m/2SFOgw4IbbIBOxtedy3clcZVSAX+tQ4IOWlZXmUVjIqr7TQZLq4GKZ2mmcigzWg4RrDBd5VdfrLxn2PZppLWKsu3js9RE0BmcJcwMyYYSBkV5l6S94Uf9WltvjCg37g1sSp9SuGlal2nKfRofMBUADV7C4oxjK7U8HM9U5jqhkGHlRGPdOrTjEuu3MzwWyRwDjDhNpOQyFzKlXK9S9FayyBS+RJed9YLhmVmpEM5udC5pjyzgryoeXQV+WU0DMOgWKIvteq+AlPPNT5HxGmDD6TbUC0Uz2lVgGzHn15RALYvy/W1vHVumcmPBIZO0TTLEqEmHftavTF5JbYnfFqBBdF1qqIKRY91v/SHX4xoKZa/J3PEI3hC4bbGjn4Oa9Xse68yY8S2hSjHS50evGjhHiJq/3lrhYLKb/LRXx+Zs+gKkkrQ+xrAX2lFcCYfsPiKHdFZgbHLFHN0ZuZ8a0pC5SWtFy9688fz/IswSLM1+UVhEWuGw2H6G50jniM7tHJw8LISzHUn5b0/ABZUnBiQn+V8kfycOYfrk35NHIT5iK67oegMbSjmFmVWuDII1SOkVYKyqq8VFecWEHR4AcBAMtvBZyrjhPw5qaXAmFt3cvPnxCOeKtUEGifN7/CtQWJUY1V8rZXwU7IVB2xZvD4cUKupRs5IM1XQ18mrvgEuFOo+55k24kFwGzIeWfeNiUmuunN2RieTJGLUZGh/0+08oVk/9dlIaOLQqKxLyDO ih1ayor5 Fx8Rf0UmAc1wxPZJ/6HwWKIpAQKeZGLN4TVPIhI0qdDO+pEo837tjgi+KT2wJ28V9z4mQVI9pmuLT+QyN5hfiouKLmW/+wUc4C1668Zql1AZX8p0Kij7KCgr6F3m1zSpxvSA+tcM+FeBn4B9iPNBCK8XR+gufvbBm60Vz9YTHnEo8ykInv1GE6LsSvjBXCufIcIbX1MwyZpxzlGJkEXCUFwi5J+eqGkBxFBcDcdtGrHc6ty8vG3InBzcxxicpE+KpiNn/lRqmQCh7iRUb950YT0YtFLFmcjMMqoqkDWIpIZLTjEvz3JDzdFYp9ckr+yHKVhOVVniFBmaLVarVqdd9j0nB9lzd4+4Dv5ZDR0MzbAU8NGqdHDVKf2/K4SmGUT9CF/VoeQdv4jq5ql+584dKlMmQPMwHn1fQdLLBjse2JR9s4SocmUvmGwC/g2qUJ0zdsSjtiXqSHuVlUWQ= 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: On 5/15/23 09:57, Peter Zijlstra wrote: > The two functions slab_update_freelist() and __slab_update_freelist() > are nearly identical, fold and add a boolean argument and rely on > constant propagation. > > Signed-off-by: Peter Zijlstra (Intel) Something like that has been tried before and the result: https://lore.kernel.org/all/CAHk-=wiJLqL2cUhJbvpyPQpkbVOu1rVSzgO2=S2jC55hneLtfQ@mail.gmail.com/ Your parameter is not called 'locked' but 'irq_save' which is better, but that's just one detail. After your refactoring in 08/11 which puts most of the code into __update_freelist_fast() and _slow() I'd say the result is not so bad already. BTW I have some suspicion that some SLUB code is based on assumptions that are no longer true these days. IIRC I've seen some microbenchmark results a while ago that showed that disabling/enabling irqs is surprisingly (to me) very cheap today, so maybe it's not so useful to keep doing the this_cpu_cmpxchg128 for the struct kmem_cache_cpu operations (less so for struct slab cmpxchg128 where actually different cpus may be involved). But it needs a closer look. > --- > mm/slub.c | 80 +++++++++++++++++++++----------------------------------------- > 1 file changed, 28 insertions(+), 52 deletions(-) > > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -559,53 +559,29 @@ __update_freelist_slow(struct slab *slab > * allocation/ free operation in hardirq context. Therefore nothing can > * interrupt the operation. > */ > -static inline bool __slab_update_freelist(struct kmem_cache *s, struct slab *slab, > - void *freelist_old, unsigned long counters_old, > - void *freelist_new, unsigned long counters_new, > - const char *n) > +static __always_inline > +bool slab_update_freelist(struct kmem_cache *s, struct slab *slab, > + void *freelist_old, unsigned long counters_old, > + void *freelist_new, unsigned long counters_new, > + bool irq_save, const char *n) > { > bool ret; > > - if (USE_LOCKLESS_FAST_PATH()) > + if (!irq_save && USE_LOCKLESS_FAST_PATH()) > lockdep_assert_irqs_disabled(); > > if (s->flags & __CMPXCHG_DOUBLE) { > ret = __update_freelist_fast(slab, freelist_old, counters_old, > freelist_new, counters_new); > } else { > - ret = __update_freelist_slow(slab, freelist_old, counters_old, > - freelist_new, counters_new); > - } > - if (likely(ret)) > - return true; > - > - cpu_relax(); > - stat(s, CMPXCHG_DOUBLE_FAIL); > - > -#ifdef SLUB_DEBUG_CMPXCHG > - pr_info("%s %s: cmpxchg double redo ", n, s->name); > -#endif > - > - return false; > -} > - > -static inline bool slab_update_freelist(struct kmem_cache *s, struct slab *slab, > - void *freelist_old, unsigned long counters_old, > - void *freelist_new, unsigned long counters_new, > - const char *n) > -{ > - bool ret; > - > - if (s->flags & __CMPXCHG_DOUBLE) { > - ret = __update_freelist_fast(slab, freelist_old, counters_old, > - freelist_new, counters_new); > - } else { > unsigned long flags; > > - local_irq_save(flags); > + if (irq_save) > + local_irq_save(flags); > ret = __update_freelist_slow(slab, freelist_old, counters_old, > freelist_new, counters_new); > - local_irq_restore(flags); > + if (irq_save) > + local_irq_restore(flags); > } > if (likely(ret)) > return true; > @@ -2250,10 +2226,10 @@ static inline void *acquire_slab(struct > VM_BUG_ON(new.frozen); > new.frozen = 1; > > - if (!__slab_update_freelist(s, slab, > - freelist, counters, > - new.freelist, new.counters, > - "acquire_slab")) > + if (!slab_update_freelist(s, slab, > + freelist, counters, > + new.freelist, new.counters, > + false, "acquire_slab")) > return NULL; > > remove_partial(n, slab); > @@ -2577,9 +2553,9 @@ static void deactivate_slab(struct kmem_ > > > if (!slab_update_freelist(s, slab, > - old.freelist, old.counters, > - new.freelist, new.counters, > - "unfreezing slab")) { > + old.freelist, old.counters, > + new.freelist, new.counters, > + true, "unfreezing slab")) { > if (mode == M_PARTIAL) > spin_unlock_irqrestore(&n->list_lock, flags); > goto redo; > @@ -2633,10 +2609,10 @@ static void __unfreeze_partials(struct k > > new.frozen = 0; > > - } while (!__slab_update_freelist(s, slab, > - old.freelist, old.counters, > - new.freelist, new.counters, > - "unfreezing slab")); > + } while (!slab_update_freelist(s, slab, > + old.freelist, old.counters, > + new.freelist, new.counters, > + false, "unfreezing slab")); > > if (unlikely(!new.inuse && n->nr_partial >= s->min_partial)) { > slab->next = slab_to_discard; > @@ -3072,10 +3048,10 @@ static inline void *get_freelist(struct > new.inuse = slab->objects; > new.frozen = freelist != NULL; > > - } while (!__slab_update_freelist(s, slab, > - freelist, counters, > - NULL, new.counters, > - "get_freelist")); > + } while (!slab_update_freelist(s, slab, > + freelist, counters, > + NULL, new.counters, > + false, "get_freelist")); > > return freelist; > } > @@ -3666,9 +3642,9 @@ static void __slab_free(struct kmem_cach > } > > } while (!slab_update_freelist(s, slab, > - prior, counters, > - head, new.counters, > - "__slab_free")); > + prior, counters, > + head, new.counters, > + true, "__slab_free")); > > if (likely(!n)) { > > >