On Fri, 10 Aug 2018 17:29:31 +0200 osalva...@techadventures.net wrote:
> From: Oscar Salvador <osalva...@suse.de>
>
> With the assumption that the relationship between
> memory_block <-> node is 1:1, we can refactor this function a bit.
>
> This assumption is being taken from register_mem_sect_under_node()
> code.
>
> register_mem_sect_under_node() takes the mem_blk's nid, and compares it
> to the pfn's nid we are checking.
> If they match, we go ahead and link both objects.
> Once done, we just return.
>
> So, the relationship between memory_block <-> node seems to stand.
>
> Currently, unregister_mem_sect_under_nodes() defines a nodemask_t
> which is being checked in the loop to see if we have already unliked certain
> node.
"unlinked a certain node"
> But since a memory_block can only belong to a node, we can drop the nodemask
"to a single node"?
> and the check within the loop.
>
> If we find a match between the mem_block->nid and the nid of the
> pfn we are checking, we unlink the objects and return, as unlink the objects
"unlinking"
> once is enough.
>
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -448,35 +448,27 @@ int register_mem_sect_under_node(struct memory_block
> *mem_blk, void *arg)
> return 0;
> }
>
> -/* unregister memory section under all nodes that it spans */
> +/* unregister memory section from the node it belongs to */
> int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
> unsigned long phys_index)
> {
> - NODEMASK_ALLOC(nodemask_t, unlinked_nodes, GFP_KERNEL);
> unsigned long pfn, sect_start_pfn, sect_end_pfn;
> -
> - if (!unlinked_nodes)
> - return -ENOMEM;
> - nodes_clear(*unlinked_nodes);
> + int nid = mem_blk->nid;
>
> sect_start_pfn = section_nr_to_pfn(phys_index);
> sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
> for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
> - int nid;
> + int page_nid = get_nid_for_pfn(pfn);
>
> - nid = get_nid_for_pfn(pfn);
> - if (nid < 0)
> - continue;
> - if (!node_online(nid))
> - continue;
> - if (node_test_and_set(nid, *unlinked_nodes))
> - continue;
> - sysfs_remove_link(&node_devices[nid]->dev.kobj,
> - kobject_name(&mem_blk->dev.kobj));
> - sysfs_remove_link(&mem_blk->dev.kobj,
> - kobject_name(&node_devices[nid]->dev.kobj));
> + if (page_nid >= 0 && page_nid == nid) {
> + sysfs_remove_link(&node_devices[nid]->dev.kobj,
> + kobject_name(&mem_blk->dev.kobj));
> + sysfs_remove_link(&mem_blk->dev.kobj,
> + kobject_name(&node_devices[nid]->dev.kobj));
> + break;
> + }
> }
> - NODEMASK_FREE(unlinked_nodes);
> +
> return 0;
> }
I guess so. But the node_online() check was silently removed?
