From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752901AbbCQMBe (ORCPT ); Tue, 17 Mar 2015 08:01:34 -0400 Received: from foss.arm.com ([217.140.101.70]:58792 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751983AbbCQMBb (ORCPT ); Tue, 17 Mar 2015 08:01:31 -0400 Date: Tue, 17 Mar 2015 12:00:58 +0000 From: Mark Rutland To: Joonsoo Kim 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: <20150317120058.GC23340@leverpostej> References: <1426261632-8911-1-git-send-email-mark.rutland@arm.com> <20150317010912.GA19483@js1304-P5Q-DELUXE> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150317010912.GA19483@js1304-P5Q-DELUXE> 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 Hi, > 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? We can use READ_ONCE(c->tid), e.g. while (IS_ENABLED(CONFIG_PREEMPT) && unlikely(tid != READ_ONCE(c->tid)); I will send a patch to that effect. I previously thought that READ_ONCE wasn't guaranteed to be atomic, and thought it could return torn values (even for a single load instruction). I now understand that this is not the case, and a READ_ONCE will be sufficient. [...] > 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. Ah. Good point. > 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. Thanks for the example; it's extremely helpful! Mark.