lkml.org 
[lkml]   [2007]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Patch](memory hotplug) Make kmem_cache_node for SLUB on memory online to avoid panic(take 3)
On Thu, 18 Oct 2007 12:25:37 +0900 Yasunori Goto <y-goto@jp.fujitsu.com> wrote:

>
> This patch fixes panic due to access NULL pointer
> of kmem_cache_node at discard_slab() after memory online.
>
> When memory online is called, kmem_cache_nodes are created for
> all SLUBs for new node whose memory are available.
>
> slab_mem_going_online_callback() is called to make kmem_cache_node()
> in callback of memory online event. If it (or other callbacks) fails,
> then slab_mem_offline_callback() is called for rollback.
>
> In memory offline, slab_mem_going_offline_callback() is called to
> shrink all slub cache, then slab_mem_offline_callback() is called later.
>
> This patch is tested on my ia64 box.
>
> ...
>
> +#if defined(CONFIG_NUMA) && defined(CONFIG_MEMORY_HOTPLUG)

hm. There should be no linkage between memory hotpluggability and
NUMA, surely?

> +static int slab_mem_going_offline_callback(void *arg)
> +{
> + struct kmem_cache *s;
> +
> + down_read(&slub_lock);
> + list_for_each_entry(s, &slab_caches, list)
> + kmem_cache_shrink(s);
> + up_read(&slub_lock);
> +
> + return 0;
> +}
> +
> +static void slab_mem_offline_callback(void *arg)
> +{
> + struct kmem_cache_node *n;
> + struct kmem_cache *s;
> + struct memory_notify *marg = arg;
> + int offline_node;
> +
> + offline_node = marg->status_change_nid;
> +
> + /*
> + * If the node still has available memory. we need kmem_cache_node
> + * for it yet.
> + */
> + if (offline_node < 0)
> + return;
> +
> + down_read(&slub_lock);
> + list_for_each_entry(s, &slab_caches, list) {
> + n = get_node(s, offline_node);
> + if (n) {
> + /*
> + * if n->nr_slabs > 0, slabs still exist on the node
> + * that is going down. We were unable to free them,
> + * and offline_pages() function shoudn't call this
> + * callback. So, we must fail.
> + */
> + BUG_ON(atomic_read(&n->nr_slabs));

Expereince tells us that WARN_ON is preferred for newly added code ;)

> + s->node[offline_node] = NULL;
> + kmem_cache_free(kmalloc_caches, n);
> + }
> + }
> + up_read(&slub_lock);
> +}
> +
> +static int slab_mem_going_online_callback(void *arg)
> +{
> + struct kmem_cache_node *n;
> + struct kmem_cache *s;
> + struct memory_notify *marg = arg;
> + int nid = marg->status_change_nid;
> +
> + /*
> + * If the node's memory is already available, then kmem_cache_node is
> + * already created. Nothing to do.
> + */
> + if (nid < 0)
> + return 0;
> +
> + /*
> + * We are bringing a node online. No memory is availabe yet. We must
> + * allocate a kmem_cache_node structure in order to bring the node
> + * online.
> + */
> + down_read(&slub_lock);
> + list_for_each_entry(s, &slab_caches, list) {
> + /*
> + * XXX: kmem_cache_alloc_node will fallback to other nodes
> + * since memory is not yet available from the node that
> + * is brought up.
> + */
> + n = kmem_cache_alloc(kmalloc_caches, GFP_KERNEL);
> + if (!n)
> + return -ENOMEM;

err, we forgot slub_lock. I'll fix that.

> + init_kmem_cache_node(n);
> + s->node[nid] = n;
> + }
> + up_read(&slub_lock);
> +
> + return 0;
> +}

So that's slub. Does slab already have this functionality or are you
not bothering to maintain slab in this area?

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

\
 
 \ /
  Last update: 2007-10-18 05:49    [W:0.380 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site