All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Christoph Lameter <cl@linux-foundation.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>,
	Nick Piggin <npiggin@suse.de>, Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	haicheng.li@intel.com,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [patch v2] slab: add memory hotplug support
Date: Sun, 4 Apr 2010 13:45:54 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.00.1004041336450.4184@chino.kir.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1003301141190.24717@router.home>

On Tue, 30 Mar 2010, Christoph Lameter wrote:

> > Nick, Christoph, lets make a a deal: you ACK, I merge. How does that
> > sound to you?
> 
> I looked through the patch before and slabwise this seems to beok but I am
> still not very sure how this interacts with the node and cpu bootstrap.
> You can have the ack with this caveat.
> 
> Acked-by: Christoph Lameter <cl@linux-foundation.org>
> 

Thanks.

I tested this for node hotplug by setting ACPI_SRAT_MEM_HOT_PLUGGABLE 
regions and then setting up a new memory section with 
/sys/devices/system/memory/probe.  I onlined the new memory section, which 
mapped to an offline node, and verified that the nwe nodelists were 
initialized correctly.  This is done before the MEM_ONLINE notifier and 
the bit being set in node_states[N_HIGH_MEMORY].  So, for node hot-add, it 
works.

MEM_GOING_OFFLINE is more interesting, but there's nothing harmful about 
draining the freelist and reporting whether there are existing full or 
partial slabs back to the memory hotplug layer to preempt a hot-remove 
since those slabs cannot be freed.  I don't consider that to be a risky 
change.

As far as the interactions between memory and cpu hotplug, they are really 
different things with many of the same implications for the slab layer.  
Both have the possibility of bringing new nodes online or offline and they 
must be dealt with accordingly.  We lack support for offlining an entire 
node at a time since we must hotplug first by adding a new memory section, 
so these notifiers won't be called simultaneously.  Even if they were, 
draining the freelist and checking if a nodelist needs to be initialized 
is not going to be harmful since both notifiers have the same checks for 
existing nodelists (which is not only necessary if we _did_ have 
simultaneous cpu and memory hot-add, but also if a node transitioned from 
online to offline and back to online).

I hope this patch is merged because it obviously fixed a problem on my box 
where a memory section could be added, a node onlined, and then no slab 
metadata being initialized for that memory.

WARNING: multiple messages have this Message-ID (diff)
From: David Rientjes <rientjes@google.com>
To: Christoph Lameter <cl@linux-foundation.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>,
	Nick Piggin <npiggin@suse.de>, Andi Kleen <andi@firstfloor.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	haicheng.li@intel.com,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [patch v2] slab: add memory hotplug support
Date: Sun, 4 Apr 2010 13:45:54 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.00.1004041336450.4184@chino.kir.corp.google.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1003301141190.24717@router.home>

On Tue, 30 Mar 2010, Christoph Lameter wrote:

> > Nick, Christoph, lets make a a deal: you ACK, I merge. How does that
> > sound to you?
> 
> I looked through the patch before and slabwise this seems to beok but I am
> still not very sure how this interacts with the node and cpu bootstrap.
> You can have the ack with this caveat.
> 
> Acked-by: Christoph Lameter <cl@linux-foundation.org>
> 

Thanks.

I tested this for node hotplug by setting ACPI_SRAT_MEM_HOT_PLUGGABLE 
regions and then setting up a new memory section with 
/sys/devices/system/memory/probe.  I onlined the new memory section, which 
mapped to an offline node, and verified that the nwe nodelists were 
initialized correctly.  This is done before the MEM_ONLINE notifier and 
the bit being set in node_states[N_HIGH_MEMORY].  So, for node hot-add, it 
works.

MEM_GOING_OFFLINE is more interesting, but there's nothing harmful about 
draining the freelist and reporting whether there are existing full or 
partial slabs back to the memory hotplug layer to preempt a hot-remove 
since those slabs cannot be freed.  I don't consider that to be a risky 
change.

As far as the interactions between memory and cpu hotplug, they are really 
different things with many of the same implications for the slab layer.  
Both have the possibility of bringing new nodes online or offline and they 
must be dealt with accordingly.  We lack support for offlining an entire 
node at a time since we must hotplug first by adding a new memory section, 
so these notifiers won't be called simultaneously.  Even if they were, 
draining the freelist and checking if a nodelist needs to be initialized 
is not going to be harmful since both notifiers have the same checks for 
existing nodelists (which is not only necessary if we _did_ have 
simultaneous cpu and memory hot-add, but also if a node transitioned from 
online to offline and back to online).

I hope this patch is merged because it obviously fixed a problem on my box 
where a memory section could be added, a node onlined, and then no slab 
metadata being initialized for that memory.

