[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 10/11] x86/intel_pstate: support the use of intel_pstate in pmstat.c



>>> On 09.09.15 at 17:16, <wei.w.wang@xxxxxxxxx> wrote:
> On 09/09/2015 21:12,  Jan Beulich wrote:
>>>> On 09.09.15 at 14:56, <wei.w.wang@xxxxxxxxx> wrote:
>> Can you please explain more why it doesn't scale? 
>> From my point of view, any other future value representation can be 
>> passed from the producer to the related consumer through this method.
> 
>> Did you read all of my earlier replies? I already said there "Just consider 
> what happens to the code when we end up gaining a few 
>> more drivers providing percentage values, and perhaps another one providing 
> a third variant of output representation."
> 
> Yes, I have read that. I am not sure if I got your point, but my meaning was 
> when we add new drivers to the code, e.g. xx_pstate driver, we can still have 
> the name, "xx_pstate", assigned to "p_cpufreq->scaling_driver" to distinguish 
> one another. If the driver uses a different variant of output representation, 
> which cannot be held by " uint32_t scaling_max_perf" (it needs "uint64_t" for 
> example, then that driver developer needs to add a new field here like  " 
> uint64_t scaling_max_perf_xx").
> What is the scaling problem? 

        if (strcmp() == 0 ||
            strcmp() == 0 ||
            strcmp() == 0) {
        ...
        } else if (strcmp() == 0) {
        ...
        } else {
        ...
        }

is just ugly, and gets all the uglier the more strcmp()s get added.
Have a boolean or enumeration indicating what kind of data there
is, and the above changes to

        switch (kind) {
        case absolute: ...
        case percentage: ...
        }

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.