Eliminate cpufreq_userspace scaling_setspeed deadlock

PATCH In review Venki Pallipadi 6 participants 10 messages Oct 26, 2007 – Nov 27, 2007 CPU frequency scaling framework

lore

Share & tools

lore

Venki Pallipadi

author

2007-10-26 17:18 UTC #0 lore raw

PATCH 1/1 Eliminate cpufreq_userspace scaling_setspeed deadlock

Eliminate cpufreq_userspace scaling_setspeed deadlock.

Luming Yu recently uncovered yet another cpufreq related deadlock. One thread that continuously switches the governors and the other thread that repeatedly cats the contents of cpufreq directory causes both these threads to go into a deadlock.

Detailed examination of the deadlock showed the exact flow before the deadlock as:

Thread 1 Thread 2
________ ________
cats files under /sys/devices/.../cpufreq/
Set governor to userspace
Adds a new sysfs entry for
scaling_setspeed
cats files under /sys/devices/.../cpufreq/

Set governor to performance
Holds cpufreq_rw_sem in write
mode
Sends a STOP notify to
userspace governor
cat /sys/devices/.../cpufreq/scaling_setspeed
Gets a handle on the above sysfs entry with sysfs_get_active
Blocks while trying to get cpufreq_rw_sem in read mode
Remove a sysfs entry for
scaling_setspeed
Blocks on sysfs_deactivate
while waiting for earlier
get_active (on other thread)
to drain

At this point both threads go into deadlock and any other thread that tries to do anything with sysfs cpufreq will also block.

There seems to be no easy way to avoid this deadlock as long as cpufreq_userspace adds/removes the sysfs entry under same kobject as cpufreq. Below patch moves scaling_setspeed to cpufreq.c, keeping it always and calling back the governor on read/write. This is the cleanest fix I could think of, even though adding two callbacks in governor structure just for this seems unnecessary.

Note that the change makes scaling_setspeed under /sys/.../cpufreq permanent and returns <unsupported> when governor is not userspace.

Signed-off-by Venkatesh Pallipadi <[email protected]>

drivers/cpufreq/cpufreq.c +27 -0

Index: linux-2.6.24-rc-mm/drivers/cpufreq/cpufreq.c
===================================================================
@@ -601,6 +601,31 @@ static ssize_t show_affected_cpus (struc
 	return i;
 }
 
+static ssize_t store_scaling_setspeed(struct cpufreq_policy * policy,
+		const char *buf, size_t count)
+{
+	unsigned int freq = 0;
+	unsigned int ret;
+
+	if (!policy->governor->store_setspeed)
+		return -EINVAL;
+
+	ret = sscanf(buf, "%u", &freq);
+	if (ret != 1)
+		return -EINVAL;
+
+	policy->governor->store_setspeed(policy, freq);
+
+	return count;
+}
+
+static ssize_t show_scaling_setspeed(struct cpufreq_policy * policy, char *buf)
+{
+	if (!policy->governor->show_setspeed)
+		return sprintf (buf, "<unsupported>\n");
+
+	return policy->governor->show_setspeed(policy, buf);
+}
 
 #define define_one_ro(_name) \
 static struct freq_attr _name = \

@@ -624,6 +649,7 @@ define_one_ro(affected_cpus);
 define_one_rw(scaling_min_freq);
 define_one_rw(scaling_max_freq);
 define_one_rw(scaling_governor);
+define_one_rw(scaling_setspeed);
 
 static struct attribute * default_attrs[] = {
 	&cpuinfo_min_freq.attr,

@@ -634,6 +660,7 @@ static struct attribute * default_attrs[
 	&scaling_governor.attr,
 	&scaling_driver.attr,
 	&scaling_available_governors.attr,
+	&scaling_setspeed.attr,
 	NULL
 };

drivers/cpufreq/cpufreq_userspace.c +7 ~~-33~~

Index: linux-2.6.24-rc-mm/drivers/cpufreq/cpufreq_userspace.c
===================================================================
@@ -65,12 +65,12 @@ static struct notifier_block userspace_c
 
 /**
  * cpufreq_set - set the CPU frequency
+ * @policy: pointer to policy struct where freq is being set
  * @freq: target frequency in kHz
- * @cpu: CPU for which the frequency is to be set
  *
  * Sets the CPU frequency to freq.
  */
-static int cpufreq_set(unsigned int freq, struct cpufreq_policy *policy)
+static int cpufreq_set(struct cpufreq_policy *policy, unsigned int freq)
 {
 	int ret = -EINVAL;

@@ -102,34 +102,11 @@ static int cpufreq_set(unsigned int freq
 }
 
 
-/************************** sysfs interface ************************/
-static ssize_t show_speed (struct cpufreq_policy *policy, char *buf)
+static ssize_t show_speed(struct cpufreq_policy *policy, char *buf)
 {
-	return sprintf (buf, "%u\n", cpu_cur_freq[policy->cpu]);
+	return sprintf(buf, "%u\n", cpu_cur_freq[policy->cpu]);
 }
 
-static ssize_t
-store_speed (struct cpufreq_policy *policy, const char *buf, size_t count)
-{
-	unsigned int freq = 0;
-	unsigned int ret;
-
-	ret = sscanf (buf, "%u", &freq);
-	if (ret != 1)
-		return -EINVAL;
-
-	cpufreq_set(freq, policy);
-
-	return count;
-}
-
-static struct freq_attr freq_attr_scaling_setspeed =
-{
-	.attr = { .name = "scaling_setspeed", .mode = 0644 },
-	.show = show_speed,
-	.store = store_speed,
-};
-
 static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
 				   unsigned int event)
 {

@@ -142,10 +119,6 @@ static int cpufreq_governor_userspace(st
 			return -EINVAL;
 		BUG_ON(!policy->cur);
 		mutex_lock(&userspace_mutex);
-		rc = sysfs_create_file (&policy->kobj,
-					&freq_attr_scaling_setspeed.attr);
-		if (rc)
-			goto start_out;
 
 		if (cpus_using_userspace_governor == 0) {
 			cpufreq_register_notifier(

@@ -160,7 +133,7 @@ static int cpufreq_governor_userspace(st
 		cpu_cur_freq[cpu] = policy->cur;
 		cpu_set_freq[cpu] = policy->cur;
 		dprintk("managing cpu %u started (%u - %u kHz, currently %u kHz)\n", cpu, cpu_min_freq[cpu], cpu_max_freq[cpu], cpu_cur_freq[cpu]);
-start_out:
+
 		mutex_unlock(&userspace_mutex);
 		break;
 	case CPUFREQ_GOV_STOP:

@@ -176,7 +149,6 @@ start_out:
 		cpu_min_freq[cpu] = 0;
 		cpu_max_freq[cpu] = 0;
 		cpu_set_freq[cpu] = 0;
-		sysfs_remove_file (&policy->kobj, &freq_attr_scaling_setspeed.attr);
 		dprintk("managing cpu %u stopped\n", cpu);
 		mutex_unlock(&userspace_mutex);
 		break;

@@ -211,6 +183,8 @@ start_out:
 struct cpufreq_governor cpufreq_gov_userspace = {
 	.name		= "userspace",
 	.governor	= cpufreq_governor_userspace,
+	.store_setspeed	= cpufreq_set,
+	.show_setspeed	= show_speed,
 	.owner		= THIS_MODULE,
 };
 EXPORT_SYMBOL(cpufreq_gov_userspace);

include/linux/cpufreq.h +4 -0

Index: linux-2.6.24-rc-mm/include/linux/cpufreq.h
===================================================================
@@ -167,6 +167,10 @@ struct cpufreq_governor {
 	char	name[CPUFREQ_NAME_LEN];
 	int 	(*governor)	(struct cpufreq_policy *policy,
 				 unsigned int event);
+	ssize_t	(*show_setspeed)	(struct cpufreq_policy *policy,
+					 char *buf);
+	int 	(*store_setspeed)	(struct cpufreq_policy *policy,
+					 unsigned int freq);
 	unsigned int max_transition_latency; /* HW must be able to switch to
 			next freq faster than this value in nano secs or we
 			will fallback to performance governor */

Yu, Luming

2007-11-05 08:58 UTC #1 lore raw

Based on my current testing results, this patch fixes my problem. Please help push it upstream.

Thanks,
Luming

Quoted 240 lines

-----Original Message-----
From: Pallipadi, Venkatesh
Sent: 2007Äê10ÔÂ27ÈÕ 1:18
To: Dave Jones
Cc: cpufreq; Yu, Luming; Yu, Fenghua
Subject: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock
Eliminate cpufreq_userspace scaling_setspeed deadlock.
Luming Yu recently uncovered yet another cpufreq related deadlock. One thread that continuously switches the governors and the other thread that
repeatedly cats the contents of cpufreq directory causes both these threads to
go into a deadlock.
Detailed examination of the deadlock showed the exact flow before the deadlock
as:
Thread 1 Thread 2
________ ________
cats files under
/sys/devices/.../cpufreq/
Set governor to userspace
Adds a new sysfs entry for
scaling_setspeed
cats files under
/sys/devices/.../cpufreq/
Set governor to performance
Holds cpufreq_rw_sem in write
mode
Sends a STOP notify to
userspace governor
cat
/sys/devices/.../cpufreq/scaling_setspeed
Gets a handle on the above
sysfs entry with
sysfs_get_active Blocks while trying to get
cpufreq_rw_sem
in read mode
Remove a sysfs entry for
scaling_setspeed
Blocks on sysfs_deactivate
while waiting for earlier
get_active (on other thread)
to drain
At this point both threads go into deadlock and any other thread that tries to
do anything with sysfs cpufreq will also block.
There seems to be no easy way to avoid this deadlock as long as cpufreq_userspace adds/removes the sysfs entry under same kobject as cpufreq.
Below patch moves scaling_setspeed to cpufreq.c, keeping it always and calling
back the governor on read/write. This is the cleanest fix I could think of,
even though adding two callbacks in governor structure just for this seems
unnecessary.
Note that the change makes scaling_setspeed under /sys/.../cpufreq permanent
and returns <unsupported> when governor is not userspace.
Signed-off-by Venkatesh Pallipadi <[email protected]>
Index: linux-2.6.24-rc-mm/drivers/cpufreq/cpufreq.c
===================================================================
drivers/cpufreq/cpufreq.c +25 -0
@@ -601,6 +601,31 @@ static ssize_t show_affected_cpus (struc
 	return i;
 }
 
+static ssize_t store_scaling_setspeed(struct cpufreq_policy * policy,
+		const char *buf, size_t count)
+{
+	unsigned int freq = 0;
+	unsigned int ret;
+
+	if (!policy->governor->store_setspeed)
+		return -EINVAL;
+
+	ret = sscanf(buf, "%u", &freq);
+	if (ret != 1)
+		return -EINVAL;
+
+	policy->governor->store_setspeed(policy, freq);
+
+	return count;
+}
+
+static ssize_t show_scaling_setspeed(struct cpufreq_policy * 
 policy, char *buf)
+{
+	if (!policy->governor->show_setspeed)
+		return sprintf (buf, "<unsupported>\n");
+
+	return policy->governor->show_setspeed(policy, buf);
+}
 
 #define define_one_ro(_name) \
static struct freq_attr _name = \
drivers/cpufreq/cpufreq.c +2 -0
@@ -624,6 +649,7 @@ define_one_ro(affected_cpus);
 define_one_rw(scaling_min_freq);
 define_one_rw(scaling_max_freq);
 define_one_rw(scaling_governor);
+define_one_rw(scaling_setspeed);
 
 static struct attribute * default_attrs[] = {
 	&cpuinfo_min_freq.attr,
@@ -634,6 +660,7 @@ static struct attribute * default_attrs[
 	&scaling_governor.attr,
 	&scaling_driver.attr,
 	&scaling_available_governors.attr,
+	&scaling_setspeed.attr,
 	NULL
 };
 
Index: linux-2.6.24-rc-mm/drivers/cpufreq/cpufreq_userspace.c
===================================================================
drivers/cpufreq/cpufreq_userspace.c +2 -2
@@ -65,12 +65,12 @@ static struct notifier_block userspace_c
 
 /**
 * cpufreq_set - set the CPU frequency
+ * @policy: pointer to policy struct where freq is being set
 * @freq: target frequency in kHz
- * @cpu: CPU for which the frequency is to be set
 *
 * Sets the CPU frequency to freq.
 */
-static int cpufreq_set(unsigned int freq, struct 
 cpufreq_policy *policy)
+static int cpufreq_set(struct cpufreq_policy *policy, 
 unsigned int freq)
 {
	int ret = -EINVAL;
drivers/cpufreq/cpufreq_userspace.c +2 ~~-25~~
@@ -102,34 +102,11 @@ static int cpufreq_set(unsigned int freq
 }
 
 
-/************************** sysfs interface ************************/
-static ssize_t show_speed (struct cpufreq_policy *policy, char *buf)
+static ssize_t show_speed(struct cpufreq_policy *policy, char *buf)
 {
-	return sprintf (buf, "%u\n", cpu_cur_freq[policy->cpu]);
+	return sprintf(buf, "%u\n", cpu_cur_freq[policy->cpu]);
 }
 
-static ssize_t
-store_speed (struct cpufreq_policy *policy, const char *buf, 
 size_t count)
-{
-	unsigned int freq = 0;
-	unsigned int ret;
-
-	ret = sscanf (buf, "%u", &freq);
-	if (ret != 1)
-		return -EINVAL;
-
-	cpufreq_set(freq, policy);
-
-	return count;
-}
-
-static struct freq_attr freq_attr_scaling_setspeed =
-{
-	.attr = { .name = "scaling_setspeed", .mode = 0644 },
-	.show = show_speed,
-	.store = store_speed,
-};
-
 static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
 				   unsigned int event)
{
drivers/cpufreq/cpufreq_userspace.c +0 -4
@@ -142,10 +119,6 @@ static int cpufreq_governor_userspace(st
 			return -EINVAL;
 		BUG_ON(!policy->cur);
 		mutex_lock(&userspace_mutex);
-		rc = sysfs_create_file (&policy->kobj,
-					
 &freq_attr_scaling_setspeed.attr);
-		if (rc)
-			goto start_out;
 
 		if (cpus_using_userspace_governor == 0) {
			cpufreq_register_notifier(
drivers/cpufreq/cpufreq_userspace.c +1 -1
@@ -160,7 +133,7 @@ static int cpufreq_governor_userspace(st
 		cpu_cur_freq[cpu] = policy->cur;
 		cpu_set_freq[cpu] = policy->cur;
 		dprintk("managing cpu %u started (%u - %u kHz, 
 currently %u kHz)\n", cpu, cpu_min_freq[cpu], 
 cpu_max_freq[cpu], cpu_cur_freq[cpu]);
-start_out:
+
 		mutex_unlock(&userspace_mutex);
		break;
	case CPUFREQ_GOV_STOP:
drivers/cpufreq/cpufreq_userspace.c +0 -1
@@ -176,7 +149,6 @@ start_out:
 		cpu_min_freq[cpu] = 0;
 		cpu_max_freq[cpu] = 0;
 		cpu_set_freq[cpu] = 0;
-		sysfs_remove_file (&policy->kobj, 
 &freq_attr_scaling_setspeed.attr);
 		dprintk("managing cpu %u stopped\n", cpu);
 		mutex_unlock(&userspace_mutex);
		break;
drivers/cpufreq/cpufreq_userspace.c +2 -0
@@ -211,6 +183,8 @@ start_out:
 struct cpufreq_governor cpufreq_gov_userspace = {
 	.name		= "userspace",
 	.governor	= cpufreq_governor_userspace,
+	.store_setspeed	= cpufreq_set,
+	.show_setspeed	= show_speed,
 	.owner		= THIS_MODULE,
 };
 EXPORT_SYMBOL(cpufreq_gov_userspace);
Index: linux-2.6.24-rc-mm/include/linux/cpufreq.h
===================================================================
include/linux/cpufreq.h +4 -0
@@ -167,6 +167,10 @@ struct cpufreq_governor {
 	char	name[CPUFREQ_NAME_LEN];
 	int 	(*governor)	(struct cpufreq_policy *policy,
 				 unsigned int event);
+	ssize_t	(*show_setspeed)	(struct cpufreq_policy *policy,
+					 char *buf);
+	int 	(*store_setspeed)	(struct cpufreq_policy *policy,
+					 unsigned int freq);
 	unsigned int max_transition_latency; /* HW must be able 

Quoted 5 lines

to switch to
next freq faster than this value in
nano secs or we
will fallback to performance governor */

_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Mattia Dongili

2007-11-07 12:56 UTC #2 lore raw

Quoted 2 lines · Luming

Based on my current testing results, this patch fixes my problem. Please help push it upstream.

BTW, this will break some userspace apps. I can think of cpufrequtils and some deamons that use the userspace governor to set frequencies.

Signature · 2 lines

mattia
:wq!

Pallipadi, Venkatesh

author

2007-11-08 06:45 UTC #3 lore raw

Quoted 10 lines

-----Original Message-----
From: Mattia Dongili [mailto:[email protected]]
Sent: Wednesday, November 07, 2007 4:57 AM
To: Yu, Luming
Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua; cpufreq
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock

Quoted 7 lines

Quoted 2 lines · Luming
Based on my current testing results, this patch fixes my problem. Please help push it upstream.
BTW, this will break some userspace apps. I can think of cpufrequtils and some deamons that use the userspace governor to set frequencies.

Yes. But, this is the only solution we have for the deadlock right now and I think it should goto upstream as soon as possible, unless we have some better alternative. Userspace breakage is better bargain than a kernel deadlock. Isn't it :-).

Thanks,
Venki

Dave Jones

2007-11-20 23:41 UTC #4 lore raw

Quoted 14 lines · Venki Pallipadi

Quoted 14 lines
-----Original Message-----
From: Mattia Dongili [mailto:[email protected]]
Sent: Wednesday, November 07, 2007 4:57 AM
To: Yu, Luming
Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua; cpufreq
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock
Quoted 2 lines · Luming
Based on my current testing results, this patch fixes my problem. Please help push it upstream.

Quoted 8 lines

Quoted 3 lines
BTW, this will break some userspace apps. I can think of cpufrequtils and some deamons that use the userspace governor to set frequencies.
Yes. But, this is the only solution we have for the deadlock right now and I think it should goto upstream as soon as possible, unless we have some better alternative. Userspace breakage is better bargain than a kernel deadlock. Isn't it :-).

I'm torn over this patch. I don't see a better way to fix this either, but the potential for breaking userspace is there. I think I'm going to merge this, and see how things go in -mm for a while.

Dave

Signature · 1 line

http://www.codemonkey.org.uk

claudio

2007-11-21 03:04 UTC #5 lore raw

Is it what I'm reading: userspacer will be over?
Sorry I haven't been aware of "how the problem happens", so how has "userspace" deadlocked the kernel?I'd like to understand (any link to the problem?) it.
[]s

Quoted 38 lines · Dave Jones

Quoted 14 lines · Venki Pallipadi
Quoted 14 lines
-----Original Message-----
From: Mattia Dongili [mailto:[email protected]]
Sent: Wednesday, November 07, 2007 4:57 AM
To: Yu, Luming
Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua; cpufreq
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock
Quoted 2 lines · Luming
Based on my current testing results, this patch fixes my problem. Please help push it upstream.
Quoted 8 lines
Quoted 3 lines
BTW, this will break some userspace apps. I can think of cpufrequtils and some deamons that use the userspace governor to set frequencies.
Yes. But, this is the only solution we have for the deadlock right now and I think it should goto upstream as soon as possible, unless we have some better alternative. Userspace breakage is better bargain than a kernel deadlock. Isn't it :-).
I'm torn over this patch. I don't see a better way to fix this either, but the potential for breaking userspace is there. I think I'm going to merge this, and see how things go in -mm for a while.
Dave
--
http://www.codemonkey.org.uk
_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Signature · 7 lines

Claudio Eduardo Marques Gomes
Estudante de Engenharia da Computação
Diretor de Tecnologia do CAECOM
Projeto DCC-INDT em Baixo Consumo de Energia em Sistemas Embarcados
Universidade Federal do Amazonas (UFAM)
Tell: 92 3088-3845
Cel:  92 9183-8100

Pallipadi, Venkatesh

author

2007-11-26 17:40 UTC #6 lore raw

Userspace will still be there. Just the semantics of scaling_setspeed will change slightly.

The problem below, userspace did not deadlock the kernel. Userspace deadlocks the cpufreq sub system.
The two threads that can reproduce the problem is: One thread that changes the governors across say performance, userspace and ondemand on all CPUs continuously and other thread just keeps on doing 'cat /sys/devices/system/cpu/cpu*/cpufreq/*' in an infinite loop.

Thanks,
Venki

________________________________

From: claudio [mailto:[email protected]]
Sent: Tuesday, November 20, 2007 7:05 PM
To: Dave Jones; [email protected]; Yu, Luming; Yu, Fenghua; Pallipadi, Venkatesh
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock

On Nov 20, 2007 7:41 PM, Dave Jones <[email protected]> wrote:

On Wed, Nov 07, 2007 at 10:45:35PM -0800, Venki Pallipadi wrote:
>
>
> >-----Original Message-----
> >From: Mattia Dongili [mailto:[email protected] ]
> >Sent: Wednesday, November 07, 2007 4:57 AM
> >To: Yu, Luming
> >Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua; cpufreq > >Subject: Re: [PATCH] Eliminate cpufreq_userspace
> >scaling_setspeed deadlock
> >
> >On Mon, Nov 05, 2007 at 04:58:18PM +0800, Yu, Luming wrote:
> >> Based on my current testing results, this patch fixes my problem. > >> Please help push it upstream.
> >
> >BTW, this will break some userspace apps. I can think of cpufrequtils > >and some deamons that use the userspace governor to set frequencies. > >
>
> Yes. But, this is the only solution we have for the deadlock right now > and I think it should goto upstream as soon as possible, unless we have > some better alternative. Userspace breakage is better bargain than a > kernel deadlock. Isn't it :-).

I'm torn over this patch. I don't see a better way to fix this either, but the potential for breaking userspace is there. I think I'm going to merge this, and see how things go in -mm for a while.

Dave

--
http://www.codemonkey.org.uk

_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

--
Claudio Eduardo Marques Gomes
Estudante de Engenharia da Computação
Diretor de Tecnologia do CAECOM
Projeto DCC-INDT em Baixo Consumo de Energia em Sistemas Embarcados Universidade Federal do Amazonas (UFAM)
Tell: 92 3088-3845
Cel: 92 9183-8100

Mauro 'Kenny' Freitas

2007-11-27 21:11 UTC #7 lore raw

Im trying to understand the same thing as Claudio. I understood how the deadlock happens but Id like to know what piece of cpufreq code is in charge of creating those threads...any help?

Quoted 103 lines · Venkatesh

Userspace will still be there. Just the semantics of scaling_setspeed will change slightly.
The problem below, userspace did not deadlock the kernel. Userspace deadlocks the cpufreq sub system.
The two threads that can reproduce the problem is: One thread that changes the governors across say performance, userspace and ondemand on all CPUs continuously and other thread just keeps on doing 'cat /sys/devices/system/cpu/cpu*/cpufreq/*' in an infinite loop.
Thanks,
Venki
________________________________
       From: claudio [mailto:[email protected]]
       Sent: Tuesday, November 20, 2007 7:05 PM
       To: Dave Jones; [email protected]; Yu, Luming; Yu, Fenghua;
Pallipadi, Venkatesh
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed
deadlock
Is it what I'm reading: userspacer will be over?
Sorry I haven't been aware of "how the problem happens", so how has
"userspace" deadlocked the kernel?I'd like to understand (any link to the problem?) it.
       []s


       On Nov 20, 2007 7:41 PM, Dave Jones <[email protected]> wrote:


               On Wed, Nov 07, 2007 at 10:45:35PM -0800, Venki Pallipadi
wrote:
                >
                >
                > >-----Original Message-----
                > >From: Mattia Dongili [mailto:[email protected] ]
                > >Sent: Wednesday, November 07, 2007 4:57 AM
                > >To: Yu, Luming
                > >Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua;
cpufreq
> >Subject: Re: [PATCH] Eliminate cpufreq_userspace > >scaling_setspeed deadlock
> >
> >On Mon, Nov 05, 2007 at 04:58:18PM +0800, Yu, Luming
wrote:
> >> Based on my current testing results, this patch fixes
my problem.
> >> Please help push it upstream.
> >
> >BTW, this will break some userspace apps. I can think
of cpufrequtils
> >and some deamons that use the userspace governor to set
frequencies.
                > >
                >
                > Yes. But, this is the only solution we have for the
deadlock right now
> and I think it should goto upstream as soon as possible,
unless we have
> some better alternative. Userspace breakage is better
bargain than a
> kernel deadlock. Isn't it :-).
I'm torn over this patch. I don't see a better way to fix
this either,
but the potential for breaking userspace is there. I think
I'm going to
merge this, and see how things go in -mm for a while.
Dave
--
http://www.codemonkey.org.uk
_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq
--
Claudio Eduardo Marques Gomes
Estudante de Engenharia da Computação
Diretor de Tecnologia do CAECOM
Projeto DCC-INDT em Baixo Consumo de Energia em Sistemas Embarcados Universidade Federal do Amazonas (UFAM)
Tell: 92 3088-3845
Cel: 92 9183-8100
_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Mattia Dongili

2007-11-21 12:02 UTC #8 lore raw

In reply to Dave Jones

Quoted 19 lines · Dave Jones

Quoted 18 lines · Venki Pallipadi
Quoted 17 lines
-----Original Message-----
From: Mattia Dongili [mailto:[email protected]]
Sent: Wednesday, November 07, 2007 4:57 AM
To: Yu, Luming
Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua; cpufreq
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock
Quoted 2 lines · Luming
Based on my current testing results, this patch fixes my problem. Please help push it upstream.
BTW, this will break some userspace apps. I can think of cpufrequtils and some deamons that use the userspace governor to set frequencies.

Quoted 8 lines

Quoted 4 lines
Yes. But, this is the only solution we have for the deadlock right now and I think it should goto upstream as soon as possible, unless we have some better alternative. Userspace breakage is better bargain than a kernel deadlock. Isn't it :-).
I'm torn over this patch. I don't see a better way to fix this either, but the potential for breaking userspace is there. I think I'm going to merge this, and see how things go in -mm for a while.

I have to reconsider my words actually.
It's probably not breaking anything (AFAICT) as the scaling_setspeed file will just sit there forever. In the beginning I thought the file would move somewhere else but that is not the case.

No userspace chould rely on the presence of the file as an indicator of which governor is being used so hopefully the patch will do much more good than bad ;)

cheers

Signature · 2 lines

mattia
:wq!

Claudio Eduardo

2007-11-26 13:05 UTC #9 lore raw

I'm trying to understand cpufreq more deeply , and then I'd like to know what sysfs_get_active and sysfs_deactivate does and what "processs" would "trigger" the second thread which cats /sys/devices/.../cpufreq related above. Could someone help me =D ?

Quoted 51 lines · Mattia Dongili

Quoted 11 lines · Dave Jones
Quoted 10 lines · Venki Pallipadi
Quoted 10 lines
-----Original Message-----
From: Mattia Dongili [mailto:[email protected]]
Sent: Wednesday, November 07, 2007 4:57 AM
To: Yu, Luming
Cc: Pallipadi, Venkatesh; Dave Jones; Yu, Fenghua; cpufreq
Subject: Re: [PATCH] Eliminate cpufreq_userspace scaling_setspeed deadlock
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines · Luming
Quoted 2 lines
Based on my current testing results, this patch fixes my problem. Please help push it upstream.
BTW, this will break some userspace apps. I can think of
cpufrequtils
Quoted 4 lines
Quoted 4 lines
Quoted 2 lines
and some deamons that use the userspace governor to set frequencies.
Yes. But, this is the only solution we have for the deadlock right
now
Quoted 1 line
Quoted 1 line
and I think it should goto upstream as soon as possible, unless we
have
Quoted 6 lines
Quoted 2 lines
some better alternative. Userspace breakage is better bargain than a kernel deadlock. Isn't it :-).
I'm torn over this patch. I don't see a better way to fix this either, but the potential for breaking userspace is there. I think I'm going to merge this, and see how things go in -mm for a while.
I have to reconsider my words actually.
It's probably not breaking anything (AFAICT) as the scaling_setspeed file will just sit there forever. In the beginning I thought the file would move somewhere else but that is not the case.
No userspace chould rely on the presence of the file as an indicator of which governor is being used so hopefully the patch will do much more good than bad ;)
cheers
--
mattia
:wq!
_______________________________________________
Cpufreq mailing list
[email protected]
http://lists.linux.org.uk/mailman/listinfo/cpufreq

Signature · 7 lines

Claudio Eduardo Marques Gomes
Estudante de Engenharia da Computação
Diretor de Tecnologia do CAECOM
Projeto DCC-INDT em Baixo Consumo de Energia em Sistemas Embarcados
Universidade Federal do Amazonas (UFAM)
Tell: 92 3088-3845
Cel:  92 9183-8100