Re: [PATCH] Update sched_domains_numa_masks when new cpus are onlined.

Tang Chen <[email protected]> Tue, 25 Sep 2012 19:45:50 +0800
Newsgroups org.kernel.vger.linux-numa,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
Hi Peter~

Sorry about the confusing log, and thanks for the patient. :)

Here, I want to say something more about the sched_domains_numa_levels
to make myself more clear. :)


Let's have an example here.

sched_init_numa()
{
	...
	// A loop set sched_domains_numa_levels to level.-------------1

	// I set sched_domains_numa_levels to 0.
	sched_domains_numa_levels = 0;--------------------------------2

	// A loop allocating memory for sched_domains_numa_masks[][]
	for (i = 0; i < level; i++) {
		......
		// Allocate memory for sched_domains_numa_masks[i]----3
		......
	}
	......

	// I reset sched_domains_numa_levels to level.
	sched_domains_numa_levels = level;----------------------------4
}

// A new function I added.
static void sched_domains_numa_masks_clear(int cpu)
{
         int i, j;
         for (i = 0; i < sched_domains_numa_levels; i++)---------------5
                 for (j = 0; j < nr_node_ids; j++)
                         cpumask_clear_cpu(cpu, 
sched_domains_numa_masks[i][j]);
}


Suppose level is 10, and at step 1, sched_domains_numa_levels is 10.

If I didn't set sched_domains_numa_levels to 0 at step 2, it will be 10
all the time.

If memory allocation at step 3 fails when i = 5, the array
sched_domains_numa_masks[][] will only have 5 members, and
sched_domains_numa_levels is 10.

As you see, I added 2 functions using sched_domains_numa_levels to
iterate sched_domains_numa_masks[][], such as at step 5.
In this case, the iteration will break out when i = 5.

This could be dangerous.

So, I set sched_domains_numa_levels to 0 at step 2. This way, even if
any memory allocation at step 3 fails, and sched_init_numa() returns,
anyone uses sched_domains_numa_levels (which is 0) won't be wrong.

I'm not sure if this is the best way to settle this problem.
If you have a better idea, please tell me. Thanks. :)


On 09/25/2012 06:33 PM, Peter Zijlstra wrote:
> On Tue, 2012-09-25 at 10:39 +0800, Tang Chen wrote:
>>>> @@ -6765,11 +6773,64 @@ static void sched_init_numa(void)
>>>>       }
>>>>
>>>>       sched_domain_topology = tl;
>>>> +
>>>> +    sched_domains_numa_levels = level;
>>
>> And I set it to level here again.
>>
> But its already set there.. its set every time we find a new level.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>