Don't take semaphore in cpufreq_quick_get()

I don't see any reason to take an expensive lock in cpufreq_quick_get() Reading policy->cur is a single atomic operation and after the lock is dropped again the state could change any time anyways.

So don't take the lock in the first place.

This also makes this function interrupt safe which is useful for some code of mine.

Signed-off-by Andi Kleen <[email protected]>

Index: linux/drivers/cpufreq/cpufreq.c
===================================================================

drivers/cpufreq/cpufreq.c +0 -0

@@ -1109,12 +1109,7 @@ unsigned int cpufreq_quick_get(unsigned 
 	unsigned int ret_freq = 0;

-----Original Message-----
From: Dave Jones [mailto:[email protected]]
Sent: Tuesday, October 09, 2007 5:43 PM
To: Andi Kleen
Cc: [email protected]; Pallipadi, Venkatesh
Subject: Re: [PATCH] Don't take semaphore in cpufreq_quick_get()

Quoted 1 line · Andi Kleen

I don't see any reason to take an expensive lock in

cpufreq_quick_get()

Quoted 16 lines

Reading policy->cur is a single atomic operation and after the lock is dropped again the state could change any time anyways.

So don't take the lock in the first place.

This also makes this function interrupt safe which is useful for some code of mine.

Signed-off-by Andi Kleen <[email protected]>

Index: linux/drivers/cpufreq/cpufreq.c
===================================================================

drivers/cpufreq/cpufreq.c +0 -0

@@ -1109,12 +1109,7 @@ unsigned int cpufreq_quick_get(unsigned 
 	unsigned int ret_freq = 0;

drivers/cpufreq/cpufreq.c unsigned int cpufreq_quick_get(unsigned · line 1114

Quoted 10 lines

drivers/cpufreq/cpufreq.c +0 -5

 
 	if (policy) {
-		if (unlikely(lock_policy_rwsem_read(cpu)))
-			return ret_freq;
-
 		ret_freq = policy->cur;
-
-		unlock_policy_rwsem_read(cpu);
 		cpufreq_cpu_put(policy);
 	}

I can't dispute this. It does look strange.
Even the initial implementation before the cpufreq-vs-hotplug-cpu refactoring happened looked as odd for the same reasons you outline.

      if (policy) {
              down(&policy->lock);
              ret = policy->cur;
              up(&policy->lock);
              cpufreq_cpu_put(policy);
      }

This isn't really protecting anything, and we grab policy->cur elsewhere without locking, so I'm inclined to merge this patch.