All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Thomas Renninger <trenn@suse.de>
Cc: Dave Jones <davej@redhat.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Yinghai Lu <yinghai@kernel.org>, Avi Kivity <avi@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	cpufreq@vger.kernel.org, mark.langsdorf@amd.com, "Pallipadi,
	Venkatesh" <venkatesh.pallipadi@intel.com>
Subject: Re: [PATCH] cpufreq: remove dbs_mutex
Date: Tue, 23 Jun 2009 20:40:40 +0200	[thread overview]
Message-ID: <20090623184040.GA6908@elte.hu> (raw)
In-Reply-To: <20090623181748.GA31148@elte.hu>


* Ingo Molnar <mingo@elte.hu> wrote:

> * Thomas Renninger <trenn@suse.de> wrote:
> 
> > > Note, this bug warning still triggers rather frequently with 
> > > latest -git (fb20871) during bootup on two test-systems - 
> > > relevant portion of the bootlog attached below. As usual i can 
> > > test any fix for this.
> >
> > Best rip out the dbs_mutex in drivers/cpufreq/cpufreq_ondemand.c 
> > totally. I can provide several locking cleanups for cpufreq for 
> > .31 the next days, including dbs_mutex removal, which I think is 
> > not needed. The dbs_mutex removal which should fix this could then 
> > be marked: CC: stable@kernel.org
> 
> drivers/cpufreq/cpufreq_conservative.c too i guess?
> 
> Something like the patch below?
> 
> Utterly untested and such.

i tested it and this blatant blind ripping out of a layer of locking 
uncovered the next layer:

[  144.961483] =======================================================
[  144.961685] [ INFO: possible circular locking dependency detected ]
[  144.961785] 2.6.30-tip-08973-gb747c8d-dirty #6295
[  144.961878] -------------------------------------------------------
[  144.961974] S99local/8461 is trying to acquire lock:
[  144.962016]  (&(&dbs_info->work)->work){+.+...}, at: [<c109962a>] wait_on_work+0x0/0xba
[  144.962016] 
[  144.962016] but task is already holding lock:
[  144.962016]  (&per_cpu(cpu_policy_rwsem, cpu)){+++++.}, at: [<c1f5dd3f>] lock_policy_rwsem_write+0x73/0xec
[  144.962016] 
[  144.962016] which lock already depends on the new lock.

(see below for the full details)

I guess someone who knows the cpufreq code will have to fix the 
locking in this code for real.

	Ingo

