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
next prev parent reply other threads:[~2009-06-23 18:42 UTC|newest]
Thread overview: 63+ 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
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:06 ` Thomas Renninger
2009-06-25 14:01 ` [PATCH 1/2] CPUFREQ: Remove unneeded dbs_mutexes from ondemand and conservative governors Thomas Renninger
2009-06-25 14:25 ` Mathieu Desnoyers
2009-06-25 15:03 ` Pallipadi, Venkatesh
2009-06-25 22:17 ` Thomas Renninger
2009-06-25 22:26 ` Thomas Renninger
2009-06-30 6:33 ` Pavel Machek
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 23:14 ` Mathieu Desnoyers
2009-06-30 23:39 ` Greg KH
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox