[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20071018173526.515B.Y-GOTO@jp.fujitsu.com>
Date: Thu, 18 Oct 2007 18:20:50 +0900
From: Yasunori Goto <y-goto@...fujitsu.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Christoph Lameter <clameter@....com>,
Linux Kernel ML <linux-kernel@...r.kernel.org>,
linux-mm <linux-mm@...ck.org>
Subject: Re: [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@...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?
Sure. IBM's powerpc boxes have to support memory hotplug even if it
is non-numa machine. They have the Dynamic Logical Partitioning feature.
> > + 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 ;)
Oh... Ok!
> > + 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.
Oops. Indeed. Thanks for your check.
Bye.
--
Yasunori Goto
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists