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

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

 



Hi, Matthias

Thank you for your help.

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

> 2011/2/10 Minoru Usui <usui@xxxxxxxxxxxxxxxxx>:
> > Hi, Eric
> >
> > On Tue, 1 Feb 2011 10:26:36 +0900
> > Minoru Usui <usui@xxxxxxxxxxxxxxxxx> wrote:
> >
> >> Hi, Eric
> >>
> >> On Mon, 31 Jan 2011 15:46:39 -0700
> >> Eric Blake <eblake@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx>
> >> >
> >> > 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@xxxxxxxxxxxxxxxxx>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]