Subject: Re: Status report: sysmon_cpufreq(9) + powerctl(8)
To: Iain Hibbert <email@example.com>
From: Juan RP <firstname.lastname@example.org>
Date: 09/28/2006 23:02:58
On Thu, 28 Sep 2006 20:12:26 +0100 (BST)
Iain Hibbert <email@example.com> wrote:
> On Thu, 28 Sep 2006, Juan RP wrote:
> > Please see the following URL for updated code:
> > http://www.xtrarom.org/~juan/sysmon_cpufreq/
> > Do you have any idea about the problem I mentioned above?
> "obj = prop_dictionary_get(..)" does not retain the object so you do
> not need to release it. I think many of your RELEASE_OBJECT()s are
> extra and it probably crashes at the end when releasing objects that
> have been deallocated.
> > Any more suggestions about the code?
> I also dont like this RELEASE_OBJECT macro - I think its probably
> better to split the copyin and the actual operation of the function
> as I did in bthub.c, which cuts down on the cruft.
I think I'll replace it very soon.
> prop_object_type(NULL) will return PROP_TYPE_UNKNOWN so you dont
> usually need to test explicitly for NULL when you do a
> the DEV_PROP_D macro contains the variable name hardcoded but its
> used in different functions. I think that is a bad recipe.
I changed it to DEV_PROP_D(x) device_properties(x)
> in sysmon_cpufreq_recv_data() you test for NULL after creating
> objects, that is not necessary (nor checking for type) since it uses
> WAIT_OK and AFAIK cannot fail. Jason?
> also, in sysmon_cpufreq_return_smcfdict() I dont think you need to
> test the type of prop_object_iterator_next(iter) as it will always be
> a keysym (it says so in the dictionary manpage. Jason?)
> Also, in that function you did the iteration wrong - I think you need
> iter = prop_dictionary_iterator(dict);
> for (j = 0 ; (key = prop_object_iterator_next(iter)) !=
> NULL ; j++)
Thanks for all these comments, I've updated the code both kernel and
Now the dictionary is added into device_t properties dictionary
when sysmon_cpufreq_register is called rather than calling it for each
driver in sysmonioctl_cpufreq.
Anyway there's something strange:
[juan@nocturno][~]> sudo modload cpufreq.o && sudo modload cpufreq2.o
Module loaded as ID 0
Module loaded as ID 1
cpufreq-lkm registered with sysmon_cpufreq.
cpufreq2-lkm registered with sysmon_cpufreq.
current: 800 MHz [470 mV]
frequencies: 800 1000 1200 1400 1600 1800 (in MHz)
current: 2133 MHz [1803 mV]
frequencies: 800 1067 1333 1600 1867 2133 (in MHz)
[juan@nocturno][~]> ./powerctl -d cpufreq2-lkm -f 1333
(cpufreq2-lkm) 2133 -> 1333
sysmonioctl_cpufreq: copyout_ioctl, error=0.
sysmonioctl_cpufreq: newfreq: 1333 cfreq_t->cf_curfreq: 2133
sysmonioctl_cpufreq: mycnt: 6
sysmonioctl_cpufreq: newfreq: 1333 cf_fqlist[i]: 800
sysmonioctl_cpufreq: newfreq: 1333 cf_fqlist[i]: 1067
sysmonioctl_cpufreq: newfreq: 1333 cf_fqlist[i]: 1333
sysmonioctl_cpufreq: freqfound: 1
cpufreq2-lkm: newfreq: 1333
sysmonioctl_cpufreq: cf_newfreq: 1333
sysmonioctl_cpufreq: error: 0
Now running powerctl again:
zsh: segmentation fault (core dumped) ./powerctl
sysmonioctl_cpufreq: copyout_ioctl, error=12.
and drvctl -p cpu0 returns ENOMEM too, WTF?
Please review/comment/help to fix the thing.