[  144.767335] CPUFREQ: ondemand sampling_rate_max sysfs file is deprecated - used by: cat
[  144.961480] 
[  144.961483] =======================================================
[  144.961685] [ INFO: possible circular locking dependency detected ]
[  144.961785] 2.6.30-tip-08973-gb747c8d-dirty #6295
[  144.961878] -------------------------------------------------------
[  144.961974] S99local/8461 is trying to acquire lock:
[  144.962016]  (&(&dbs_info->work)->work){+.+...}, at: [<c109962a>] wait_on_work+0x0/0xba
[  144.962016] 
[  144.962016] but task is already holding lock:
[  144.962016]  (&per_cpu(cpu_policy_rwsem, cpu)){+++++.}, at: [<c1f5dd3f>] lock_policy_rwsem_write+0x73/0xec
[  144.962016] 
[  144.962016] which lock already depends on the new lock.
[  144.962016] 
[  144.962016] 
[  144.962016] the existing dependency chain (in reverse order) is:
[  144.962016] 
[  144.962016] -> #1 (&per_cpu(cpu_policy_rwsem, cpu)){+++++.}:
[  144.962016]        [<c10bcd0d>] check_prev_add+0xf0/0x151
[  144.962016]        [<c10bcdd3>] check_prevs_add+0x65/0xbf
[  144.962016]        [<c10bce9e>] validate_chain+0x71/0x99
[  144.962016]        [<c10bd184>] __lock_acquire+0x2be/0x33d
[  144.962016]        [<c10bd27f>] lock_acquire+0x7c/0x9f
[  144.962016]        [<c23b1b36>] down_write+0x32/0x95
[  144.962016]        [<c1f5dd3f>] lock_policy_rwsem_write+0x73/0xec
[  144.962016]        [<c1f627cd>] do_dbs_timer+0x50/0x160
[  144.962016]        [<c1098de1>] run_workqueue+0xec/0x243
[  144.962016]        [<c109badf>] worker_thread+0x13b/0x14c
[  144.962016]        [<c10a05ed>] kthread+0x89/0x92
[  144.962016]        [<c10064a7>] kernel_thread_helper+0x7/0x10
[  144.962016]        [<ffffffff>] 0xffffffff
[  144.962016] 
[  144.962016] -> #0 (&(&dbs_info->work)->work){+.+...}:
[  144.962016]        [<c10bcc50>] check_prev_add+0x33/0x151
[  144.962016]        [<c10bcdd3>] check_prevs_add+0x65/0xbf
[  144.962016]        [<c10bce9e>] validate_chain+0x71/0x99
[  144.962016]        [<c10bd184>] __lock_acquire+0x2be/0x33d
[  144.962016]        [<c10bd27f>] lock_acquire+0x7c/0x9f
[  144.962016]        [<c1099662>] wait_on_work+0x38/0xba
[  144.962016]        [<c109975c>] __cancel_work_timer+0x78/0x99
[  144.962016]        [<c109978d>] cancel_delayed_work_sync+0x10/0x12
[  144.962016]        [<c1f62710>] dbs_timer_exit+0x17/0x19
[  144.962016]        [<c1f62d68>] cpufreq_governor_dbs+0x23f/0x2df
[  144.962016]        [<c1f5e7cb>] __cpufreq_governor+0x9a/0xde
[  144.962016]        [<c1f5ea3c>] __cpufreq_set_policy+0x22d/0x2fa
[  144.967630]        [<c1f5ebce>] store_scaling_governor+0xc5/0x108
[  144.967630]        [<c1f5e11d>] store+0xa4/0xbd
[  144.967630]        [<c11fa00f>] flush_write_buffer+0x6d/0x81
[  144.967630]        [<c11fb23f>] sysfs_write_file+0x66/0xa6
[  144.967630]        [<c11814e0>] vfs_write+0x1ad/0x1f9
[  144.967630]        [<c1181fc6>] sys_write+0x5e/0x80
[  144.967630]        [<c100582b>] sysenter_do_call+0x12/0x38
[  144.967630]        [<ffffffff>] 0xffffffff
[  144.967630] 
[  144.967630] other info that might help us debug this:
[  144.967630] 
[  144.967630] 2 locks held by S99local/8461:
[  144.967630]  #0:  (&buffer->mutex){+.+.+.}, at: [<c11fb201>] sysfs_write_file+0x28/0xa6
[  144.967630]  #1:  (&per_cpu(cpu_policy_rwsem, cpu)){+++++.}, at: [<c1f5dd3f>] lock_policy_rwsem_write+0x73/0xec
[  144.967630] 
[  144.967630] stack backtrace:
[  144.967630] Pid: 8461, comm: S99local Tainted: G        W  2.6.30-tip-08973-gb747c8d-dirty #6295
[  144.967630] Call Trace:
[  144.967630]  [<c10bb9d8>] print_circular_bug_tail+0x5d/0x68
[  144.967630]  [<c10bcc50>] check_prev_add+0x33/0x151
[  144.967630]  [<c10b8974>] ? list_add_tail_rcu+0xd/0xf
[  144.967630]  [<c10bcdd3>] check_prevs_add+0x65/0xbf
[  144.967630]  [<c10bce9e>] validate_chain+0x71/0x99
[  144.967630]  [<c10bd184>] __lock_acquire+0x2be/0x33d
[  144.967630]  [<c10bd27f>] lock_acquire+0x7c/0x9f
[  144.967630]  [<c109962a>] ? wait_on_work+0x0/0xba
[  144.967630]  [<c1099662>] wait_on_work+0x38/0xba
[  144.967630]  [<c109962a>] ? wait_on_work+0x0/0xba
[  144.967630]  [<c110c292>] ? ftrace_likely_update+0x11/0x22
[  144.967630]  [<c109975c>] __cancel_work_timer+0x78/0x99
[  144.967630]  [<c109978d>] cancel_delayed_work_sync+0x10/0x12
[  144.967630]  [<c1f62710>] dbs_timer_exit+0x17/0x19
[  144.967630]  [<c1f62d68>] cpufreq_governor_dbs+0x23f/0x2df
[  144.967630]  [<c1f5e7cb>] __cpufreq_governor+0x9a/0xde
[  144.967630]  [<c1f5ea3c>] __cpufreq_set_policy+0x22d/0x2fa
[  144.967630]  [<c1f5ebce>] store_scaling_governor+0xc5/0x108
[  144.967630]  [<c1f60123>] ? handle_update+0x0/0x2d
[  144.967630]  [<c1f5dd6f>] ? lock_policy_rwsem_write+0xa3/0xec
[  144.967630]  [<c1f5e11d>] store+0xa4/0xbd
[  144.967630]  [<c11fa00f>] flush_write_buffer+0x6d/0x81
[  144.967630]  [<c11fb23f>] sysfs_write_file+0x66/0xa6
[  144.967630]  [<c11814e0>] vfs_write+0x1ad/0x1f9
[  144.967630]  [<c1181fc6>] sys_write+0x5e/0x80
[  144.967630]  [<c100582b>] sysenter_do_call+0x12/0x38
[  146.085749] PM: Adding info for No Bus:vcs4
[  146.085864] PM: Adding info for No Bus:vcsa4
[  146.090924] PM: Adding info for No Bus:vcs9
[  146.091077] PM: Adding info for No Bus:vcsa9
[  146.092977] PM: Adding info for No Bus:vcs3

  reply	other threads:[~2009-06-23 18:42 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-04 21:00 [PATCH] kvm: fix kvm reboot crash when MAXSMP is used Yinghai Lu
2009-06-04 21:01 ` [PATCH] cpumask: alloc blank cpumask left over Yinghai Lu
2009-06-05  4:58   ` Rusty Russell
2009-06-05  5:18     ` Avi Kivity
2009-06-05  5:56     ` Yinghai Lu
2009-06-05 13:41       ` Rusty Russell
2009-06-05 17:34         ` Linus Torvalds
2009-06-05 17:46           ` Yinghai Lu
2009-06-05 17:57           ` Yinghai Lu
2009-06-06 23:40             ` Rusty Russell
2009-06-06 23:43           ` Rusty Russell
2009-06-06  9:22         ` Avi Kivity
2009-06-06  9:36           ` Yinghai Lu
2009-06-06  9:39             ` Avi Kivity
2009-06-06 10:57               ` Yinghai Lu
2009-06-06 21:50                 ` [PATCH 1/6] cpumask: introduce zalloc_cpumask_var Yinghai Lu
2009-06-06 21:51                   ` Subject: [PATCH 2/6] cpumask: alloc zeroed cpumask for static cpumask_var_ts Yinghai Lu
2009-06-06 21:52                   ` [PATCH 3/6] kvm: fix kvm reboot crash when MAXSMP is used Yinghai Lu
2009-06-06 21:53                   ` [PATCH 4/6] x86/cpufreq: use cpumask_copy instead of = Yinghai Lu
2009-06-09  6:57                     ` Rusty Russell
2009-06-09  8:13                       ` Yinghai Lu
2009-06-10  4:20                         ` Rusty Russell
2009-06-10 13:39                           ` Dave Jones
2009-06-10 17:01                             ` Ingo Molnar
2009-06-09 15:46                       ` Linus Torvalds
2009-06-09 16:28                         ` Dave Jones
2009-06-09 16:41                           ` Linus Torvalds
2009-06-10  4:55                             ` Rusty Russell
2009-06-10  6:22                         ` Rusty Russell
2009-06-10 11:10                           ` S06cpuspeed/2637 is trying to acquire lock (&(&dbs_info->work)->work (was: Re: [PATCH 4/6] x86/cpufreq: use cpumask_copy instead of =) Ingo Molnar
2009-06-10 20:58                             ` Dave Jones
2009-06-11 10:52                               ` Ingo Molnar
2009-06-20 12:48                                 ` Ingo Molnar
2009-06-21 19:55                                   ` Thomas Renninger
2009-06-23 18:17                                     ` [PATCH] cpufreq: remove dbs_mutex Ingo Molnar
2009-06-23 18:40                                       ` Ingo Molnar [this message]
2009-06-23 18:51                                         ` Pallipadi, Venkatesh
2009-06-23 19:14                                           ` Ingo Molnar
2009-06-23 19:24                                             ` Pallipadi, Venkatesh
2009-06-23 19:32                                               ` Ingo Molnar
     [not found]                                                 ` <20090623193215.GA31374-X9Un+BFzKDI@public.gmane.org>
2009-06-25 14:01                                                   ` Fix dead lock in cpufreq for CPU hotplug and suspend for 2.6.30.stable Thomas Renninger
2009-06-25 14:01                                                     ` Thomas Renninger
     [not found]                                                     ` <1245938485-12663-1-git-send-email-trenn-l3A5Bk7waGM@public.gmane.org>
2009-06-25 14:06                                                       ` Thomas Renninger
2009-06-25 14:06                                                         ` Thomas Renninger
2009-06-25 14:01                                                 ` [PATCH 1/2] CPUFREQ: Remove unneeded dbs_mutexes from ondemand and conservative governors Thomas Renninger
     [not found]                                                   ` <1245938485-12663-2-git-send-email-trenn-l3A5Bk7waGM@public.gmane.org>
2009-06-25 14:25                                                     ` Mathieu Desnoyers
2009-06-25 14:25                                                       ` Mathieu Desnoyers
2009-06-25 15:03                                                       ` Pallipadi, Venkatesh
2009-06-25 15:03                                                         ` Pallipadi, Venkatesh
2009-06-25 22:17                                                       ` Thomas Renninger
2009-06-25 22:17                                                         ` Thomas Renninger
     [not found]                                                         ` <200906260017.10730.trenn-l3A5Bk7waGM@public.gmane.org>
2009-06-25 22:26                                                           ` Thomas Renninger
2009-06-25 22:26                                                             ` Thomas Renninger
2009-06-30  6:33                                                     ` Pavel Machek
2009-06-30  6:33                                                       ` Pavel Machek
     [not found]                                                       ` <20090630063339.GF1351-+ZI9xUNit7I@public.gmane.org>
2009-07-03 10:10                                                         ` Thomas Renninger
2009-07-03 10:10                                                           ` Thomas Renninger
2009-07-05 19:46                                                           ` Pavel Machek
2009-06-30 22:58                                                     ` [stable] " Greg KH
2009-06-30 22:58                                                       ` Greg KH
     [not found]                                                       ` <20090630225813.GB2634-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2009-06-30 23:14                                                         ` Mathieu Desnoyers
2009-06-30 23:14                                                           ` Mathieu Desnoyers
2009-06-30 23:39                                                           ` Greg KH
2009-06-30 23:39                                                             ` Greg KH
     [not found]                                                             ` <20090630233912.GA3709-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2009-07-01  9:07                                                               ` Thomas Renninger
2009-07-01  9:07                                                                 ` Thomas Renninger
2009-06-25 14:01                                                 ` [PATCH 2/2] remove rwsem lock from CPUFREQ_GOV_STOP call (second call site) Thomas Renninger
2009-06-10 19:42                           ` [PATCH 4/6] x86/cpufreq: use cpumask_copy instead of = Langsdorf, Mark
2009-06-11  2:34                             ` Rusty Russell
2009-09-21 16:44                               ` Langsdorf, Mark
2009-06-06 21:55                   ` [PATCH 5/6] core: use cpumask_copy instead of = for cpus_allowed in fork Yinghai Lu
2009-06-06 21:56                   ` [PATCH 6/6] x86/cpufreq: don't use SPEEDSTEP with MAXSMP Yinghai Lu
2009-06-06 21:56                   ` [PATCH 1/6] cpumask: introduce zalloc_cpumask_var Andrew Morton
2009-06-06 22:07                     ` Yinghai Lu
2009-06-06 21:58                   ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090623184040.GA6908@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=avi@redhat.com \
    --cc=cpufreq@vger.kernel.org \
    --cc=davej@redhat.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.langsdorf@amd.com \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=trenn@suse.de \
    --cc=venkatesh.pallipadi@intel.com \
    --cc=yinghai@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.