lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3000308.Hi6zX8my0D@vostro.rjw.lan>
Date:	Thu, 30 May 2013 00:29:49 +0200
From:	"Rafael J. Wysocki" <rjw@...k.pl>
To:	Stratos Karafotis <stratosk@...aphore.gr>
Cc:	Viresh Kumar <viresh.kumar@...aro.org>, cpufreq@...r.kernel.org,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] cpufreq: ondemand: Increase frequency to any value proportional to load

On Wednesday, May 29, 2013 06:15:56 PM Stratos Karafotis wrote:
> On 05/28/2013 11:54 PM, Rafael J. Wysocki wrote:
> > On Tuesday, May 28, 2013 08:03:19 PM Stratos Karafotis wrote:
> >> I mean any value of freq table. Please let me know if you want me to rephrase
> >> it in description.
> > 
> > Yes, it would be nice to be more precise.
> 
> OK sure, I will add a more precise description.
> 
> 
> > Which is equivalent to saying that __cpufreq_driver_getavg() is not useful and
> > may be removed (but the patch doesn't do that and I wonder why?), but surely
> > the developer who added it wouldn't agree.
> > 
> > So, please explain: why can we drop __cpufreq_driver_getavg()?
> > 
> 
> With the new proposed method the next frequency is not dependent from current
> or average frequency. The next frequency is dependent only from load.
> So, we don't need support for freq feedback from hardware anymore.

OK, but that's a more significant change than the changelog suggests.
The changelog should tell the whole story and explain the rationale. :-)

So, please explain that in fact it is not necessary to use the current or
average frequency to compute the target and why that is the case.

Also the patch should remove __cpufreq_driver_getavg() and the callback used by
it, since that code will be dead after applying it anyway.

> Even if, due to underlying hardware coordination mechanism, CPU runs in different 
> frequency than the actual, the calculation of load and of target frequency will
> remain the unaffected, with this patch.
> 
> With full respect to ondemand coauthor, and if the new method is acceptable,
> I could send a patch to revert the original one about the IA32_APERF and
> IA32_MPERF MSR support.

I'm not sure what you mean by "revert", but please do as I said above.

> >> Thus, in the comparison with up_threshold to increase frequency we actually
> >> do this (in cases that getavg is not implemented):
> >> if (load > up_theshold)
> >> 	increase to max
> >>
> >> So, after the patch we keep the same comparison to decide about the max frequency.
> >> I thought, that below up_threshold is 'fair' to decide about the next
> >> frequency with formula that frequency is proportional to load.
> >> For example in a CPU with min freq 100MHz and max 1000MHz with a
> >> load 50 target frequency should be 500MHz.
> > 
> > Well, that sounds reasonable, but the patch actually does more than that.
> 
> I'm sorry, but I didn't understand your last point.

Please see above.

The changelog doesn't even mention that the code is being switched from using
measured past frequencies to not using them, because you think that there's a
better way of computing the target (which by the way I can agree with :-)).

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