linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).