All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Pekka Enberg <penberg@kernel.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Mike Galbraith <efault@gmx.de>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Mel Gorman <mgorman@techsingularity.net>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Jann Horn <jannh@google.com>, Vlastimil Babka <vbabka@suse.cz>
Subject: [PATCH v5 11/35] mm, slub: simplify kmem_cache_cpu and tid setup
Date: Mon, 23 Aug 2021 16:58:02 +0200	[thread overview]
Message-ID: <20210823145826.3857-12-vbabka@suse.cz> (raw)
In-Reply-To: <20210823145826.3857-1-vbabka@suse.cz>

In slab_alloc_node() and do_slab_free() fastpaths we need to guarantee that
our kmem_cache_cpu pointer is from the same cpu as the tid value. Currently
that's done by reading the tid first using this_cpu_read(), then the
kmem_cache_cpu pointer and verifying we read the same tid using the pointer and
plain READ_ONCE().

This can be simplified to just fetching kmem_cache_cpu pointer and then reading
tid using the pointer. That guarantees they are from the same cpu. We don't
need to read the tid using this_cpu_read() because the value will be validated
by this_cpu_cmpxchg_double(), making sure we are on the correct cpu and the
freelist didn't change by anyone preempting us since reading the tid.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Mel Gorman <mgorman@techsingularity.net>
---
 mm/slub.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index b13424444cae..1f65d75077ce 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2868,15 +2868,14 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
 	 * reading from one cpu area. That does not matter as long
 	 * as we end up on the original cpu again when doing the cmpxchg.
 	 *
-	 * We should guarantee that tid and kmem_cache are retrieved on
-	 * the same cpu. It could be different if CONFIG_PREEMPTION so we need
-	 * to check if it is matched or not.
+	 * We must guarantee that tid and kmem_cache_cpu are retrieved on the
+	 * same cpu. We read first the kmem_cache_cpu pointer and use it to read
+	 * the tid. If we are preempted and switched to another cpu between the
+	 * two reads, it's OK as the two are still associated with the same cpu
+	 * and cmpxchg later will validate the cpu.
 	 */
-	do {
-		tid = this_cpu_read(s->cpu_slab->tid);
-		c = raw_cpu_ptr(s->cpu_slab);
-	} while (IS_ENABLED(CONFIG_PREEMPTION) &&
-		 unlikely(tid != READ_ONCE(c->tid)));
+	c = raw_cpu_ptr(s->cpu_slab);
+	tid = READ_ONCE(c->tid);
 
 	/*
 	 * Irqless object alloc/free algorithm used here depends on sequence
@@ -3150,11 +3149,8 @@ static __always_inline void do_slab_free(struct kmem_cache *s,
 	 * data is retrieved via this pointer. If we are on the same cpu
 	 * during the cmpxchg then the free will succeed.
 	 */
-	do {
-		tid = this_cpu_read(s->cpu_slab->tid);
-		c = raw_cpu_ptr(s->cpu_slab);
-	} while (IS_ENABLED(CONFIG_PREEMPTION) &&
-		 unlikely(tid != READ_ONCE(c->tid)));
+	c = raw_cpu_ptr(s->cpu_slab);
+	tid = READ_ONCE(c->tid);
 
 	/* Same with comment on barrier() in slab_alloc_node() */
 	barrier();
-- 
2.32.0


  parent reply	other threads:[~2021-08-23 14:59 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-23 14:57 [PATCH v5 00/35] SLUB: reduce irq disabled scope and make it RT compatible Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 01/35] mm, slub: don't call flush_all() from slab_debug_trace_open() Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 02/35] mm, slub: allocate private object map for debugfs listings Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 03/35] mm, slub: allocate private object map for validate_slab_cache() Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 04/35] mm, slub: don't disable irq for debug_check_no_locks_freed() Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 05/35] mm, slub: remove redundant unfreeze_partials() from put_cpu_partial() Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 06/35] mm, slub: unify cmpxchg_double_slab() and __cmpxchg_double_slab() Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 07/35] mm, slub: extract get_partial() from new_slab_objects() Vlastimil Babka
2021-08-23 14:57 ` [PATCH v5 08/35] mm, slub: dissolve new_slab_objects() into ___slab_alloc() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 09/35] mm, slub: return slab page from get_partial() and set c->page afterwards Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 10/35] mm, slub: restructure new page checks in ___slab_alloc() Vlastimil Babka
2021-08-23 14:58 ` Vlastimil Babka [this message]
2021-08-23 14:58 ` [PATCH v5 12/35] mm, slub: move disabling/enabling irqs to ___slab_alloc() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 13/35] mm, slub: do initial checks in ___slab_alloc() with irqs enabled Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 14/35] mm, slub: move disabling irqs closer to get_partial() in ___slab_alloc() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 15/35] mm, slub: restore irqs around calling new_slab() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 16/35] mm, slub: validate slab from partial list or page allocator before making it cpu slab Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 17/35] mm, slub: check new pages with restored irqs Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 18/35] mm, slub: stop disabling irqs around get_partial() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 19/35] mm, slub: move reset of c->page and freelist out of deactivate_slab() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 20/35] mm, slub: make locking in deactivate_slab() irq-safe Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 21/35] mm, slub: call deactivate_slab() without disabling irqs Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 22/35] mm, slub: move irq control into unfreeze_partials() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 23/35] mm, slub: discard slabs in unfreeze_partials() without irqs disabled Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 24/35] mm, slub: detach whole partial list at once in unfreeze_partials() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 25/35] mm, slub: separate detaching of partial list in unfreeze_partials() from unfreezing Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 26/35] mm, slub: only disable irq with spin_lock in __unfreeze_partials() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 27/35] mm, slub: don't disable irqs in slub_cpu_dead() Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 28/35] mm, slab: make flush_slab() possible to call with irqs enabled Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 29/35] mm: slub: Move flush_cpu_slab() invocations __free_slab() invocations out of IRQ context Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 30/35] mm: slub: Make object_map_lock a raw_spinlock_t Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 31/35] mm, slub: optionally save/restore irqs in slab_[un]lock()/ Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 32/35] mm, slub: make slab_lock() disable irqs with PREEMPT_RT Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 33/35] mm, slub: protect put_cpu_partial() with disabled irqs instead of cmpxchg Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 34/35] mm, slub: use migrate_disable() on PREEMPT_RT Vlastimil Babka
2021-08-23 14:58 ` [PATCH v5 35/35] mm, slub: convert kmem_cpu_slab protection to local_lock Vlastimil Babka
2021-08-23 15:11 ` [PATCH v5 00/35] SLUB: reduce irq disabled scope and make it RT compatible Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210823145826.3857-12-vbabka@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=brouer@redhat.com \
    --cc=cl@linux.com \
    --cc=efault@gmx.de \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.