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

[Paul Jackson]

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@xxxxxxx> 1.925.600.0401
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/