linux-kernel - Re: [PATCH] cpufreq: intel_pstate: Fix rounding of core

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1815178.vjJU51ubGD@vostro.rjw.lan>
Date:	Thu, 12 Jun 2014 22:03:15 +0200
From:	"Rafael J. Wysocki" <rjw@...ysocki.net>
To:	Stratos Karafotis <stratosk@...aphore.gr>
Cc:	Doug Smythies <dsmythies@...us.net>, viresh.kumar@...aro.org,
	dirk.j.brandewie@...el.com, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct

On Thursday, June 12, 2014 05:35:59 PM Stratos Karafotis wrote:
> On 12/06/2014 12:15 πμ, Doug Smythies wrote:
> > 
> > 
> > -----Original Message-----
> > From: Stratos Karafotis [mailto:stratosk@...aphore.gr] 
> > Sent: June-11-2014 13:20
> > To: Doug Smythies
> > Cc: linux-pm@...r.kernel.org; linux-kernel@...r.kernel.org; rjw@...ysocki.net; viresh.kumar@...aro.org; dirk.j.brandewie@...el.com
> > Subject: Re: [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct
> > 
> > On 2014.06.11 13:20 Stratos Karafotis wrote:
> >> On 11/06/2014 06:02 μμ, Doug Smythies wrote:
> >>>
> >>> On 2104.06.11 07:08 Stratos Karafotis wrote:
> >>>> On 11/06/2014 04:41 μμ, Doug Smythies wrote:
> >>>>
> >>>> No.
> >>>
> >>>> The intent was only ever to round properly the pseudo floating point result of the divide.
> >>>> It was much more important (ugh, well 4 times more) when FRACBITS was still 6, which also got changed to 8 in a recent patch.
> >>>>
> >>>
> >>> Are you sure?
> >>>
> >>> Yes.
> >>>
> >>>> This rounding was very recently added.
> >>>> As far as I can understand, I don't see the meaning of this rounding, as is.
> >>>> Even if FRAC_BITS was 6, I think it would have almost no improvement in
> >>>> calculations.
> >>>
> >>> Note: I had not seen this e-mail when I wrote a few minutes ago:
> >>>
> >>> You may be correct.
> >>> If Dirk agrees, I will re-analyse the entire driver for rounding effects soon.
> >>> When FRACBITS was 6 there were subtle cases where the driver would get stuck, and not make a final pstate change, with the default PID gains.
> >>> Other things have changed, and the analysis needs to be re-done.
> >>>
> > 
> >> Could you please elaborate a little bit more what we need these 2 lines below?
> >>
> >>        if ((rem << 1) >= int_tofp(sample->mperf))
> >>                core_pct += 1;
> >>
> >> Because nothing is mentioned for them in commit's changelog.
> >> Do we need to round core_pct or not?
> >> Because if we try to round it, I think this patch should work.
> > 
> > As mentioned originally, they are there just to round the pseudo floating number, not the integer portion only.
> > Let us bring back the very numbers you originally gave and work through it.
> > 
> > aperf = 5024
> > mperf = 10619
> > 
> > core_pct = 47.31142292%
> > or 47 and 79.724267 256ths
> > or to the closest kept fractional part 47 and 80 256ths
> > or 12112 as a pseudo float.
> > The maximum error with this rounding will be 1 part in 512 and symmetric instead of the 1 part in 256 always in one direction without.
> > 
> > Now if FRACBITS was still 6:
> > core_pct = 47.31142292%
> > or 47 and 19.931 64ths
> > or to the closest kept fractional part 47 and 20 64ths
> > or 3028 as a pseudo float.
> > The maximum error with this rounding will be 1 part in 128 and symmetric instead of the 1 part in 64 (1.6% !!!) always in one direction without.
> > 
> > Hope this helps.
> > 
> 
> Yes, it helps. Thanks a lot!
> 
> But please note that the maximum error without this rounding will be 1.6% _only_
> in fractional part. In the real number it will be much smaller:
> 
> 47.19 instead of 47.20
> 
> And using FRAC_BITS 8:
> 
> 47.79 instead of 47.80
> 
> This is a 0.0002% difference. I can't see how this is can affect the calculations
> even with FRAC_BITS 6.
> 
> Another thing is that this algorithm generally is used to round to the
> nearest integer. I'm not sure if it's valid to apply it for the rounding of
> the fractional part of fixed point number.

Depending on the original reason, it may or may not be.

In theory, it may help reduce numerical drift resulting from rounding always in
one direction only, but I'm not really sure if that matters here.

Doug seems to have carried out full analysis, though.

Rafael

--
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/