Messages in this thread | | | Date | Mon, 14 Apr 2008 21:35:39 +0200 | From | Heiko Carstens <> | Subject | Re: 2.6.25-rc9 -- INFO: possible circular locking dependency detected |
| |
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.
| |