Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756224AbZDFOZ2 (ORCPT ); Mon, 6 Apr 2009 10:25:28 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755111AbZDFOZS (ORCPT ); Mon, 6 Apr 2009 10:25:18 -0400 Received: from e28smtp04.in.ibm.com ([59.145.155.4]:44199 "EHLO e28smtp04.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754312AbZDFOZQ (ORCPT ); Mon, 6 Apr 2009 10:25:16 -0400 Date: Mon, 6 Apr 2009 19:54:52 +0530 From: Gautham R Shenoy To: "Rafael J. Wysocki" Cc: Ingo Molnar , Peter Zijlstra , Rusty Russell , Ming Lei , Andrew Morton , Linux-pm mailing list , Linux Kernel List , Venkatesh Pallipadi Subject: Re: pm-hibernate : possible circular locking dependency detected Message-ID: <20090406142452.GA17559@in.ibm.com> Reply-To: ego@in.ibm.com References: <20090405134454.GB25250@elte.hu> <20090406005515.GA28200@in.ibm.com> <200904061529.44780.rjw@sisk.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200904061529.44780.rjw@sisk.pl> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5203 Lines: 144 On Mon, Apr 06, 2009 at 03:29:43PM +0200, Rafael J. Wysocki wrote: > On Monday 06 April 2009, Gautham R Shenoy wrote: > > On Sun, Apr 05, 2009 at 03:44:54PM +0200, Ingo Molnar wrote: > > > > > > * Rafael J. Wysocki wrote: > > > > > > > On Sunday 05 April 2009, Ming Lei wrote: > > > > > kernel version : one simple usb-serial patch against commit > > > > > 6bb597507f9839b13498781e481f5458aea33620. > > > > > > > > > > Thanks. > > > > > > > > Hmm, CPU hotplug again, it seems. > > > > > > > > I'm not sure who's the maintainer at the moment. Andrew, is that > > > > Gautham? > > > > > > CPU hotplug tends to land on the scheduler people's desk normally. > > > > > > But i'm not sure that's the real thing here - key appears to be this > > > work_on_cpu() worklet by the cpufreq code: > > > > Actually, there are two dependency chains here which can lead to a deadlock. > > The one we're seeing here is the longer of the two. > > > > If the relevant locks are numbered as follows: > > [1]: cpu_policy_rwsem > > [2]: work_on_cpu > > [3]: cpu_hotplug.lock > > [4]: dpm_list_mtx > > > > > > The individual callpaths are: > > > > 1) do_dbs_timer()[1] --> dbs_check_cpu() --> __cpufreq_driver_getavg() > > | > > work_on_cpu()[2] <-- get_measured_perf() <--| > > > > > > 2) pci_device_probe() --> .. --> pci_call_probe() [3] --> work_on_cpu()[2] > > | > > [4] device_pm_add() <-- ..<-- local_pci_probe() <--| > > This should block on [4] held by hibernate(). That's why it calls > device_pm_lock() after all. Agreed. But it does so holding the cpu_hotplug.lock at pci_call_probe(). See below. > > > 3) hibernate() --> hibernatioin_snapshot() --> create_image() > > | > > disable_nonboot_cpus() <-- [4] device_pm_lock() <--| > > | > > |--> _cpu_down() [3] --> cpufreq_cpu_callback() [1] > > > > > > The two chains which can deadlock are > > > > a) [1] --> [2] --> [4] --> [3] --> [1] (The one in this log) > > and > > b) [3] --> [2] --> [4] --> [3] > > What exactly is the b) scenario? pci_call_probe() calls work_on_cpu() within get_online_cpus()/put_online_cpus(), the cpu hotplug read path. Thus we have a cpu_hotplug.lock --> work_on_cpu dependency here. This work_on_cpu() calls local_pci_probe() which, in one of the registration paths calls pcie_portdrv_probe(). This would eventually end up calling device_pm_add() which takes the dpm_list_mtx. Thus we have a work_on_cpu --> dpm_list_mtx dependency here. This is reflected in the lockdep log for dpm_list_mtx: > > [ 2276.033054] -> #3 (dpm_list_mtx){+.+.+.}: > > [ 2276.033057] [] __lock_acquire+0x1402/0x178c > > [ 2276.033061] [] lock_acquire+0x93/0xbf > > [ 2276.033065] [] mutex_lock_nested+0x6a/0x362 > > [ 2276.033068] [] device_pm_add+0x46/0xed > > [ 2276.033073] [] device_add+0x488/0x61a > > [ 2276.033077] [] device_register+0x19/0x1d > > [ 2276.033080] [] pcie_port_device_register+0x450/0x4b6 > > [ 2276.033085] [] pcie_portdrv_probe+0x69/0x82 > > [ 2276.033089] [] local_pci_probe+0x12/0x16 > > [ 2276.033093] [] do_work_for_cpu+0x13/0x1b > > [ 2276.033097] [] worker_thread+0x1b2/0x2c9 > > [ 2276.033100] [] kthread+0x49/0x76 > > [ 2276.033104] [] child_rip+0xa/0x20 > > [ 2276.033107] [] 0xffffffffffffffff The dependency chain on this device_registration path would be cpu_hotplug.lock --> work_on_cpu --> dpm_list_mtx. On the hibernate path, we hold the dpm_list_mtx and call disable_nonboot_cpus() in create_image(). disable_nonboot_cpus() calls _cpu_down() which again takes the cpu_hotplug.lock, this time the write-path. Thus we have a dpm_list_mtx --> cpu_hotplug.lock dependency here. These two dependency chains being in reverse order can cause a dead-lock, right ? Or am I reading something wrong here? > > > > > Rafael, > > Sorry, I am not well versed with the hibernation code. But does the > > following make sense: > > Not really -> > > > create_image() > > { > > device_pm_lock(); > > device_power_down(PMSG_FREEZE); > > platform_pre_snapshot(platform_mode); > > > > device_pm_unlock(); > > -> because dpm_list is under control of the hibernation code at this point > and it should remain locked. > > > disable_nonboot_cpus() > > disable_nonboot_cpus() must not take dpm_list_mtx itself. > > > device_pm_lock(); > > . > > . > > . > > . > > } > > Thanks, > Rafael -- Thanks and Regards gautham -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/