From: Saravana Kannan <skannan@codeaurora.org>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: eas-dev@lists.linaro.org, linux-pm@vger.kernel.org,
Peter Zijlstra <peterz@infradead.org>,
Rafael Wysocki <rjw@rjwysocki.net>,
linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
smuckle.linux@gmail.com, Len Brown <lenb@kernel.org>
Subject: Re: [Eas-dev] [PATCH V3 1/3] sched: cpufreq: Allow remote cpufreq callbacks
Date: Fri, 28 Jul 2017 14:05:39 -0700 [thread overview]
Message-ID: <597BA723.1060908@codeaurora.org> (raw)
In-Reply-To: <20170728060054.GU352@vireshk-i7>
On 07/27/2017 11:00 PM, Viresh Kumar wrote:
> On 27-07-17, 12:55, Saravana Kannan wrote:
>> Yes. Simplifying isn't always about number of lines of code. It's also about
>> abstraction. Having generic scheduler code care about HW details doesn't
>> seem nice.
>
> I can argue that even the policy->cpus field is also hardware
> specific, isn't it ?
Yes.
> And we are using that in the schedutil governor
> anyway.
Yes
> What's wrong with having another field (in a generic way) in
> the same structure that tells us more about hardware ?
Nothing wrong. I'm not saying you shouldn't have the cpu field in the
data or as a parameter to the hook. You'll definitely need that.
> And then schedutil isn't really scheduler, but a cpufreq governor.
> Just like ondemand/conservative, which are also called from the same
> scheduler path.
Exactly. I never debated anything about schedutil. I'm just saying don't
have any CPU limitations or check on the scheduler side when sending
notification. Scheduler shouldn't have to know/care of the driver can
only set the freq on that CPU or across CPUs in a cluster or across the
entire system.
>> It'll literally one simple check (cpu == smp_processor_id()) or (cpu "in"
>> policy->cpus).
>>
>> Also, this is only for drivers that currently support fast switching. How
>> many of those do you have?
>
> Why? Why shouldn't we do that for the other drivers? I think it should
> be done across everyone.
Because if I remember it right, the "don't send the notification if it's
not the same CPU" limitation is only for the fast switching case? I
might be mistaken about this part though.
>>> The core already has most of the data required and I believe that we
>>> need to handle it in the governor's code as is handled in this series.
>>
>> Clearly, it doesn't. You are just making assumptions about HW.
>
> So assuming that any CPU from a policy can change freq on behalf of
> all the CPUs of the same policy is wrong? That is the basis of how the
> cpufreq core is designed.
1. I'm not saying that. I'm saying assuming CPUs can change the freq
only on behalf of all the CPUs in the same policy is wrong. Again, the
scheduler or governor shouldn't even be making any of that assumption.
That's a CPUfreq driver problem.
2. No, that is not the basis of the entire cpufreq core design. None of
the existing CPUfreq code has any assumptions that only CPUs in a policy
can change their frequency. It doesn't break in any way in system where
any CPU can change any other CPU's frequency -- all Qualcomm chips are
like that. It's only the recent scheduler notifier changes that are
adding this additional limitation and breaking stuff for systems where
any CPU can change any other CPU's frequency.
-Saravana
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
next prev parent reply other threads:[~2017-07-28 21:05 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-13 6:44 [PATCH V3 0/3] sched: cpufreq: Allow remote callbacks Viresh Kumar
2017-07-13 6:44 ` [PATCH V3 1/3] sched: cpufreq: Allow remote cpufreq callbacks Viresh Kumar
2017-07-21 13:03 ` Peter Zijlstra
2017-07-24 11:01 ` Viresh Kumar
2017-07-24 13:47 ` Peter Zijlstra
2017-07-26 6:29 ` Viresh Kumar
2017-07-26 8:12 ` Peter Zijlstra
2017-07-26 17:26 ` Rafael J. Wysocki
2017-07-26 21:00 ` [Eas-dev] " Saravana Kannan
2017-07-27 3:30 ` Viresh Kumar
2017-07-27 19:55 ` Saravana Kannan
2017-07-28 4:33 ` Joel Fernandes (Google)
2017-07-28 6:00 ` Viresh Kumar
2017-07-28 21:05 ` Saravana Kannan [this message]
2017-07-31 3:58 ` Viresh Kumar
2017-07-13 6:44 ` [PATCH V3 2/3] cpufreq: schedutil: Process remote callback for shared policies Viresh Kumar
2017-07-14 2:02 ` [Eas-dev] " Saravana Kannan
2017-07-14 5:03 ` Viresh Kumar
2017-07-20 13:58 ` Peter Zijlstra
2017-07-20 12:22 ` Peter Zijlstra
2017-07-20 15:11 ` Sudeep Holla
2017-07-26 20:56 ` Saravana Kannan
2017-07-13 6:44 ` [PATCH V3 3/3] cpufreq: governor: " Viresh Kumar
2017-07-13 15:17 ` [PATCH V3 0/3] sched: cpufreq: Allow remote callbacks Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=597BA723.1060908@codeaurora.org \
--to=skannan@codeaurora.org \
--cc=eas-dev@lists.linaro.org \
--cc=lenb@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=smuckle.linux@gmail.com \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).