[libvirt] [PATCH 1/2] Get cpuMhz of virNodeGetInfo() from cpufreq/cpuinfo_max_freq, if exist

[Minoru Usui]

Hi, Matthias

Thank you for your help.

On Thu, 10 Feb 2011 19:12:46 +0100
Matthias Bolte <matthias.bolte at googlemail.com> wrote:

> 2011/2/10 Minoru Usui <usui at mxm.nes.nec.co.jp>:
> > Hi, Eric
> >
> > On Tue, 1 Feb 2011 10:26:36 +0900
> > Minoru Usui <usui at mxm.nes.nec.co.jp> wrote:
> >
> >> Hi, Eric
> >>
> >> On Mon, 31 Jan 2011 15:46:39 -0700
> >> Eric Blake <eblake at redhat.com> wrote:
> >>
> >> > On 01/27/2011 02:51 AM, Minoru Usui wrote:
> >> > > virNodeGetInfo() gets from
> >> > > /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq, first.
> >> > >
> >> > > Signed-off-by: Minoru Usui <usui at mxm.nes.nec.co.jp>
> >> >
> >> > I haven't looked closely at this series yet...
> >> >
> >> > > +    /*
> >> > > +     * nodeinfo->mhz should return maximum frequency,
> >> > > +     * but "cpu MHz" of /proc/cpuinfo is scaled by power saving feature.
> >> > > +     * So it gets cpufreq/cpuinfo_max_freq, if possible.
> >> > > +     */
> >> > > +    ret = get_cpu_value(0, "cpufreq/cpuinfo_max_freq", true);
> >> > > +    if (ret < 0)
> >> > > + return -1;
> >> > > +    else if (ret != 1) {
> >> > > + /* convert unit */
> >> > > + cpu_mhz = ret / 1000;
> >> >
> >> > But which units is this converting between, and should it truncate or
> >> > round up?
> >>
> >> I think it divide by 1000 is collect, because my machine returns below values.
> >>
> >> -----------------------------------------------------------------
> >> # cat /sys/devices/system/cpu/cpu?/cpufreq/cpuinfo_max_freq
> >> 2333331
> >> 2333331
> >> 2333331
> >> 2333331
> >> 2333331
> >> 2333331
> >> 2333331
> >> 2333331
> >>
> >> # grep 'cpu MHz' /proc/cpuinfo
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> cpu MHz         : 2333.331
> >> -----------------------------------------------------------------
> >>
> >> On the other hand, I don't have clear opinion about truncate or round up.
> >> Present implementation of linuxNodeInfoCPUPopulate() selects truncate,
> >> so I implement to truncate logic.
> >
> > Are my explanations enough?
> > If not or I'm misunderstanding something, please let me know.
> >
> 
> I think we should round up in case we request something, because then
> we get at least what we requested. If we truncate a request we might
> get less than we requested, this might result in a bad situation.
> Recently we switched from truncation to rounding up for memory and
> storage request because of this.
> 
> But here we are reporting something. In that case we should truncate,
> because if we would round up we could report more than there actually
> is, this might result in a bad situation too.
> 
> So with rounding up request and truncating reports we are on the safe
> side in both cases.
> 
> Matthias

I agree with you.
In this case we are reporting cpu MHz, so we should truncate it.

Eric, what do you think?
-- 
Minoru Usui <usui at mxm.nes.nec.co.jp>