linux-kernel - Re: [REGRESSION] [PATCH 1/3] mm/slab: use percpu allocator for cpu cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140929074418.GA29310@js1304-P5Q-DELUXE>
Date:	Mon, 29 Sep 2014 16:44:18 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	Jeremiah Mahler <jmmahler@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christoph Lameter <cl@...ux.com>,
	Pekka Enberg <penberg@...nel.org>,
	David Rientjes <rientjes@...gle.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [REGRESSION] [PATCH 1/3] mm/slab: use percpu allocator for cpu
 cache

On Sat, Sep 27, 2014 at 11:24:49PM -0700, Jeremiah Mahler wrote:
> On Thu, Aug 21, 2014 at 05:11:13PM +0900, Joonsoo Kim wrote:
> > Because of chicken and egg problem, initializaion of SLAB is really
> > complicated. We need to allocate cpu cache through SLAB to make
> > the kmem_cache works, but, before initialization of kmem_cache,
> > allocation through SLAB is impossible.
> > 
> > On the other hand, SLUB does initialization with more simple way. It
> > uses percpu allocator to allocate cpu cache so there is no chicken and
> > egg problem.
> > 
> > So, this patch try to use percpu allocator in SLAB. This simplify
> > initialization step in SLAB so that we could maintain SLAB code more
> > easily.
> > 
> > From my testing, there is no performance difference.
> > 
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@....com>
> 
> I just encountered a problem on a Lenovo Carbon X1 where it will
> suspend but won't resume.  A bisect indicated that this patch
> is causing the problem.
> 
> 997888488ef92da365b870247de773255227ce1f
> 
> I imagine the patch author, Joonsoo Kim, might have a better idea
> why this is happening than I do.  But if I can provide any information
> or run any tests that might be of help just let me know.

Hello,

Yeah, there is a bug. Below will fix your issue.
Could you test it and report the result?

Thanks for reporting it.

--------->8---------------
>From e03ed6cc554e038b86d7b3a72b89d94e9ea808ba Mon Sep 17 00:00:00 2001
From: Joonsoo Kim <iamjoonsoo.kim@....com>
Date: Mon, 29 Sep 2014 16:30:43 +0900
Subject: [PATCH] mm/slab: fix cpu on/off handling

When cpu off, we flush all cpu cached objects to it's own slab.
free_block() is used for this purpose and it's role is just to flush
objects from array_cache to proper slab. It doesn't adjust array_cache's
internal fields so we should manually reset them to proper value.
Without this fix, we maintain free objects duplicately, one is in
cpu cache, and, the other one is in the slab. So system would be broken.

Reported-by: Jeremiah Mahler <jmmahler@...il.com>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@....com>
---
 mm/slab.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/mm/slab.c b/mm/slab.c
index 1162f0e..ce289b4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1102,8 +1102,10 @@ static void cpuup_canceled(long cpu)
 
 		/* cpu is dead; no one can alloc from it. */
 		nc = per_cpu_ptr(cachep->cpu_cache, cpu);
-		if (nc)
+		if (nc) {
 			free_block(cachep, nc->entry, nc->avail, node, &list);
+			nc->avail = 0;
+		}
 
 		if (!cpumask_empty(mask)) {
 			spin_unlock_irq(&n->list_lock);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/