linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: linux-mm@kvack.org, Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Pekka Enberg <penberg@kernel.org>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.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>,
	Vlastimil Babka <vbabka@suse.cz>
Subject: [PATCH v6 30/33] mm, slub: make slab_lock() disable irqs with PREEMPT_RT
Date: Sat,  4 Sep 2021 12:50:00 +0200	[thread overview]
Message-ID: <20210904105003.11688-31-vbabka@suse.cz> (raw)
In-Reply-To: <20210904105003.11688-1-vbabka@suse.cz>

We need to disable irqs around slab_lock() (a bit spinlock) to make it
irq-safe. Most calls to slab_lock() are nested under spin_lock_irqsave() which
doesn't disable irqs on PREEMPT_RT, so add explicit disabling with PREEMPT_RT.
The exception is cmpxchg_double_slab() which already disables irqs, so use a
__slab_[un]lock() variant without irq disable there.

slab_[un]lock() thus needs a flags pointer parameter, which is unused on !RT.
free_debug_processing() now has two flags variables, which looks odd, but only
one is actually used - the one used in spin_lock_irqsave() on !RT and the one
used in slab_lock() on RT.

As a result, __cmpxchg_double_slab() and cmpxchg_double_slab() become
effectively identical on RT, as both will disable irqs, which is necessary on
RT as most callers of this function also rely on irqsaving lock operations.
Thus, assert that irqs are already disabled in __cmpxchg_double_slab() only on
!RT and also change the VM_BUG_ON assertion to the more standard lockdep_assert
one.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/slub.c | 58 +++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 76e56ce55d21..a04c36e173c0 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -359,25 +359,44 @@ static inline unsigned int oo_objects(struct kmem_cache_order_objects x)
 /*
  * Per slab locking using the pagelock
  */
-static __always_inline void slab_lock(struct page *page)
+static __always_inline void __slab_lock(struct page *page)
 {
 	VM_BUG_ON_PAGE(PageTail(page), page);
 	bit_spin_lock(PG_locked, &page->flags);
 }
 
-static __always_inline void slab_unlock(struct page *page)
+static __always_inline void __slab_unlock(struct page *page)
 {
 	VM_BUG_ON_PAGE(PageTail(page), page);
 	__bit_spin_unlock(PG_locked, &page->flags);
 }
 
-/* Interrupts must be disabled (for the fallback code to work right) */
+static __always_inline void slab_lock(struct page *page, unsigned long *flags)
+{
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_irq_save(*flags);
+	__slab_lock(page);
+}
+
+static __always_inline void slab_unlock(struct page *page, unsigned long *flags)
+{
+	__slab_unlock(page);
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_irq_restore(*flags);
+}
+
+/*
+ * Interrupts must be disabled (for the fallback code to work right), typically
+ * by an _irqsave() lock variant. Except on PREEMPT_RT where locks are different
+ * so we disable interrupts as part of slab_[un]lock().
+ */
 static inline bool __cmpxchg_double_slab(struct kmem_cache *s, struct page *page,
 		void *freelist_old, unsigned long counters_old,
 		void *freelist_new, unsigned long counters_new,
 		const char *n)
 {
-	VM_BUG_ON(!irqs_disabled());
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		lockdep_assert_irqs_disabled();
 #if defined(CONFIG_HAVE_CMPXCHG_DOUBLE) && \
     defined(CONFIG_HAVE_ALIGNED_STRUCT_PAGE)
 	if (s->flags & __CMPXCHG_DOUBLE) {
@@ -388,15 +407,18 @@ static inline bool __cmpxchg_double_slab(struct kmem_cache *s, struct page *page
 	} else
 #endif
 	{
-		slab_lock(page);
+		/* init to 0 to prevent spurious warnings */
+		unsigned long flags = 0;
+
+		slab_lock(page, &flags);
 		if (page->freelist == freelist_old &&
 					page->counters == counters_old) {
 			page->freelist = freelist_new;
 			page->counters = counters_new;
-			slab_unlock(page);
+			slab_unlock(page, &flags);
 			return true;
 		}
-		slab_unlock(page);
+		slab_unlock(page, &flags);
 	}
 
 	cpu_relax();
@@ -427,16 +449,16 @@ static inline bool cmpxchg_double_slab(struct kmem_cache *s, struct page *page,
 		unsigned long flags;
 
 		local_irq_save(flags);
-		slab_lock(page);
+		__slab_lock(page);
 		if (page->freelist == freelist_old &&
 					page->counters == counters_old) {
 			page->freelist = freelist_new;
 			page->counters = counters_new;
-			slab_unlock(page);
+			__slab_unlock(page);
 			local_irq_restore(flags);
 			return true;
 		}
-		slab_unlock(page);
+		__slab_unlock(page);
 		local_irq_restore(flags);
 	}
 
@@ -1269,11 +1291,11 @@ static noinline int free_debug_processing(
 	struct kmem_cache_node *n = get_node(s, page_to_nid(page));
 	void *object = head;
 	int cnt = 0;
-	unsigned long flags;
+	unsigned long flags, flags2;
 	int ret = 0;
 
 	spin_lock_irqsave(&n->list_lock, flags);
-	slab_lock(page);
+	slab_lock(page, &flags2);
 
 	if (s->flags & SLAB_CONSISTENCY_CHECKS) {
 		if (!check_slab(s, page))
@@ -1306,7 +1328,7 @@ static noinline int free_debug_processing(
 		slab_err(s, page, "Bulk freelist count(%d) invalid(%d)\n",
 			 bulk_cnt, cnt);
 
-	slab_unlock(page);
+	slab_unlock(page, &flags2);
 	spin_unlock_irqrestore(&n->list_lock, flags);
 	if (!ret)
 		slab_fix(s, "Object at 0x%p not freed", object);
@@ -4087,11 +4109,12 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page,
 {
 #ifdef CONFIG_SLUB_DEBUG
 	void *addr = page_address(page);
+	unsigned long flags;
 	unsigned long *map;
 	void *p;
 
 	slab_err(s, page, text, s->name);
-	slab_lock(page);
+	slab_lock(page, &flags);
 
 	map = get_map(s, page);
 	for_each_object(p, s, addr, page->objects) {
@@ -4102,7 +4125,7 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page,
 		}
 	}
 	put_map(map);
-	slab_unlock(page);
+	slab_unlock(page, &flags);
 #endif
 }
 
@@ -4834,8 +4857,9 @@ static void validate_slab(struct kmem_cache *s, struct page *page,
 {
 	void *p;
 	void *addr = page_address(page);
+	unsigned long flags;
 
-	slab_lock(page);
+	slab_lock(page, &flags);
 
 	if (!check_slab(s, page) || !on_freelist(s, page, NULL))
 		goto unlock;
@@ -4850,7 +4874,7 @@ static void validate_slab(struct kmem_cache *s, struct page *page,
 			break;
 	}
 unlock:
-	slab_unlock(page);
+	slab_unlock(page, &flags);
 }
 
 static int validate_slab_node(struct kmem_cache *s,
-- 
2.33.0


  parent reply	other threads:[~2021-09-04 10:51 UTC|newest]

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

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=20210904105003.11688-31-vbabka@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=bigeasy@linutronix.de \
    --cc=cl@linux.com \
    --cc=efault@gmx.de \
    --cc=iamjoonsoo.kim@lge.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 \
    --subject='Re: [PATCH v6 30/33] mm, slub: make slab_lock() disable irqs with PREEMPT_RT' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).