Re: [PATCH: 004/017](RFC) Memory hotplug for new nodes v.3. (generic alloc pgdat)

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

> > +#ifdef CONFIG_HAVE_ARCH_NODEDATA_EXTENSION
> > +/*
> > + * For supporint node-hotadd, we have to allocate new pgdat.
> > + *
> > + * If an arch have generic style NODE_DATA(),
> > + * node_data[nid] = kzalloc() works well . But it depends on each arch.
> > + *
> > + * In general, generic_alloc_nodedata() is used.
> > + * generic...is a local function in mm/memory_hotplug.c
> > + *
> > + * Now, arch_free_nodedata() is just defined for error path of node_hot_add.
> > + *
> > + */
> > +extern struct pglist_data * arch_alloc_nodedata(int nid);
> > +extern void arch_free_nodedata(pg_data_t *pgdat);
> > +
> > +#else /* !CONFIG_HAVE_ARCH_NODEDATA_EXTENSION */
> > +#define arch_alloc_nodedata(nid)	generic_alloc_nodedata(nid)
> > +#define arch_free_nodedata(pgdat)	generic_free_nodedata(pgdat)
> > +
> > +#ifdef CONFIG_NUMA
> > +/*
> > + * If ARCH_HAS_NODEDATA_EXTENSION=n, this func is used to allocate pgdat.
> > + */
> > +static inline struct pglist_data *generic_alloc_nodedata(int nid)
> > +{
> > +	return kzalloc(sizeof(struct pglist_data), GFP_ATOMIC);
> > +}
> 
> >From an interface design point of view it's usually best to pass the
> gfp_flags ito a function which performs memory allocation, rather than
> assuming the worst-case like this.
> 
> If it's known that callers of generic_alloc_nodedata() can just never ever
> be permitted to sleep then OK.  But GFP_KERNEL allocations are always
> preferable.

Ok. I'll change GFP_KERNEL for it.

> > +/*
> > + * This definition is just for error path in node hotadd.
> > + * For node hotremove, we have to replace this.
> > + */
> > +static inline void generic_free_nodedata(struct pglist_data *pgdat)
> > +{
> > +	kfree(pgdat);
> > +}
> > +
> > +#else /* !CONFIG_NUMA */
> > +/* never called */
> > +static inline struct pglist_data *generic_alloc_nodedata(int nid)
> > +{
> > +	BUG();
> > +	return NULL;
> > +}
> > +static inline void generic_free_nodedata(struct pglist_data *pgdat)
> > +{
> > +}
> > +#endif /* CONFIG_NUMA */
> > +#endif /* CONFIG_HAVE_ARCH_NODEDATA_EXTENSION */
> > +
> 
> Should the patch provide stubs for generic_alloc_nodedata() and
> generic_alloc_nodedata() if !CONFIG_HAVE_ARCH_NODEDATA_EXTENSION?
> 
> (If all callers are also inside #ifdef CONFIG_HAVE_ARCH_NODEDATA_EXTENSION
> then the answer would be "no").

No. 
They are stubs for !CONFIG_HAVE_ARCH_NODEDATA_EXTENSION.
They are inside of !CONFIG case. Not for special archtectures.
I intend that if an architecture needs some kind of extension, 
it should define CONFIG_HAVE_ARCH..... and arch_alloc_nodedata(nid).

Did I make mistake comment for #ifdef?

Bye.

-- 
Yasunori Goto 


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