From: Andrew Morton <akpm@linux-foundation.org>
To: osalvador@techadventures.net
Cc: mhocko@suse.com, vbabka@suse.cz, dan.j.williams@intel.com,
yasu.isimatu@gmail.com, jonathan.cameron@huawei.com,
david@redhat.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org, Oscar Salvador <osalvador@suse.de>
Subject: Re: [PATCH 3/3] mm/memory_hotplug: Cleanup unregister_mem_sect_under_nodes
Date: Fri, 10 Aug 2018 15:37:27 -0700 [thread overview]
Message-ID: <20180810153727.c9ae4aab518f1b84e04c999a@linux-foundation.org> (raw)
In-Reply-To: <20180810152931.23004-4-osalvador@techadventures.net>
On Fri, 10 Aug 2018 17:29:31 +0200 osalvador@techadventures.net wrote:
> From: Oscar Salvador <osalvador@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?
next prev parent reply other threads:[~2018-08-10 22:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-10 15:29 [PATCH 0/3] Refactor/cleanup for remove_memory_section/unregister_mem_sect_under_nodes osalvador
2018-08-10 15:29 ` [PATCH 1/3] mm/memory_hotplug: Drop unused args from remove_memory_section osalvador
2018-08-10 15:29 ` [PATCH 2/3] mm/memory_hotplug: Drop unneeded check from unregister_mem_sect_under_nodes osalvador
2018-08-10 15:29 ` [PATCH 3/3] mm/memory_hotplug: Cleanup unregister_mem_sect_under_nodes osalvador
2018-08-10 22:37 ` Andrew Morton [this message]
2018-08-11 8:08 ` Oscar Salvador
2018-08-13 8:55 ` Oscar Salvador
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180810153727.c9ae4aab518f1b84e04c999a@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=dan.j.williams@intel.com \
--cc=david@redhat.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=osalvador@suse.de \
--cc=osalvador@techadventures.net \
--cc=vbabka@suse.cz \
--cc=yasu.isimatu@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).