Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <289873432407e0569401c8894843d7004b59d90b.camel@gmx.de>
Subject: Re: [RFC v2 00/34] SLUB: reduce irq disabled scope and make it RT
 compatible
From:   Mike Galbraith <efault@gmx.de>
To:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Vlastimil Babka <vbabka@suse.cz>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Christoph Lameter <cl@linux.com>,
        David Rientjes <rientjes@google.com>,
        Pekka Enberg <penberg@kernel.org>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Mel Gorman <mgorman@techsingularity.net>,
        Jesper Dangaard Brouer <brouer@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Jann Horn <jannh@google.com>
Date:   Tue, 06 Jul 2021 19:56:29 +0200
In-Reply-To: <206d6a81e08cb23a2c97e67335be6017e938bac5.camel@gmx.de>
References: <20210609113903.1421-1-vbabka@suse.cz>
         <20210702182944.lqa7o2a25to6czju@linutronix.de>
         <206d6a81e08cb23a2c97e67335be6017e938bac5.camel@gmx.de>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, 2021-07-05 at 18:00 +0200, Mike Galbraith wrote:
> On Fri, 2021-07-02 at 20:29 +0200, Sebastian Andrzej Siewior wrote:
> >
> > > The remaining patches to upstream from the RT tree are small ones
> > > related to KConfig. The patch that restricts PREEMPT_RT to SLUB
> > > (not SLAB or SLOB) makes sense. The patch that disables
> > > CONFIG_SLUB_CPU_PARTIAL with PREEMPT_RT could perhaps be re-
> > > evaluated as the series also addresses some latency issues with
> > > percpu partial slabs.
> >
> > With that series the PARTIAL slab can be indeed enabled. I have
> > (had)
> > a half done series where I had PARTIAL enabled and noticed a slight
> > increase in latency so made it "default y on !RT". It wasn't
> > dramatic
> > but appeared to be outside of noise.
>
> I'm seeing warnings/explosions while exercising box IFF PARTIAL slab
> thingy is enabled.  I aborted -PARTIAL after a little over 4 hours,
> whereas the longest survival of 4 +PARTIAL runs was 50 minutes, so
> I'm fairly confident that PARTIAL really really is the trigger.

Resurrecting local exclusion around unfreeze_partials() seems to have
put an end to that.  Guess I can chop these trees down now.

=2D--
 mm/slub.c |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

=2D-- a/mm/slub.c
+++ b/mm/slub.c
@@ -2497,7 +2497,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 =3D NULL;
 				pobjects =3D 0;
 				pages =3D 0;
@@ -2579,7 +2581,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)
@@ -3358,13 +3362,12 @@ static __always_inline void do_slab_free
 		 * we need to take the local_lock. We shouldn't simply defer to
 		 * __slab_free() as that wouldn't use the cpu freelist at all.
 		 */
-		unsigned long flags;
 		void **freelist;

-		local_lock_irqsave(&s->cpu_slab->lock, flags);
+		local_lock(&s->cpu_slab->lock);
 		c =3D this_cpu_ptr(s->cpu_slab);
 		if (unlikely(page !=3D c->page)) {
-			local_unlock_irqrestore(&s->cpu_slab->lock, flags);
+			local_unlock(&s->cpu_slab->lock);
 			goto redo;
 		}
 		tid =3D c->tid;
@@ -3374,7 +3377,7 @@ static __always_inline void do_slab_free
 		c->freelist =3D head;
 		c->tid =3D next_tid(tid);

-		local_unlock_irqrestore(&s->cpu_slab->lock, flags);
+		local_unlock(&s->cpu_slab->lock);
 #endif
 		stat(s, FREE_FASTPATH);
 	} else
@@ -3601,7 +3604,7 @@ int kmem_cache_alloc_bulk(struct kmem_ca
 				slab_want_init_on_alloc(flags, s));
 	return i;
 error:
-	local_unlock_irq(&s->cpu_slab->lock);
+	slub_put_cpu_ptr(s->cpu_slab);
 	slab_post_alloc_hook(s, objcg, flags, i, p, false);
 	__kmem_cache_free_bulk(s, i, p);
 	return 0;