Re: [PATCH 3/3] cpufreq: schedutil: remove redundant code from sugov_next_freq_shared()

From: Rafael J. Wysocki
Date: Tue Mar 07 2017 - 08:20:04 EST


On Tue, Mar 7, 2017 at 11:31 AM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> On 06-03-17, 13:24, Rafael J. Wysocki wrote:
>> On Mon, Mar 6, 2017 at 5:45 AM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>> > On 04-03-17, 01:11, Rafael J. Wysocki wrote:
>> >> So one idea is that if SCHED_CPUFREQ_RT_DL is set in flags, we don't even
>> >> need to start the loop which is quite a cost to simply notice that there's
>> >> nothing to do.
>> >
>> > Hmm. Isn't the probability of this flag being set, same for all CPUs in the
>> > policy?
>>
>> No, I don't think so.
>
> Why do you think so? I thought all CPU in the policy can have the RT/DL flag set
> and the probability of all of them is just the same.

Well, yes, but if the current CPU has that flag set already, we surely
don't need to check the other ones in the policy?

>> So to the point, the code was written this way on purpose and not just
>> by accident as your changelog suggests and
>
> I didn't wanted to convey that really and I knew that it was written on purpose.
>
>> if you want to change it, you need numbers.
>
> What kind of numbers can we get for such a change ? I tried to take the running
> average of the time it takes to execute this routine over 10000 samples, but it
> varies a lot even with the same build. Any tests like hackbench, etc wouldn't be
> of any help as well.

So why do you think it needs to be changed, but really?

Is that because it is particularly hard to follow or similar?

Thanks,
Rafael