linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Christoph Lameter <cl@linux.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	linux-mm <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Pekka Enberg <penberg@kernel.org>, Matt Mackall <mpm@selenic.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	RT <linux-rt-users@vger.kernel.org>,
	Clark Williams <clark@redhat.com>, John Kacur <jkacur@gmail.com>,
	"Luis Claudio R. Goncalves" <lgoncalv@redhat.com>
Subject: Re: [RFC][PATCH v2] slub: Keep page and object in sync in slab_alloc_node()
Date: Fri, 18 Jan 2013 10:04:44 -0500	[thread overview]
Message-ID: <1358521484.7383.8.camel@gandalf.local.home> (raw)
In-Reply-To: <0000013c4e1ea131-b8ab56b9-bfca-44fe-b5da-f030551194c9-000000@email.amazonses.com>

On Fri, 2013-01-18 at 14:44 +0000, Christoph Lameter wrote:
> On Thu, 17 Jan 2013, Steven Rostedt wrote:
> 
> > In slab_alloc_node(), after the cpu_slab is assigned, if the task is
> > preempted and moves to another CPU, there's nothing keeping the page and
> > object in sync. The -rt kernel crashed because page was NULL and object
> > was not, and the node_match() dereferences page. Even though the crash
> > happened on -rt, there's nothing that's keeping this from happening on
> > mainline.
> >
> > The easiest fix is to disable interrupts for the entire time from
> > acquiring the current CPU cpu_slab and assigning the object and page.
> > After that, it's fine to allow preemption.
> 
> Its easiest to just check for the NULL pointer as initally done. The call
> to __slab_alloc can do what the fastpath does.
> 
> And the fastpath will verify that the c->page pointer was not changed.

The problem is that the changes can happen on another CPU, which means
that barrier isn't sufficient.

	CPU0			CPU1
	----			----
<cpu fetches c->page>
			updates c->tid
			updates c->page
			updates c->freelist
<cpu fetches c->tid>
<cpu fetches c->freelist>

  node_match() succeeds even though
    current c->page wont

 this_cpu_cmpxchg_double() only tests
   the object (freelist) and tid, both which
   will match, but the page that was tested
   isn't the right one.


That barrier() is meaningless as soon as another CPU is involved. The
CPU can order things anyway it wants, even if the assembly did in
differently. Due to cacheline misses and such, we have no idea if
c->page has been prefetched into memory or not.

We may get by with just disabling preemption and testing for page ==
NULL (just in case an interrupt comes in between objects and page and
resets that). But we can't grab freelist and page if c points to another
CPUs object.

-- Steve

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2013-01-18 15:04 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-17 18:10 [RFC][PATCH] slub: Check for page NULL before doing the node_match check Steven Rostedt
2013-01-17 18:37 ` Steven Rostedt
2013-01-17 21:28   ` Christoph Lameter
2013-01-17 21:36     ` Eric Dumazet
2013-01-17 21:44       ` Steven Rostedt
2013-01-17 21:43     ` Steven Rostedt
2013-01-17 21:51       ` Christoph Lameter
2013-01-17 22:07         ` Steven Rostedt
2013-01-17 22:46         ` Steven Rostedt
2013-01-17 23:10           ` Steven Rostedt
2013-01-17 23:20             ` [RFC][PATCH] slub: Keep page and object in sync in slab_alloc_node() Steven Rostedt
2013-01-18  0:23               ` Steven Rostedt
2013-01-18  0:28                 ` [RFC][PATCH v2] " Steven Rostedt
2013-01-18  4:42                   ` Joonsoo Kim
2013-01-18 14:52                     ` Christoph Lameter
2013-01-18 15:29                     ` Steven Rostedt
2013-01-18 14:44                   ` Christoph Lameter
2013-01-18 15:04                     ` Steven Rostedt [this message]
2013-01-18 15:55                       ` Steven Rostedt
2013-01-18 18:29                         ` Christoph Lameter
2013-01-18 18:52                           ` Steven Rostedt
2013-01-21  1:48                             ` Christoph Lameter
2013-01-21  8:11                         ` Joonsoo Kim
2013-01-21 12:19                           ` Steven Rostedt
2013-01-18 18:23                       ` Christoph Lameter
2013-01-18 15:09                   ` [RFC][PATCH v3] " Steven Rostedt
2013-01-18 18:40                     ` Christoph Lameter
2013-01-18 19:09                       ` Eric Dumazet
2013-01-18 19:20                         ` Steven Rostedt
2013-01-21  1:40                         ` Christoph Lameter
2013-01-18 14:43             ` [RFC][PATCH] slub: Check for page NULL before doing the node_match check Christoph Lameter
     [not found]       ` <alpine.DEB.2.02.1301171547370.2774@gentwo.org>
2013-01-17 21:56         ` Christoph Lameter
2013-01-17 22:10           ` Steven Rostedt
2013-01-17 21:22 ` Christoph Lameter

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=1358521484.7383.8.camel@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=clark@redhat.com \
    --cc=jkacur@gmail.com \
    --cc=lgoncalv@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mpm@selenic.com \
    --cc=penberg@kernel.org \
    --cc=tglx@linutronix.de \
    /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).