linux-kernel - RE: [PATCH 2/2] i7300

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: <7E82351C108FA840AB1866AC776AEC4637D389DB@orsmsx505.amr.corp.intel.com>
Date:	Wed, 22 Oct 2008 21:59:30 -0700
From:	"Pallipadi, Venkatesh" <venkatesh.pallipadi@...el.com>
To:	Andi Kleen <ak@...ux.intel.com>
CC:	Len Brown <lenb@...nel.org>, Ingo Molnar <mingo@...e.hu>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"Henroid, Andrew D" <andrew.d.henroid@...el.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	"paulmck@...ibm.com" <paulmck@...ibm.com>
Subject: RE: [PATCH 2/2] i7300_idle driver v1.55



>-----Original Message-----
>From: Andi Kleen [mailto:ak@...ux.intel.com]
>Sent: Wednesday, October 22, 2008 6:11 PM
>To: Pallipadi, Venkatesh
>Cc: Len Brown; Ingo Molnar; linux-acpi@...r.kernel.org; Linux
>Kernel Mailing List; Henroid, Andrew D; Linus Torvalds; Thomas
>Gleixner; H. Peter Anvin; paulmck@...ibm.com
>Subject: Re: [PATCH 2/2] i7300_idle driver v1.55
>
>>>>> ;
>>>>>> +static void __exit i7300_idle_exit(void)
>>>>>> +{
>>>>>> +     idle_notifier_unregister(&i7300_idle_nb);
>>>>> I still think this needs some kind of idle synchronization.
>>>> This unregister uses atomic_notifier_chain_unregister() which
>>>> uses RCU and handles the race conditions that way.
>>> I didn't do a full analysis, but I didn't think RCU protects
>>> against the idle handler itself.  At least not standard RCU.
>>> You would need to go through at least two quiescent periods,
>>> not one as standard code.
>>>
>>> There used to be an older
>>> idle synchronization interface for this case (which was removed
>>> at some point), perhaps it needs to be readded.
>>>
>>> Or just use stop_machine(). That protects against idle too
>>> and while it's a big hammer for unloading it should be ok.
>>>
>>
>> stop_machine() for unregistering idle callback will deadlock
>due to RCU.
>>
>> I am not sure why we need two cycles. RCU in the unregister makes
>> sure that no CPU is executing the callback handler
>> and only then unregister
>> returns. So, that will mean no one can be in the callback
>rountine and we are
>> safe to remove the module. Correct?
>
>Perhaps I'm confused, but I thought with noidlehz the RCU code
>got changed to take out idle CPUs out of the rcu masks. This makes
>normally sense because you don't want to delay the grace period
>for potentially very long sleeping CPUs.
>
>But it means it won't work for code inside the idle loop. Ok maybe
>if you're very careful with the ordering, but I'm not sure that's
>the case currently.
>
>Yes two cycles won't work either, sorry was confused earlier,
>because even the second cycle wouldn't force the CPU out of idle.
>
>The old idle sync code was more a thread that just cycled through
>all the CPus. That would work.
>

This should not matter as we can safely remove the module even when
one or more CPUs are idle. As long as one CPU is running, we are sure
that throttling is not in effect. And as long as we make sure no one
is executing the notifier that is going to be rmmoded, we should
be OK. Say one CPU comes out of the idle later, it wont see the notifier
in the notifier list anymore, because of RCU in atomic notifier
and things will work fine. I mean we are not rmmoding the routine
that enters the idle state. We are just rmmoding the routine that
Can get called before and after idle routine.

Thanks,
Venki
--
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/