linux-kernel - Re: Deadlock between cpu_hotplug_begin and cpu_add_remove

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]

Date:	Thu, 23 Jan 2014 11:06:29 +0530
From:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	Paul Mackerras <paulus@...ba.org>, linux-kernel@...r.kernel.org,
	Peter Zijlstra <peterz@...radead.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...nel.org>,
	Oleg Nesterov <oleg@...hat.com>, Tejun Heo <tj@...nel.org>,
	Michel Lespinasse <walken@...gle.com>, ego@...ux.vnet.ibm.com,
	Thomas Gleixner <tglx@...utronix.de>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>
Subject: Re: Deadlock between cpu_hotplug_begin and cpu_add_remove_lock

On 01/23/2014 07:59 AM, Rusty Russell wrote:
> "Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com> writes:
>> On 01/22/2014 02:00 PM, Srivatsa S. Bhat wrote:
>>> Hi Paul,
> 
> I find an old patch for register_allcpu_notifier(), but the "bool
> replay_history" should be eliminated (always true): it's too weird.
> 

Sorry, I didn't get this part. Why do you say that replay_history
will always be true?

replay_history will be set to true whenever the caller wants to
get notified of CPU_UP_PREPARE and CPU_ONLINE notifications for the
already online CPUs, or wants to run a custom setup-routine of its
own. And it will be false whenever the caller simply wants to just
register the callback.

Note that passing NULL for the setup-routine, by itself isn't enough
to make a decision. NULL + replay_history == True will invoke the normal
CPU_UP_PREPARE/CPU_ONLINE notifiers for the already online CPUs before
registering the callback. NULL + replay_history == False will just
register the callback and do nothing else.

> Then we should get rid of register_cpu_notifier, or at least hide it.
> 

Why? Isn't it easier to use (since you don't have to pass 2 additional
parameters)? I see register_allcpu_notifier (or whatever better name
we can give it), as an API for special cases where there is something
more to be done than just registering the callback. And register_cpu_notifier
will continue to be the API for the regular case when the caller wants
to just register the callback. This latter case is the majority in the
kernel. So I don't think eliminating the regular API would be a good idea.

By the way, I'm still tempted to try out the simpler-looking alternative
idea of exporting cpu_maps_update_begin() and cpu_maps_update_done()
and then mandating that the callers do:

cpu_maps_update_begin();
for_each_online_cpu(cpu) {
	...
}

__register_cpu_notifier(); // this doesn't take the add_remove_lock
cpu_maps_update_done();

I'm working on a patchset that does this and performs a tree-wide
conversion. Please let me know if you have any objections to exporting
cpu_maps_update_begin/done() in this manner.

I thought I'd give this solution a try first, before going to the much
fancier register_allcpu_notifier() method.

Regards,
Srivatsa S. Bhat

--
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/