From: Mike Galbraith <efault@gmx.de>
To: Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Cc: linux-rt-users@vger.kernel.org,
Mel Gorman <mgorman@techsingularity.net>,
Vlastimil Babka <vbabka@suse.cz>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: [patch] v2 mm/slub: restore/expand unfreeze_partials() local exclusion scope
Date: Sat, 17 Jul 2021 16:58:05 +0200 [thread overview]
Message-ID: <476d147ab6eec386a1e8b8e11cb09708377f8c3e.camel@gmx.de> (raw)
In-Reply-To: <7431ceb9761c566cf2d1f6f263247acd8d38c4b5.camel@gmx.de>
On Thu, 2021-07-15 at 18:34 +0200, Mike Galbraith wrote:
> Greetings crickets,
>
> Methinks he problem is the hole these patches opened only for RT.
>
> static void put_cpu_partial(struct kmem_cache *s, struct page *page,
> int drain)
> {
> #ifdef CONFIG_SLUB_CPU_PARTIAL
> struct page *oldpage;
> int pages;
> int pobjects;
>
> slub_get_cpu_ptr(s->cpu_slab);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Bah, I'm tired of waiting to see what if anything mm folks do about
this little bugger, so I'm gonna step on it my damn self and be done
with it. Fly or die little patchlet.
mm/slub: restore/expand unfreeze_partials() local exclusion scope
2180da7ea70a ("mm, slub: use migrate_disable() on PREEMPT_RT") replaced
preempt_disable() in put_cpu_partial() with migrate_disable(), which when
combined with ___slab_alloc() having become preemptibile, leads to
kmem_cache_free()/kfree() blowing through ___slab_alloc() unimpeded,
and vice versa, resulting in PREMPT_RT exclusive explosions in both
paths while stress testing with both SLUB_CPU_PARTIAL/MEMCG enabled,
___slab_alloc() during allocation (duh), and __unfreeze_partials()
during free, both while accessing an unmapped page->freelist.
Serialize put_cpu_partial()/unfreeze_partials() on cpu_slab->lock to
ensure that alloc/free paths cannot pluck cpu_slab->partial out from
underneath each other unconstrained.
Signed-off-by: Mike Galbraith <efault@gmx.de>
Fixes: 2180da7ea70a ("mm, slub: use migrate_disable() on PREEMPT_RT")
---
mm/slub.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2418,6 +2418,17 @@ static void __unfreeze_partials(struct k
if (n)
spin_unlock_irqrestore(&n->list_lock, flags);
+ /*
+ * If we got here via __slab_free() -> put_cpu_partial(),
+ * memcg_free_page_obj_cgroups() ->kfree() may send us
+ * back to __slab_free() -> put_cpu_partial() for an
+ * unfortunate second encounter with cpu_slab->lock.
+ */
+ if (IS_ENABLED(CONFIG_PREEMPT_RT) && memcg_kmem_enabled()) {
+ lockdep_assert_held(this_cpu_ptr(&s->cpu_slab->lock.lock));
+ local_unlock(&s->cpu_slab->lock);
+ }
+
while (discard_page) {
page = discard_page;
discard_page = discard_page->next;
@@ -2426,6 +2437,9 @@ static void __unfreeze_partials(struct k
discard_slab(s, page);
stat(s, FREE_SLAB);
}
+
+ if (IS_ENABLED(CONFIG_PREEMPT_RT) && memcg_kmem_enabled())
+ local_lock(&s->cpu_slab->lock);
}
/*
@@ -2497,7 +2511,9 @@ static void put_cpu_partial(struct kmem_
* partial array is full. Move the existing
* set to the per node partial list.
*/
+ local_lock(&s->cpu_slab->lock);
unfreeze_partials(s);
+ local_unlock(&s->cpu_slab->lock);
oldpage = NULL;
pobjects = 0;
pages = 0;
@@ -2579,7 +2595,9 @@ static void flush_cpu_slab(struct work_s
if (c->page)
flush_slab(s, c, true);
+ local_lock(&s->cpu_slab->lock);
unfreeze_partials(s);
+ local_unlock(&s->cpu_slab->lock);
}
static bool has_cpu_slab(int cpu, struct kmem_cache *s)
@@ -2632,8 +2650,11 @@ static int slub_cpu_dead(unsigned int cp
struct kmem_cache *s;
mutex_lock(&slab_mutex);
- list_for_each_entry(s, &slab_caches, list)
+ list_for_each_entry(s, &slab_caches, list) {
+ local_lock(&s->cpu_slab->lock);
__flush_cpu_slab(s, cpu);
+ local_unlock(&s->cpu_slab->lock);
+ }
mutex_unlock(&slab_mutex);
return 0;
}
next prev parent reply other threads:[~2021-07-17 14:58 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-08 9:42 [ANNOUNCE] v5.13-rt1 Thomas Gleixner
2021-07-09 5:20 ` [patch] mm/slub: Fix kmem_cache_alloc_bulk() error path Mike Galbraith
2021-07-09 5:20 ` [patch] mm/slub: Replace do_slab_free() local_lock_irqsave/restore() calls in PREEMPT_RT scope Mike Galbraith
2021-07-09 5:21 ` [rfc/patch] mm/slub: restore/expand unfreeze_partials() local exclusion scope Mike Galbraith
2021-07-09 5:25 ` Mike Galbraith
2021-07-09 19:28 ` Thomas Gleixner
2021-07-10 1:12 ` Mike Galbraith
2021-07-15 16:34 ` Mike Galbraith
2021-07-17 14:58 ` Mike Galbraith [this message]
2021-07-18 7:58 ` [patch] v2 " Vlastimil Babka
2021-07-18 8:11 ` Mike Galbraith
2021-07-18 15:43 ` Mike Galbraith
2021-07-18 21:19 ` Vlastimil Babka
2021-07-19 4:01 ` Mike Galbraith
2021-07-19 13:15 ` Mike Galbraith
2021-07-20 2:46 ` kernel test robot
2021-07-20 8:56 ` [rfc/patch] " Vlastimil Babka
2021-07-20 11:26 ` Mike Galbraith
2021-07-21 4:56 ` Mike Galbraith
2021-07-21 8:44 ` Vlastimil Babka
2021-07-21 9:33 ` Mike Galbraith
2021-07-23 22:39 ` Vlastimil Babka
2021-07-24 2:25 ` Mike Galbraith
2021-07-25 14:09 ` Mike Galbraith
2021-07-25 14:16 ` Vlastimil Babka
2021-07-25 15:02 ` Mike Galbraith
2021-07-25 16:27 ` Vlastimil Babka
2021-07-25 19:12 ` Vlastimil Babka
2021-07-25 19:34 ` Vlastimil Babka
2021-07-26 10:04 ` Mike Galbraith
2021-07-26 17:00 ` Mike Galbraith
2021-07-26 21:26 ` Vlastimil Babka
2021-07-27 4:09 ` Mike Galbraith
2021-07-28 16:59 ` Vlastimil Babka
2021-07-29 4:51 ` Mike Galbraith
2021-07-29 9:51 ` Vlastimil Babka
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=476d147ab6eec386a1e8b8e11cb09708377f8c3e.camel@gmx.de \
--to=efault@gmx.de \
--cc=bigeasy@linutronix.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rt-users@vger.kernel.org \
--cc=mgorman@techsingularity.net \
--cc=tglx@linutronix.de \
--cc=vbabka@suse.cz \
/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 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).