Re: [PATCH 2/3] mm: don't rely on system state to detect hot-plug operations

[David Hildenbrand <david@redhat.com>]

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

... and while we're at it, we should rename register_mem_sect_under_node
to something like "register_memory_block_under_node" - "section" is a
legacy leftover here.

We could factor out both sysfs_create_link_nowarn() calls into something
like "do_register_memory_block_under_node" or similar, to minimize code
duplication.

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

I don't particularly like passing around function pointers where it can
be avoided (e.g., here exporting 3 functions now instead 1). Makes the
interface harder to get IMHO. But I don't really care about that
interface, easy to change later on.

-- 
Thanks,

David / dhildenb