linux-kernel - Re: [PATCH] Avoid mask based num_possible_cpus and num_online

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CABeCy1bgRe8=KKimBtB39F9KaJko=aD7xqYfAT0t6YcS6CMLZA@mail.gmail.com>
Date:	Mon, 23 Jan 2012 11:28:49 -0800
From:	Venki Pallipadi <venki@...gle.com>
To:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
Cc:	KOSAKI Motohiro <kosaki.motohiro@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mike Travis <travis@....com>,
	"Paul E. McKenney" <paul.mckenney@...aro.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Avoid mask based num_possible_cpus and num_online_cpus

On Sun, Jan 22, 2012 at 9:22 PM, Srivatsa S. Bhat
<srivatsa.bhat@...ux.vnet.ibm.com> wrote:
> On 01/21/2012 05:25 AM, Venki Pallipadi wrote:
>
>> On Fri, Jan 20, 2012 at 3:45 PM, KOSAKI Motohiro
>> <kosaki.motohiro@...il.com> wrote:
>>>>>> +int nr_online_cpus __read_mostly;
>>>>>> +EXPORT_SYMBOL(nr_online_cpus);
>>>>>> +
>>>>>>  void set_cpu_possible(unsigned int cpu, bool possible)
>>>>>>  {
>>>>>>       if (possible)
>>>>>
>>>>>
>>>>> Did you forget to add:
>>>>>
>>>>>        nr_possible_cpus = cpumask_weight(cpu_possible_mask);
>>>>>
>>>>> inside set_cpu_possible() ?
>>>>
>>>> No. That was intentional as I have that coupled with nr_cpu_ids and
>>>> set once after all the bits are set in setup_nr_cpu_ids() instead of
>>>> doing for each bit set.
>>>
>>> But, Srivatsa's way seems more safer, no? Is there any advantage to make couple
>>> with nr_cpu_ids?
>>
>> I think it is a tradeoff between safer and cleaner :). infact, that's
>> how I had coded the patch first. But, then I changed it to be in sync
>> with nr_cpu_ids as it seemed a bit cleaner (and also to make sure 2048
>> CPU guys won't come after me for doing the mask calculation 2048 times
>> during the boot).
>>
>
>
> I knew you were trying to optimize further when I saw your patch. That's
> precisely the reason I cross-checked the code to ensure that the optimization
> didn't go beyond correctness :)
>
> And this is what I found:
>
> start_kernel()
>  setup_nr_cpu_ids() // This is not the end of setting up cpu_possible_mask
>  rest_init()
>    kernel_init()
>      smp_prepare_cpus();
>
> And on x86, this becomes:
>      native_smp_prepare_cpus();
>        smp_sanity_check(); // cpu_possible_mask & nr_cpu_ids can change here!
>           ^^^^^^^^^
>

Ack. Thanks for pointing this out. I missed this in my testing as this is under
if !defined(CONFIG_X86_BIGSMP) && defined(CONFIG_X86_32)

Proper way to handle this would be to call
setup_nr_cpu_ids();
After set_cpu_possible() loop.

I did a grep for any other place where nr_cpu_ids may be getting
written. And only other place I find
arch/powerpc/platforms/iseries/setup.c: nr_cpu_ids = max(nr_cpu_ids, 64);
but it does not have a accompanying set_cpu_possible(). So, the change
here should not affect this.

-Venki

> And there is another place where things can change:
> prefill_possible_map(). But this is called in setup_arch(), which is called
> before setup_nr_cpu_ids(). So we need not worry about this.
>
> (Btw, I checked only the x86 arch. Not sure how other architectures handle
> things.)
>
> Regards,
> Srivatsa S. Bhat
> IBM Linux Technology Center
>
--
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/