--
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:[~2010-04-04 20:46 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-11 20:53 [PATCH] [0/4] Update slab memory hotplug series Andi Kleen
2010-02-11 20:53 ` Andi Kleen
2010-02-11 20:54 ` [PATCH] [1/4] SLAB: Handle node-not-up case in fallback_alloc() v2 Andi Kleen
2010-02-11 20:54   ` Andi Kleen
2010-02-11 21:41   ` David Rientjes
2010-02-11 21:41     ` David Rientjes
2010-02-11 21:55     ` Andi Kleen
2010-02-11 21:55       ` Andi Kleen
2010-02-15  6:04   ` Nick Piggin
2010-02-15  6:04     ` Nick Piggin
2010-02-15 10:07     ` Andi Kleen
2010-02-15 10:07       ` Andi Kleen
2010-02-15 10:22       ` Nick Piggin
2010-02-15 10:22         ` Nick Piggin
2010-02-11 20:54 ` [PATCH] [2/4] SLAB: Separate node initialization into separate function Andi Kleen
2010-02-11 20:54   ` Andi Kleen
2010-02-11 21:44   ` David Rientjes
2010-02-11 21:44     ` David Rientjes
2010-02-11 20:54 ` [PATCH] [3/4] SLAB: Set up the l3 lists for the memory of freshly added memory v2 Andi Kleen
2010-02-11 20:54   ` Andi Kleen
2010-02-11 21:45   ` David Rientjes
2010-02-11 21:45     ` David Rientjes
2010-02-15  6:06     ` Nick Piggin
2010-02-15  6:06       ` Nick Piggin
2010-02-15 21:47       ` David Rientjes
2010-02-15 21:47         ` David Rientjes
2010-02-16 14:04         ` Nick Piggin
2010-02-16 14:04           ` Nick Piggin
2010-02-16 20:45           ` Pekka Enberg
2010-02-16 20:45             ` Pekka Enberg
2010-02-11 20:54 ` [PATCH] [4/4] SLAB: Fix node add timer race in cache_reap Andi Kleen
2010-02-11 20:54   ` Andi Kleen
2010-02-11 21:45   ` David Rientjes
2010-02-11 21:45     ` David Rientjes
2010-02-15  6:15   ` Nick Piggin
2010-02-15  6:15     ` Nick Piggin
2010-02-15 10:32     ` Andi Kleen
2010-02-15 10:32       ` Andi Kleen
2010-02-15 10:41       ` Nick Piggin
2010-02-15 10:41         ` Nick Piggin
2010-02-15 10:52         ` Andi Kleen
2010-02-15 10:52           ` Andi Kleen
2010-02-15 11:01           ` Nick Piggin
2010-02-15 11:01             ` Nick Piggin
2010-02-15 15:30             ` Andi Kleen
2010-02-15 15:30               ` Andi Kleen
2010-02-19 18:22             ` Christoph Lameter
2010-02-19 18:22               ` Christoph Lameter
2010-02-20  9:01               ` Andi Kleen
2010-02-20  9:01                 ` Andi Kleen
2010-02-22 10:53                 ` Pekka Enberg
2010-02-22 10:53                   ` Pekka Enberg
2010-02-22 14:31                   ` Andi Kleen
2010-02-22 14:31                     ` Andi Kleen
2010-02-22 16:11                     ` Pekka Enberg
2010-02-22 16:11                       ` Pekka Enberg
2010-02-22 20:20                       ` Andi Kleen
2010-02-22 20:20                         ` Andi Kleen
2010-02-24 15:49                 ` Christoph Lameter
2010-02-24 15:49                   ` Christoph Lameter
2010-02-25  7:26                   ` Pekka Enberg
2010-02-25  7:26                     ` Pekka Enberg
2010-02-25  8:01                     ` David Rientjes
2010-02-25  8:01                       ` David Rientjes
2010-02-25 18:30                       ` Christoph Lameter
2010-02-25 18:30                         ` Christoph Lameter
2010-02-25 21:45                         ` David Rientjes
2010-02-25 21:45                           ` David Rientjes
2010-02-25 22:31                           ` Christoph Lameter
2010-02-25 22:31                             ` Christoph Lameter
2010-02-26 10:45                             ` Pekka Enberg
2010-02-26 10:45                               ` Pekka Enberg
2010-02-26 11:43                               ` Andi Kleen
2010-02-26 11:43                                 ` Andi Kleen
2010-02-26 12:35                                 ` Pekka Enberg
2010-02-26 12:35                                   ` Pekka Enberg
2010-02-26 14:08                                   ` Andi Kleen
2010-02-26 14:08                                     ` Andi Kleen
2010-02-26  1:09                         ` KAMEZAWA Hiroyuki
2010-02-26  1:09                           ` KAMEZAWA Hiroyuki
2010-02-26 11:41                         ` Andi Kleen
2010-02-26 11:41                           ` Andi Kleen
2010-02-26 15:04                           ` Christoph Lameter
2010-02-26 15:04                             ` Christoph Lameter
2010-02-26 15:05                             ` Christoph Lameter
2010-02-26 15:05                               ` Christoph Lameter
2010-02-26 15:59                               ` Andi Kleen
2010-02-26 15:59                                 ` Andi Kleen
2010-02-26 15:57                             ` Andi Kleen
2010-02-26 15:57                               ` Andi Kleen
2010-02-26 17:24                               ` Christoph Lameter
2010-02-26 17:24                                 ` Christoph Lameter
2010-02-26 17:31                                 ` Andi Kleen
2010-02-26 17:31                                   ` Andi Kleen
2010-03-01  1:59                                   ` KAMEZAWA Hiroyuki
2010-03-01  1:59                                     ` KAMEZAWA Hiroyuki
2010-03-01 10:27                                     ` David Rientjes
2010-03-01 10:27                                       ` David Rientjes
2010-02-27  0:01                                 ` David Rientjes
2010-02-27  0:01                                   ` David Rientjes
2010-03-01 10:24                                   ` [patch] slab: add memory hotplug support David Rientjes
2010-03-01 10:24                                     ` David Rientjes
2010-03-02  5:53                                     ` Pekka Enberg
2010-03-02  5:53                                       ` Pekka Enberg
2010-03-02 20:20                                       ` Christoph Lameter
2010-03-02 20:20                                         ` Christoph Lameter
2010-03-02 21:03                                         ` David Rientjes
2010-03-02 21:03                                           ` David Rientjes
2010-03-03  1:28                                         ` KAMEZAWA Hiroyuki
2010-03-03  1:28                                           ` KAMEZAWA Hiroyuki
2010-03-03  2:39                                           ` David Rientjes
2010-03-03  2:39                                             ` David Rientjes
2010-03-03  2:51                                             ` KAMEZAWA Hiroyuki
2010-03-03  2:51                                               ` KAMEZAWA Hiroyuki
2010-03-02 12:53                                     ` Andi Kleen
2010-03-02 12:53                                       ` Andi Kleen
2010-03-02 15:04                                       ` Pekka Enberg
2010-03-02 15:04                                         ` Pekka Enberg
2010-03-03 14:34                                         ` Andi Kleen
2010-03-03 14:34                                           ` Andi Kleen
2010-03-03 15:46                                           ` Christoph Lameter
2010-03-03 15:46                                             ` Christoph Lameter
2010-03-02 21:17                                       ` David Rientjes
2010-03-02 21:17                                         ` David Rientjes
2010-03-05  6:20                                     ` Nick Piggin
2010-03-05  6:20                                       ` Nick Piggin
2010-03-05 12:47                                       ` Anca Emanuel
2010-03-05 12:47                                         ` Anca Emanuel
2010-03-05 13:58                                         ` Anca Emanuel
2010-03-05 13:58                                           ` Anca Emanuel
2010-03-05 14:11                                         ` Christoph Lameter
2010-03-05 14:11                                           ` Christoph Lameter
2010-03-08  3:06                                           ` Andi Kleen
2010-03-08  3:06                                             ` Andi Kleen
2010-03-08  2:58                                         ` Andi Kleen
2010-03-08  2:58                                           ` Andi Kleen
2010-03-08 23:19                                       ` David Rientjes
2010-03-08 23:19                                         ` David Rientjes
2010-03-09 13:46                                         ` Nick Piggin
2010-03-09 13:46                                           ` Nick Piggin
2010-03-22 17:28                                           ` Pekka Enberg
2010-03-22 17:28                                             ` Pekka Enberg
2010-03-22 21:12                                             ` Nick Piggin
2010-03-22 21:12                                               ` Nick Piggin
2010-03-28  2:13                                           ` David Rientjes
2010-03-28  2:13                                             ` David Rientjes
2010-03-28  2:40                                             ` [patch v2] " David Rientjes
2010-03-28  2:40                                               ` David Rientjes
2010-03-30  9:01                                               ` Pekka Enberg
2010-03-30  9:01                                                 ` Pekka Enberg
2010-03-30 16:43                                                 ` Christoph Lameter
2010-03-30 16:43                                                   ` Christoph Lameter
2010-04-04 20:45                                                   ` David Rientjes [this message]
2010-04-04 20:45                                                     ` David Rientjes
2010-04-07 16:29                                               ` Pekka Enberg
2010-04-07 16:29                                                 ` Pekka Enberg
2010-02-25 18:34                     ` [PATCH] [4/4] SLAB: Fix node add timer race in cache_reap Christoph Lameter
2010-02-25 18:34                       ` Christoph Lameter
2010-02-25 18:46                       ` Pekka Enberg
2010-02-25 18:46                         ` Pekka Enberg
2010-02-25 19:19                         ` Christoph Lameter
2010-02-25 19:19                           ` Christoph Lameter
2010-03-02 12:55                         ` Andi Kleen
2010-03-02 12:55                           ` Andi Kleen
2010-02-19 18:22       ` Christoph Lameter
2010-02-19 18:22         ` Christoph Lameter
2010-02-22 10:57         ` Pekka Enberg
2010-02-22 10:57           ` Pekka Enberg
2010-02-13 10:24 ` [PATCH] [0/4] Update slab memory hotplug series Pekka Enberg
2010-02-13 10:24   ` Pekka Enberg

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=alpine.DEB.2.00.1004041336450.4184@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=andi@firstfloor.org \
    --cc=cl@linux-foundation.org \
    --cc=haicheng.li@intel.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npiggin@suse.de \
    --cc=penberg@cs.helsinki.fi \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.