Re: [Patch 002/002] Create/delete kmem_cache_node for SLUB on memory online callback

[Yasunori Goto <y-goto@jp.fujitsu.com>]

> On Fri, 12 Oct 2007, Yasunori Goto wrote:
> 
> > > > +	down_read(&slub_lock);
> > > > +	list_for_each_entry(s, &slab_caches, list) {
> > > > +		local_node = page_to_nid(virt_to_page(s));
> > > > +		if (local_node == offline_node)
> > > > +			/* This slub is on the offline node. */
> > > > +			return -EBUSY;
> > > > +	}
> > > > +	up_read(&slub_lock);
> > > 
> > > So this checks if the any kmem_cache structure is on the offlined node? If
> > > so then we cannot offline the node?
> > 
> > Right. If slabs' migration is possible, here would be good place for
> > doing it. But, it is not possible (at least now).
> 
> I think you can avoid this check. The kmem_cache structures are allocated 
> from the kmalloc array. The check if the kmalloc slabs are empty will fail 
> if kmem_cache structures still exist on the node.

Ah, Ok.


> 
> > > > +			 * because the node is used by slub yet.
> > > > +			 */
> > > 
> > > It may be clearer to say:
> > > 
> > > "If nr_slabs > 0 then slabs still exist on the node that is going down.
> > > We were unable to free them so we must fail."
> > 
> > Again. If nr_slabs > 0, offline_pages must be fail due to slabs
> > remaining on the node before. So, this callback isn't called.
> 
> Ok then we can remove these checks?

Hmm. Yes. I'll remove it.

> 
> > > > +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 already has memory, then nothing is necessary. */
> > > > +	if (nid < 0)
> > > > +		return 0;
> > > 
> > > The node must have memory????  Or we have already brought up the code?
> > 
> > kmem_cache_node is created at boot time if the node has memory.
> > (Or, it is created by this callback on first added memory on the node).
> > 
> > When nid = - 1, kmem_cache_node is created before this node due to
> > node has memory. 
> 
> So the function can be called for a node that is already online?

already "node memory available", accurately ;-)


> 
> > > > +	 * New memory will be onlined on the node which has no memory so far.
> > > > +	 * New kmem_cache_node is necssary for it.
> > > 
> > > "We are bringing a node online. No memory is available yet. We must 
> > > allocate a kmem_cache_node structure in order to bring the node online." ?
> > 
> > Your mention might be ok.
> > But. I would like to prefer to define status of node hotplug for
> > exactitude like followings
> > 
> > 
> > A)Node online -- pgdat is created and can be accessed for this node.
> >                  but there are no gurantee that cpu or memory is onlined.
> >                  This status is very close from memory-less node.
> >                  But this might be halfway status for node hotplug.
> >                  Node online bit is set. But N_HIGH_MEMORY
> >                  (or N_NORMAL_MEMORY) might be not set.
> 
> Ahh.. Okay.
> 
> > B)Node has memory--
> >                  one or more sections memory is onlined on the node.
> >                  N_HIGH_MEMORY (or N_NORMAL_MEMORY) is set.
> > 
> > If first memory is onlined on the node, the node status changes
> > from A) to B).
> > 
> > I feel this is very useful to manage "halfway status" of node
> > hotplug. (So, memory-less node patch is very helpful for me.)
> > 
> > So, I would like to avoid using the word "node online" at here.
> > But, if above definition is messy for others, I'll change it.
> 
> Ok can we talk about this as
> 
> 	node online
> 
> and
> 
> 	node memory available?

Yes. Thanks.


-- 
Yasunori Goto 


-
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/