linux-kernel - Re: memory hotplug - looking for good place for cpuset hook

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20060825095718.9e22e777.pj@sgi.com>
Date:	Fri, 25 Aug 2006 09:57:18 -0700
From:	Paul Jackson <pj@....com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc:	haveblue@...ibm.com, linux-kernel@...r.kernel.org, anton@...ba.org,
	simon.derr@...l.net, nathanl@...tin.ibm.com, akpm@...l.org,
	y-goto@...fujitsu.com
Subject: Re: memory hotplug - looking for good place for cpuset hook

Kame wrote:
> maybe
> 
> if (new_pgdat) {
> 	register_one_node(nid); <-- add sysfs entry of node
> 	<here>
> }

I am not sure why you suggest this, but perhaps you ar saying that I
don't need to call my new cpuset hook (that tracks node_online_map)
everytime we set a bit here in node_online_map, but only when we set a
bit that wasn't previously set.

Hmmm ... this mm/memory_hotplug.c:add_memory() code seems to have
more code than it needs:

================================================================
int add_memory(int nid, u64 start, u64 size)
{
        pg_data_t *pgdat = NULL;
        int new_pgdat = 0;
        ...
        if (!node_online(nid)) {
                pgdat = hotadd_new_pgdat(nid, start);
                if (!pgdat)
                        return -ENOMEM;
		new_pgdat = 1;
                ...
        }
        ...
        /* we online node here. we can't roll back from here. */
        node_set_online(nid);

        if (new_pgdat) {
                ret = register_one_node(nid);                         
================================================================

It looks like the call to node_set_online() could also be called
only if it wasn't already set, and that the 'new_pgdat' is not
needed as we can just test for pgdat != NULL.

That leads to this code that's a couple lines shorter:

================================================================
int add_memory(int nid, u64 start, u64 size)
{
        pg_data_t *pgdat = NULL;
        ...
        if (!node_online(nid)) {
                pgdat = hotadd_new_pgdat(nid, start);
                if (!pgdat)
                        return -ENOMEM;
               ...
        }
        ...
        if (pgdat) {
                /* we online node here. we can't roll back from here. */
                node_set_online(nid);
                ret = register_one_node(nid);                         
================================================================

Is this second code chunk just as good?

I'd still be inclined to add my new cpuset hook to track
node_online_map right after the node_set_online() call, since
that's what changes node_online_map.  I don't think I care
whether or not the "sysfs entry of node" is setup or not.

> (When I implements node-hotplug invoked by cpu-hotplug, I'll care cpuset.)

Good - thanks.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@....com> 1.925.600.0401
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/