From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Kernel Testers List <kernel-testers@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Chris Wright <chrisw@sous-sol.org>, Dave Jones <davej@redhat.com>,
Ben Slusky <sluskyb@paranoiacs.org>, Ingo Molnar <mingo@elte.hu>,
Greg KH <greg@kroah.com>,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Subject: [PATCH] cpufreq: remove rwsem lock from CPUFREQ_GOV_STOP call
Date: Sun, 17 May 2009 10:23:52 -0400 [thread overview]
Message-ID: <20090517142352.GA27882@Krystal> (raw)
In-Reply-To: <pQ9WADCjgyE.A.PTG.HA0DKB@chimera>
* Rafael J. Wysocki (rjw@sisk.pl) wrote:
> This message has been generated automatically as a part of a report
> of regressions introduced between 2.6.28 and 2.6.29.
>
> The following bug entry is on the current list of known regressions
> introduced between 2.6.28 and 2.6.29. Please verify if it still should
> be listed and let me know (either way).
>
>
> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=13186
> Subject : cpufreq timer teardown problem
> Submitter : Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Date : 2009-04-23 14:00 (24 days old)
> References : http://marc.info/?l=linux-kernel&m=124049523515036&w=4
> Handled-By : Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Patch : http://patchwork.kernel.org/patch/19754/
> http://patchwork.kernel.org/patch/19753/
The patches linked above depend on the following patch to remove
circular locking dependency :
cpufreq: remove rwsem lock from CPUFREQ_GOV_STOP call
(the following issue was faced when using cancel_delayed_work_sync() in the
timer teardown (which fixes a race).
* KOSAKI Motohiro (kosaki.motohiro@jp.fujitsu.com) wrote:
> Hi
>
> my box output following warnings.
> it seems regression by commit 7ccc7608b836e58fbacf65ee4f8eefa288e86fac.
>
> A: work -> do_dbs_timer() -> cpu_policy_rwsem
> B: store() -> cpu_policy_rwsem -> cpufreq_governor_dbs() -> work
>
>
Hrm, I think it must be due to my attempt to fix the timer teardown race
in ondemand governor mixed with new locking behavior in 2.6.30-rc.
The rwlock seems to be taken around the whole call to
cpufreq_governor_dbs(), when it should be only taken around accesses to
the locked data, and especially *not* around the call to
dbs_timer_exit().
Reverting my fix attempt would put the teardown race back in place
(replacing the cancel_delayed_work_sync by cancel_delayed_work).
Instead, a proper fix would imply modifying this critical section :
cpufreq.c: __cpufreq_remove_dev()
...
if (cpufreq_driver->target)
__cpufreq_governor(data, CPUFREQ_GOV_STOP);
unlock_policy_rwsem_write(cpu);
To make sure the __cpufreq_governor() callback is not called with rwsem
held. This would allow execution of cancel_delayed_work_sync() without
being nested within the rwsem.
Applies on top of the 2.6.30-rc5 tree.
Required to remove circular dep in teardown of both conservative and
ondemande governors so they can use cancel_delayed_work_sync().
CPUFREQ_GOV_STOP does not modify the policy, therefore this locking seemed
unneeded.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
CC: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Greg KH <greg@kroah.com>
CC: Ingo Molnar <mingo@elte.hu>
CC: "Rafael J. Wysocki" <rjw@sisk.pl>
CC: Ben Slusky <sluskyb@paranoiacs.org>
CC: Dave Jones <davej@redhat.com>
CC: Chris Wright <chrisw@sous-sol.org>
CC: Andrew Morton <akpm@linux-foundation.org>
---
drivers/cpufreq/cpufreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6-lttng/drivers/cpufreq/cpufreq.c
===================================================================
--- linux-2.6-lttng.orig/drivers/cpufreq/cpufreq.c 2009-05-10 14:41:53.000000000 -0400
+++ linux-2.6-lttng/drivers/cpufreq/cpufreq.c 2009-05-10 14:42:29.000000000 -0400
@@ -1070,11 +1070,11 @@ static int __cpufreq_remove_dev(struct s
spin_unlock_irqrestore(&cpufreq_driver_lock, flags);
#endif
+ unlock_policy_rwsem_write(cpu);
+
if (cpufreq_driver->target)
__cpufreq_governor(data, CPUFREQ_GOV_STOP);
- unlock_policy_rwsem_write(cpu);
-
kobject_put(&data->kobj);
/* we need to make sure that the underlying kobj is actually
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
next prev parent reply other threads:[~2009-05-17 14:24 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-16 19:58 2.6.30-rc6: Reported regressions 2.6.28 -> 2.6.29 Rafael J. Wysocki
2009-05-16 19:59 ` [Bug #12490] ath5k related kernel panic in 2.6.29-rc1 Rafael J. Wysocki
2009-05-16 20:04 ` [Bug #12499] Problem with using bluetooth adaper connected to usb port Rafael J. Wysocki
2009-05-17 7:58 ` Maciej Rutecki
2009-05-17 10:21 ` Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12861] Xorg fails to start "Failed to allocate space for kernel memory manager" Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12681] s2ram: fails to wake up on Acer Extensa 4220 (SMP disabled) Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12705] X200: Brightness broken since 2.6.29-rc4-58-g4c098bc Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12765] i915 VT switch with AIGLX causes X lock up Rafael J. Wysocki
2009-05-18 7:13 ` Sitsofe Wheeler
2009-05-18 17:10 ` Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12909] boot/kernel init duration regression from 2.6.28 Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12971] "tg3 transmit timed out" when transmitting at high bitrate Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12899] Crash in i915.ko: i915_driver_irq_handler Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12947] r128: system hangs when X is started with DRI enabled Rafael J. Wysocki
2009-05-17 16:08 ` Jos van der Ende
2009-05-16 20:05 ` [Bug #13001] PCI-DMA: Out of IOMMU space Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #13024] nozomi: pppd fails on kernel 2.6.29 Rafael J. Wysocki
2009-05-16 20:05 ` [Bug #12980] lockup in X.org Rafael J. Wysocki
2009-05-17 15:39 ` Marcus Better
2009-05-17 17:27 ` Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13074] gspca_stv06xx doesn't work with Logitech QuickCam Express (046d:0840) Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13025] After upgrading to kernel 2.6.29, pulseaudio stopped with some strange error Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13072] forcedeth seems to switch off eth on shutdown Rafael J. Wysocki
[not found] ` <1242563611.15249.2.camel@laptop.workgroup>
2009-05-17 12:56 ` Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13100] can't anymore even do a s2ram-s2disk-s2ram cycle on acer aspire 5720G Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13144] resume from suspend fails using video card i915 Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13175] sata_nv incompatible with async scsi scan Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13172] Spontaneous reboots since 2.6.29-rc* Rafael J. Wysocki
2009-05-17 7:53 ` Maciej Rutecki
2009-05-17 10:23 ` Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13178] Booting very slow Rafael J. Wysocki
2009-05-18 8:15 ` Martin Knoblauch
2009-05-18 17:13 ` Rafael J. Wysocki
2009-05-19 0:26 ` Kay Sievers
2009-05-19 7:22 ` Martin Knoblauch
2009-05-19 8:58 ` Kay Sievers
2009-05-20 10:14 ` Martin Knoblauch
2009-05-16 20:06 ` [Bug #13186] cpufreq timer teardown problem Rafael J. Wysocki
2009-05-17 14:23 ` Mathieu Desnoyers [this message]
2009-05-17 14:29 ` [PATCH] cpufreq fix timer teardown in conservative governor Mathieu Desnoyers
2009-05-17 14:30 ` [PATCH] cpufreq fix timer teardown in ondemand governor Mathieu Desnoyers
2009-05-16 20:06 ` [Bug #13225] [2.6.29 regression] Software suspend no longer works Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13183] forcedeth: no link during initialization Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13269] WARNING: at kernel/hrtimer.c:625 hres_timers_resume+0x3c/0x48() when resuming Rafael J. Wysocki
2009-05-16 20:06 ` [Bug #13232] ext3/4 with synchronous writes gets wedged by Postfix Rafael J. Wysocki
2009-05-18 13:25 ` Theodore Tso
2009-05-19 17:17 ` David Watson
2009-05-19 17:53 ` Theodore Tso
2009-05-19 18:27 ` John Stoffel
2009-05-19 20:41 ` Theodore Tso
2009-05-20 16:53 ` John Stoffel
2009-05-16 20:06 ` [Bug #13271] ath9k stop working since 2.6.29 Rafael J. Wysocki
2009-05-17 7:56 ` 2.6.30-rc6: Reported regressions 2.6.28 -> 2.6.29 Maciej Rutecki
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=20090517142352.GA27882@Krystal \
--to=mathieu.desnoyers@polymtl.ca \
--cc=akpm@linux-foundation.org \
--cc=chrisw@sous-sol.org \
--cc=davej@redhat.com \
--cc=greg@kroah.com \
--cc=kernel-testers@vger.kernel.org \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rjw@sisk.pl \
--cc=sluskyb@paranoiacs.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;
as well as URLs for NNTP newsgroup(s).