On Mon, 2009-08-10 at 08:48 -0700, Thomas Renninger wrote: > 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. Yes. Looking at the code, this deadlock should happen again with the new code. > 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. > I can't think of any recent changes that will help with this particular deadlock. ondemand/conservative does not have this problem as they add their own sysfs directory and not a file under cpufreq directory and they do not go through cpufreq rwsem. The problem with setspeed was, it goes through show/store in cpufreq.c which will try to hold cpufreq rwsem. And userspace stop flow also holds rwsem and cannot remove sysfs setspeed file on STOP path, as there is a blocking read/write of setspeed. IIRC, tests were similar to what you have below. We had two tasks and we typically hit the deadlock, with cpufreq debug messages turned on, in about 30 mins or so. # task 1 while : do cat /sys/devices/system/cpu/cpu0/cpufreq/* done # task 2 while : do echo userspace > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor echo performance > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor done Thanks, Venki > 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
