Re: 2.6.25-rc9 -- INFO: possible circular locking dependency detected

[Heiko Carstens <heiko.carstens@de.ibm.com>]

On Mon, Apr 14, 2008 at 09:16:42PM +0530, Gautham R Shenoy wrote:
> On Mon, Apr 14, 2008 at 05:19:46PM +0200, Heiko Carstens wrote:
> > On Mon, Apr 14, 2008 at 08:18:01PM +0530, Gautham R Shenoy wrote:
> > > On Mon, Apr 14, 2008 at 02:42:27PM +0200, Peter Zijlstra wrote:
> > 
> > While you're fixing the cpu hotplug stuff anyway, there's still a bug
> > present in a few modules init code:
> > 
> > Usually they do something like:
> > 
> > 	register_hotcpu_notifier(...);
> > 	for_each_online_cpu(i)
> > 		...
> > 
> > A module's init functions gets called from sys_init_module and there is nothing
> > that would protect from cpu hotplug.
> > Therefore the sequence of for_each_online_cpu() and register_hotcpu_notifier()
> > better should be protected by a surrounding get/put_online_cpus() like this:
> > 
> > 	get_online_cpus();
> > 	register_hotcpu_notifier(...);
> > 	for_each_online_cpu(i)
> > 		...
> > 	put_online_cpus();
> 
> But shouldn't this be:
> 	register_hotcpu_notifier(...);
> 	get_online_cpus();
> 	for_each_online_cpus()
> 		...
> 	put_online_cpus();
> 
> What's the problem with this ordering?

The problem here is that between register_hotcpu_notifier() and
get_online_cpus() a cpu might have been hotplugged.
So on cpu down the registered function might try to undo something that
wasn't prepared in the first place.
On cpu up however it will do things twice. Once for the cpus that got
added between register_hotcpu_notifier() and for_each_online_cpus()
and then again in the for_each_online_cpus() loop.

Of course all of these scenarios could be fixed in each driver, but that
would be a lot of duplicated work. Making sure the combination of
get_online_cpus() and register_hotcpu_notifier() cannot deadlock would
make things much easier.