On Saturday 08 August 2009 15:55:12 Dominik Brodowski wrote:
> Hey,
>
> how does this new patch square with
>
> commit 9e76988e9390a4ff4d171f690586d0c58186b47e
> Author: Venki Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
> Date: Fri Oct 26 10:18:21 2007 -0700
>
> [CPUFREQ] Eliminate cpufreq_userspace scaling_setspeed deadlock
>
Oops, I thought the scaling_setspeed always was in cpufreq.c and
my patch was intended as cleanup. I did not realize that it explicitly
was moved there to workaround a deadlock and my patch is more or
less a revert of that workaround.
On the other hand side I wonder why this should not work (having
show/store_scaling_setspeed in cpufreq_userspace.c) as
ondemand/conservative do the same.
Hmm, but it seems to work with recent changes. I tried to run into
this with two test apps run in parallel (see at the end) and could
not run into a deadlock.
>
> 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.
>
> I.e. -- could the deadlock situation still occur?
It seems it cannot.
If Venki points me to another test app he used when wrote the patch, I
can give it a try.
> Best,
>
> Dominik
>
> PS: what are the merging plans wrt one-governor-per-system? I.e., when
> should I fix cpufrequtils / libcpufreq?
If we go for one-governor-per-system the real cleanup (in kernel) would
be to get rid of .governor(START/STOP) per CPU calls and provide a
machine wide start/stop function in the governor (after long deprecation
time), right?
We would still have to provide the .governor(UPDATE) per cpu
functionality.
I still like to do another test with it if before this gets
merged into a mainline kernel.
The global ondemand/conservative sysfs changes should be fine.
Test apps run in parallel without any deadlock occuring:
cat_cpufreq.sh
==============
#!/bin/bash
for((x=0;x<100000;x++));do
cat /sys/devices/system/cpu/cpu*/cpufreq/*
cat /sys/devices/system/cpu/cpu*/cpufreq/scaling_setspeed
usleep 100
done
governor_change.sh
==================
#!/bin/bash
set -x
for((x=0;x<100000;x++));do
for y in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor;do
echo userspace >$y
done
usleep 100
for y in /sys/devices/system/cpu/cpu*/cpufreq/scaling_governor;do
echo performance >$y
done
usleep 100
done
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
