Re: [Xen-devel] [RFC PATCH v2 10/12] cpufreq: add dom0-cpufreq driver

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.10.14 at 10:43, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
> On Fri, Oct 17, 2014 at 4:03 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 16.10.14 at 13:27, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
>>> +static void notify_cpufreq_domains(void)
>> Why "domains", not "hwdom"? You don't really want to send this
>> to other than the hardware domain I hope.
> All domains (not only hwdomain) should receive this interrupt.
> In case is hwdomain is Linux kernel it can automaticaly recalculate
> jiffies. But other domains should recalculate jiffies too. I'll
> implement this mechanism a bit later.

This needs more explanation then: If all domains need to do
adjustments when the frequency changes, there's something
wrong here imo. Such changes are supposed to be visible to
the hypervisor only (with, in the case here, assistance - but
nothing else - by the hardware domain).

>>> +    /* Do send cmd for Dom0 */
>>> +    spin_lock(&sysctl_cpufreq_lock);
>>> +    /* return previous result */
>>> +    ret = sysctl_cpufreq_data.result;
>>> +
>>> +    sysctl_cpufreq_data.cpu = policy->cpu;
>>> +    sysctl_cpufreq_data.freq = freqs.new;
>>> +    sysctl_cpufreq_data.relation = (uint32_t)relation;
>>> +    spin_unlock(&sysctl_cpufreq_lock);
>>
>> sysctl_cpufreq_data fields appear to be accessed only here. Is the
>> patch incomplete? It is certainly not possible to understand how this
>> is intended to work without the other sides being there.
> Now sysctl_cpufreq_data fields is accessed only here. But it will be 
> accessed
> in XEN_SYSCTL_cpufreq_op handler (in the next patch in this series).

I.e. the split of changes among patches is suboptimal.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel