All of lore.kernel.org
 help / color / mirror / Atom feed
* + cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch added to -mm tree
@ 2010-05-14 22:15 akpm
  2010-05-14 22:42 ` + cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-c all-site.patch " Mathieu Desnoyers
  0 siblings, 1 reply; 2+ messages in thread
From: akpm @ 2010-05-14 22:15 UTC (permalink / raw)
  To: mm-commits
  Cc: andrej.gelenberg, davej, mathieu.desnoyers, venkatesh.pallipadi


The patch titled
     cpufreq: revert "[CPUFREQ] remove rwsem lock from CPUFREQ_GOV_STOP call (second call site)"
has been added to the -mm tree.  Its filename is
     cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/SubmitChecklist when testing your code ***

See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
out what to do about this

The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/

------------------------------------------------------
Subject: cpufreq: revert "[CPUFREQ] remove rwsem lock from CPUFREQ_GOV_STOP call (second call site)"
From: Andrej Gelenberg <andrej.gelenberg@udo.edu>

395913d0b1db37092ea3d9d69b832183b1dd84c5 ("[CPUFREQ] remove rwsem lock
from CPUFREQ_GOV_STOP call (second call site)") is not needed, because
there is no rwsem lock in cpufreq_ondemand and cpufreq_conservativ
anymore.  Lock should not be released until the work done.

Addresses https://bugzilla.kernel.org/show_bug.cgi?id=1594

Signed-off-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Cc: Dave Jones <davej@codemonkey.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/cpufreq/cpufreq.c |   11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff -puN drivers/cpufreq/cpufreq.c~cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site drivers/cpufreq/cpufreq.c
--- a/drivers/cpufreq/cpufreq.c~cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site
+++ a/drivers/cpufreq/cpufreq.c
@@ -1758,17 +1758,8 @@ static int __cpufreq_set_policy(struct c
 			dprintk("governor switch\n");
 
 			/* end old governor */
-			if (data->governor) {
-				/*
-				 * Need to release the rwsem around governor
-				 * stop due to lock dependency between
-				 * cancel_delayed_work_sync and the read lock
-				 * taken in the delayed work handler.
-				 */
-				unlock_policy_rwsem_write(data->cpu);
+			if (data->governor)
 				__cpufreq_governor(data, CPUFREQ_GOV_STOP);
-				lock_policy_rwsem_write(data->cpu);
-			}
 
 			/* start new governor */
 			data->governor = policy->governor;
_

Patches currently in -mm which might be from andrej.gelenberg@udo.edu are

origin.patch
cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: + cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-c all-site.patch added to -mm tree
  2010-05-14 22:15 + cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch added to -mm tree akpm
@ 2010-05-14 22:42 ` Mathieu Desnoyers
  0 siblings, 0 replies; 2+ messages in thread
From: Mathieu Desnoyers @ 2010-05-14 22:42 UTC (permalink / raw)
  To: akpm; +Cc: mm-commits, andrej.gelenberg, davej, venkatesh.pallipadi,
	linux-kernel

* akpm@linux-foundation.org (akpm@linux-foundation.org) wrote:
> 
> The patch titled
>      cpufreq: revert "[CPUFREQ] remove rwsem lock from CPUFREQ_GOV_STOP call (second call site)"
> has been added to the -mm tree.  Its filename is
>      cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch
> 
> Before you just go and hit "reply", please:
>    a) Consider who else should be cc'ed
>    b) Prefer to cc a suitable mailing list as well
>    c) Ideally: find the original patch on the mailing list and do a
>       reply-to-all to that, adding suitable additional cc's
> 
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> 
> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> out what to do about this
> 
> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
> 
> ------------------------------------------------------
> Subject: cpufreq: revert "[CPUFREQ] remove rwsem lock from CPUFREQ_GOV_STOP call (second call site)"
> From: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> 
> 395913d0b1db37092ea3d9d69b832183b1dd84c5 ("[CPUFREQ] remove rwsem lock
> from CPUFREQ_GOV_STOP call (second call site)") is not needed, because
> there is no rwsem lock in cpufreq_ondemand and cpufreq_conservativ

nit: missing "e"

> anymore.  Lock should not be released until the work done.

Yep, locking has been restructured in the workqueue handler so the
deadlock scenario cannot occur anymore, so this fix seems appropriate.
It is needed to synchronize the governor start/stop. Without this fix, a
race between stop/start could corrupt the ondemand/conservative timer.

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

> 
> Addresses https://bugzilla.kernel.org/show_bug.cgi?id=1594
> 
> Signed-off-by: Andrej Gelenberg <andrej.gelenberg@udo.edu>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Dave Jones <davej@codemonkey.org.uk>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/cpufreq/cpufreq.c |   11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff -puN drivers/cpufreq/cpufreq.c~cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site drivers/cpufreq/cpufreq.c
> --- a/drivers/cpufreq/cpufreq.c~cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site
> +++ a/drivers/cpufreq/cpufreq.c
> @@ -1758,17 +1758,8 @@ static int __cpufreq_set_policy(struct c
>  			dprintk("governor switch\n");
>  
>  			/* end old governor */
> -			if (data->governor) {
> -				/*
> -				 * Need to release the rwsem around governor
> -				 * stop due to lock dependency between
> -				 * cancel_delayed_work_sync and the read lock
> -				 * taken in the delayed work handler.
> -				 */
> -				unlock_policy_rwsem_write(data->cpu);
> +			if (data->governor)
>  				__cpufreq_governor(data, CPUFREQ_GOV_STOP);
> -				lock_policy_rwsem_write(data->cpu);
> -			}
>  
>  			/* start new governor */
>  			data->governor = policy->governor;
> _
> 
> Patches currently in -mm which might be from andrej.gelenberg@udo.edu are
> 
> origin.patch
> cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-05-14 22:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-14 22:15 + cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-call-site.patch added to -mm tree akpm
2010-05-14 22:42 ` + cpufreq-revert-remove-rwsem-lock-from-cpufreq_gov_stop-call-second-c all-site.patch " Mathieu Desnoyers

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.