• Mathieu Desnoyers's avatar
    [CPUFREQ] remove rwsem lock from CPUFREQ_GOV_STOP call · 42a06f21
    Mathieu Desnoyers authored
    * 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: default avatarMathieu 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: Chris Wright <chrisw@sous-sol.org>
    CC: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarDave Jones <davej@redhat.com>
    42a06f21
cpufreq.c 48.6 KB