From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751874AbbCQBJl (ORCPT ); Mon, 16 Mar 2015 21:09:41 -0400 Received: from LGEMRELSE7Q.lge.com ([156.147.1.151]:53009 "EHLO lgemrelse7q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751201AbbCQBJh (ORCPT ); Mon, 16 Mar 2015 21:09:37 -0400 X-Original-SENDERIP: 10.177.222.209 X-Original-MAILFROM: iamjoonsoo.kim@lge.com Date: Tue, 17 Mar 2015 10:09:12 +0900 From: Joonsoo Kim To: Mark Rutland Cc: linux-kernel@vger.kernel.org, Andrew Morton , Catalin Marinas , Christoph Lameter , David Rientjes , Jesper Dangaard Brouer , Linus Torvalds , Pekka Enberg , Steve Capper Subject: Re: [PATCH] mm/slub: fix lockups on PREEMPT && !SMP kernels Message-ID: <20150317010912.GA19483@js1304-P5Q-DELUXE> References: <1426261632-8911-1-git-send-email-mark.rutland@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426261632-8911-1-git-send-email-mark.rutland@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Fri, Mar 13, 2015 at 03:47:12PM +0000, Mark Rutland wrote: > Commit 9aabf810a67cd97e ("mm/slub: optimize alloc/free fastpath by > removing preemption on/off") introduced an occasional hang for kernels > built with CONFIG_PREEMPT && !CONFIG_SMP. > > The problem is the following loop the patch introduced to > slab_alloc_node and slab_free: > > do { > tid = this_cpu_read(s->cpu_slab->tid); > c = raw_cpu_ptr(s->cpu_slab); > } while (IS_ENABLED(CONFIG_PREEMPT) && unlikely(tid != c->tid)); > > GCC 4.9 has been observed to hoist the load of c and c->tid above the > loop for !SMP kernels (as in this case raw_cpu_ptr(x) is compile-time > constant and does not force a reload). On arm64 the generated assembly > looks like: > > ffffffc00016d3c4: f9400404 ldr x4, [x0,#8] > ffffffc00016d3c8: f9400401 ldr x1, [x0,#8] > ffffffc00016d3cc: eb04003f cmp x1, x4 > ffffffc00016d3d0: 54ffffc1 b.ne ffffffc00016d3c8 > > If the thread is preempted between the load of c->tid (into x1) and tid > (into x4), and and allocation or free occurs in another thread (bumping > the cpu_slab's tid), the thread will be stuck in the loop until > s->cpu_slab->tid wraps, which may be forever in the absence of > allocations on the same CPU. Is there any method to guarantee refetching these in each loop? > > The loop itself is somewhat pointless as the thread can be preempted at > any point after the loop before the this_cpu_cmpxchg_double, and the > window for preemption within the loop is far smaller. Given that we > assume accessing c->tid is safe for the loop condition, and we retry > when the cmpxchg fails, we can get rid of the loop entirely and just > access c->tid via the raw_cpu_ptr for s->cpu_slab. Hmm... IIUC, loop itself is not pointless. It guarantees that tid and c (s->cpu_slab) are fetched on right and same processor and this is for algorithm correctness. Think about your code. c = raw_cpu_ptr(s->cpu_slab); tid = READ_ONCE(c->tid); This doesn't guarantee that tid is fetched on the cpu where c is fetched if preemption/migration happens between these operations. If c->tid, c->freelist, c->page are fetched on the other cpu, there is no ordering guarantee and c->freelist, c->page could be stale value even if c->tid is recent one. Think about following free case with your patch. Assume initial cpu 0's state as following. c->tid: 1, c->freelist: NULL, c->page: A User X: try to free object X for page A User X: fetch c (s->cpu_slab) Preemtion and migration happens... The other allocation/free happens... so cpu 0's state is as following. c->tid: 3, c->freelist: NULL, c->page: B User X: read c->tid: 3, c->freelist: NULL, c->page A (stale value) Because tid and freelist are matched with current ones, free would succeed, but, current c->page is B and object is for A so this success is wrong. Loop prevents this possibility. Thanks.