Stefan Seyfried wrote:
> Hi,
>
> strolling around the source i came around some very nasty constructs in
> cpufreq.cpp that still need to first check if userspace or kernelspace
> governors are used to decide which object methods to invoke. I came up
> with the attached patch (which does not work yet ;-) and would like to
> hear if it could be done the way i tried and if not, why.
>
> I obviously cannot do it (else i would have attached a working patch),
> but i can see that the current implementation is not very nice.
>
>
> ------------------------------------------------------------------------
>
> Index: cpufreq.cpp
> ===================================================================
> RCS file: /cvsroot/powersave/powersave/daemon/cpufreq.cpp,v
> retrieving revision 1.32
> diff -u -p -r1.32 cpufreq.cpp
> --- cpufreq.cpp 26 Jul 2005 13:47:04 -0000 1.32
> +++ cpufreq.cpp 26 Jul 2005 18:46:16 -0000
> @@ -19,9 +19,7 @@ int CPUFreq_Interface::cpus = -1;
> CPUFREQ_MODE CPUFreq_Interface::mode = _DYNAMIC;
> CPUFREQ_CONTROL_MODE CPUFreq_Interface::control = CPUFREQ_USERSPACE;
>
> -map<int, CPUFreq_Kernel> CPUFreq_Interface::cpu_objects_kernel = map<int,
> CPUFreq_Kernel>();
> -map<int, CPUFreq_Userspace> CPUFreq_Interface::cpu_objects_userspace =
> map<int, CPUFreq_Userspace>();
> -
> +map<int, CPUFreq_Interface> CPUFreq_Interface::cpu_objects = map<int,
> CPUFreq_Interface>();
>
If this would be possible I'd have done it.
You cannot use virtual classes in Standard Template Library Containers.
Better ask/discuss before you make such big changes.
I agree that it's not nice.
As long as we stay to two cpufreq classes this shouldn't be a big problem.
A final solution could be to introduce our own list/container.
We could also do it with an array (as before), but this will (same for an own
container - as STL won't do memory management) need fiddling around with
memory allocation/deallocation. This was exactly what I wanted to avoid with
the STL map.
> #ifdef CPUFREQ_MEASURE
> unsigned long CPUFreq_Interface::polling_interval = 333;
> @@ -151,24 +149,18 @@ void CPUFreq_Interface::createCPUFreqObj
> for (x = 0; x < cpus; x++){
> switch(control) {
> case CPUFREQ_USERSPACE:
> - cpu_objects_userspace[x] =
> CPUFreq_Userspace(use_sysfs, x);
> - if
> (cpu_objects_userspace[x].readFrequencies()){
> - cpu_objects_userspace.clear();
> - /* indicates no support ! */
> - cpus = -1;
> - return;
> - }
> + cpu_objects[x] =
> CPUFreq_Userspace(use_sysfs, x);
> break;
> case CPUFREQ_KERNEL:
> - cpu_objects_kernel[x] =
> CPUFreq_Kernel(use_sysfs, x, sampling_rate);
> - if (cpu_objects_kernel[x].readFrequencies()){
> - cpu_objects_kernel.clear();
> - /* indicates no support ! */
> - cpus = -1;
> - return;
> - }
> + cpu_objects[x] = CPUFreq_Kernel(use_sysfs,
> x, sampling_rate);
> break;
> }
> + if (cpu_objects[x].readFrequencies()){
> + cpu_objects.clear();
> + /* indicates no support ! */
> + cpus = -1;
> + return;
> + }
> }
> }
> // SYSFS OR NOT ... ********************************************/
> @@ -198,15 +190,8 @@ CPUFREQ_MODE CPUFreq_Interface::getMode(
> }
>
> int CPUFreq_Interface::isSupported(){
> - int ret = 0;
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - ret = cpu_objects_userspace.empty();
> - break;
> - case CPUFREQ_KERNEL:
> - ret = cpu_objects_kernel.empty();
> - break;
> - }
> + int ret;
> + ret = cpu_objects.empty();
> if (ret)
> /* supported */
> return 0;
> @@ -215,20 +200,10 @@ int CPUFreq_Interface::isSupported(){
> }
>
> void CPUFreq_Interface::adjustSpeeds(){
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - for (userspace_iter cpu_core =
> cpu_objects_userspace.begin();
> - cpu_core != cpu_objects_userspace.end();
> cpu_core++)
> - (*cpu_core).second.adjustSpeed(
> - (int)getCPULoadSMP(consider_nice,
> (*cpu_core).first));
> - break;
> - case CPUFREQ_KERNEL:
> - for (kernel_iter cpu_core = cpu_objects_kernel.begin();
> - cpu_core != cpu_objects_kernel.end(); cpu_core++)
> - (*cpu_core).second.adjustSpeed(
> - (int)getCPULoadSMP(consider_nice,
> (*cpu_core).first));
> - break;
> - }
> + for (cpufreq_iter cpu_core = cpu_objects.begin();
> + cpu_core != cpu_objects.end(); cpu_core++)
> + (*cpu_core).second.adjustSpeed(
> + (int)getCPULoadSMP(consider_nice,
> (*cpu_core).first));
> }
>
> /**
> @@ -238,63 +213,30 @@ void CPUFreq_Interface::adjustSpeeds(){
> */
> int CPUFreq_Interface::setModes(CPUFREQ_MODE mode){
> int ret = 0;
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - for (userspace_iter cpu_core =
> cpu_objects_userspace.begin();
> - cpu_core != cpu_objects_userspace.end();
> cpu_core++)
> - ret |= (*cpu_core).second.setMode(mode);
> - break;
> - case CPUFREQ_KERNEL:
> - for (kernel_iter cpu_core = cpu_objects_kernel.begin();
> - cpu_core != cpu_objects_kernel.end(); cpu_core++)
> - ret |= (*cpu_core).second.setMode(mode);
> - }
> + for (cpufreq_iter cpu_core = cpu_objects.begin();
> + cpu_core != cpu_objects.end(); cpu_core++)
> + ret |= (*cpu_core).second.setMode(mode);
> return ret;
> }
>
> void CPUFreq_Interface::reinitSpeeds(){
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - for (userspace_iter cpu_core =
> cpu_objects_userspace.begin();
> - cpu_core != cpu_objects_userspace.end();
> cpu_core++)
> - (*cpu_core).second.reinitSpeed();
> - break;
> - case CPUFREQ_KERNEL:
> - for (kernel_iter cpu_core = cpu_objects_kernel.begin();
> - cpu_core != cpu_objects_kernel.end(); cpu_core++)
> - (*cpu_core).second.reinitSpeed();
> - break;
> - }
> + for (cpufreq_iter cpu_core = cpu_objects.begin();
> + cpu_core != cpu_objects.end(); cpu_core++)
> + (*cpu_core).second.reinitSpeed();
> }
>
> void CPUFreq_Interface::rereadFrequencies(){
> int ret = 0;
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - for (userspace_iter cpu_core =
> cpu_objects_userspace.begin();
> - cpu_core != cpu_objects_userspace.end();
> cpu_core++)
> - ret |= (*cpu_core).second.readFrequencies();
> - break;
> - case CPUFREQ_KERNEL:
> - for (kernel_iter cpu_core = cpu_objects_kernel.begin();
> - cpu_core != cpu_objects_kernel.end(); cpu_core++)
> - ret |= (*cpu_core).second.readFrequencies();
> - break;
> - }
> + for (cpufreq_iter cpu_core = cpu_objects.begin();
> + cpu_core != cpu_objects.end(); cpu_core++)
> + ret |= (*cpu_core).second.readFrequencies();
> if (!ret)
> adjustSpeeds();
> }
>
> void CPUFreq_Interface::destroy(){
> freeCPULoadSMPCalls();
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - cpu_objects_userspace.clear();
> - break;
> - case CPUFREQ_KERNEL:
> - cpu_objects_kernel.clear();
> - break;
> - }
> + cpu_objects.clear();
> }
>
> int CPUFreq_Interface::setGovernor(const string new_governor){
> @@ -313,18 +255,9 @@ int CPUFreq_Interface::setGovernor(const
>
> int CPUFreq_Interface::setGovernors(const string new_governor){
> int ret = 0;
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - for (userspace_iter cpu_core =
> cpu_objects_userspace.begin();
> - cpu_core != cpu_objects_userspace.end();
> cpu_core++)
> - ret |=
> (*cpu_core).second.setGovernor(new_governor);
> - break;
> - case CPUFREQ_KERNEL:
> - for (kernel_iter cpu_core = cpu_objects_kernel.begin();
> - cpu_core != cpu_objects_kernel.end(); cpu_core++)
> - ret |=
> (*cpu_core).second.setGovernor(new_governor);
> - break;
> - }
> + for (cpufreq_iter cpu_core = cpu_objects.begin();
> + cpu_core != cpu_objects.end(); cpu_core++)
> + ret |= (*cpu_core).second.setGovernor(new_governor);
> /* if (ret != 0) -> something wrong */
> return ret;
> }
> @@ -367,16 +300,7 @@ void CPUFreq_Interface::setConfigs(int m
> cpu_hysteresis = hyster;
>
> consider_nice = consider;
> - switch (CPUFreq_Interface::control){
> - case CPUFREQ_USERSPACE:
> - for (userspace_iter cpu_core =
> cpu_objects_userspace.begin();
> - cpu_core != cpu_objects_userspace.end();
> cpu_core++)
> - (*cpu_core).second.setConfig();
> - break;
> - case CPUFREQ_KERNEL:
> - for (kernel_iter cpu_core = cpu_objects_kernel.begin();
> - cpu_core != cpu_objects_kernel.end(); cpu_core++)
> - (*cpu_core).second.setConfig();
> - break;
> - }
> + for (cpufreq_iter cpu_core = cpu_objects.begin();
> + cpu_core != cpu_objects.end(); cpu_core++)
> + (*cpu_core).second.setConfig();
> }
> Index: cpufreq.h
> ===================================================================
> RCS file: /cvsroot/powersave/powersave/daemon/cpufreq.h,v
> retrieving revision 1.14
> diff -u -p -r1.14 cpufreq.h
> --- cpufreq.h 6 Jul 2005 16:18:15 -0000 1.14
> +++ cpufreq.h 26 Jul 2005 18:46:17 -0000
> @@ -47,11 +47,8 @@ class CPUFreq_Interface{
> * and userspace (there shouldn't be any more subclasses
> * in the future ...
> */
> - static map<int, CPUFreq_Kernel> cpu_objects_kernel;
> - static map<int, CPUFreq_Userspace> cpu_objects_userspace;
> - typedef map<int, CPUFreq_Kernel>::iterator kernel_iter;
> - typedef map<int, CPUFreq_Userspace>::iterator userspace_iter;
> -
> + static map<int, CPUFreq_Interface> cpu_objects;
> + typedef map<int, CPUFreq_Interface>::iterator cpufreq_iter;
>
> /** @brief file where active governor is in */
> string GOVERNOR_FILE;
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> powersave-devel mailing list
> [email protected]
> http://forge.novell.com/mailman/listinfo/powersave-devel
_______________________________________________
powersave-devel mailing list
[email protected]
http://forge.novell.com/mailman/listinfo/powersave-devel
