On Mon, Sep 14, 2020 at 09:57:46AM +0200, David Hildenbrand wrote:
> > /* register memory section under specified node if it spans that node */
> > +struct rmsun_args {
> > + int nid;
> > + enum memplug_context context;
> > +};
Uhmf, that is a not so descriptive name.
> Instead of handling this in register_mem_sect_under_node(), I
> think it would be better two have two separate
> register_mem_sect_under_node() implementations.
>
> static int register_mem_sect_under_node_hotplug(struct memory_block *mem_blk,
> void *arg)
> {
> const int nid = *(int *)arg;
> int ret;
>
> /* Hotplugged memory has no holes and belongs to a single node. */
> mem_blk->nid = nid;
> ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
> &mem_blk->dev.kobj,
> kobject_name(&mem_blk->dev.kobj));
> if (ret)
> returnr et;
> return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
> &node_devices[nid]->dev.kobj,
> kobject_name(&node_devices[nid]->dev.kobj));
>
> }
>
> Cleaner, right? :) No unnecessary checks.
I tend to agree here, I like more a simplistic version for hotplug.
> One could argue if link_mem_section_hotplug() would be better than passing around the context.
I am not sure if I would duplicate the code there.
We could just pass the pointer of the function we want to call to
link_mem_sections? either register_mem_sect_under_node_hotplug or
register_mem_sect_under_node_early?
Would not that be clean and clear enough?
--
Oscar Salvador
SUSE L3
|