> On Thu, 18 Oct 2007 12:25:37 +0900 Yasunori Goto <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/