cpufreq governors broken with !CONFIG_SMP?

cpufreq governors broken with !CONFIG_SMP?

[Steve Muckle]

While working on a few patches for schedutil I noticed that the CFS
cpufreq hooks depend on PELT, which depends on CONFIG_SMP.

I compiled and ran a UP kernel with intel_pstate. Running a cpu-bound
task did not result in the frequency increasing beyond fmin. For some reason
ondemand is working for me with the same test, not sure why yet.

It appears dbs/intel-pstate/schedutil have a dependency on CONFIG_SMP
now. Or am I missing something?

Re: cpufreq governors broken with !CONFIG_SMP?

[Rafael J. Wysocki]

On Thursday, May 05, 2016 04:49:22 PM Steve Muckle wrote:
> While working on a few patches for schedutil I noticed that the CFS
> cpufreq hooks depend on PELT, which depends on CONFIG_SMP.
> 
> I compiled and ran a UP kernel with intel_pstate. Running a cpu-bound
> task did not result in the frequency increasing beyond fmin. For some reason
> ondemand is working for me with the same test, not sure why yet.
> 
> It appears dbs/intel-pstate/schedutil have a dependency on CONFIG_SMP
> now. Or am I missing something?

You're right AFAICS.

For governors other than schedutil fixing that would be a matter of
adding a !CONFIG_SMP variant of update_load_avg() that will call
cpufreq_update_util() and do nothing else.  It doesn't matter what
is passed via util and max then.

In turn, schedutil should probably depend on CONFIG_SMP.

Peter?

Re: cpufreq governors broken with !CONFIG_SMP?

[Rafael J. Wysocki]

On Friday, May 06, 2016 02:09:07 AM Rafael J. Wysocki wrote:
> On Thursday, May 05, 2016 04:49:22 PM Steve Muckle wrote:
> > While working on a few patches for schedutil I noticed that the CFS
> > cpufreq hooks depend on PELT, which depends on CONFIG_SMP.
> > 
> > I compiled and ran a UP kernel with intel_pstate. Running a cpu-bound
> > task did not result in the frequency increasing beyond fmin. For some reason
> > ondemand is working for me with the same test, not sure why yet.
> > 
> > It appears dbs/intel-pstate/schedutil have a dependency on CONFIG_SMP
> > now. Or am I missing something?
> 
> You're right AFAICS.
> 
> For governors other than schedutil fixing that would be a matter of
> adding a !CONFIG_SMP variant of update_load_avg() that will call
> cpufreq_update_util() and do nothing else.  It doesn't matter what
> is passed via util and max then.

Maybe something like the below, FWIW, as a quick fix for 4.6?

---
 kernel/sched/fair.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Index: linux-pm/kernel/sched/fair.c
===================================================================
--- linux-pm.orig/kernel/sched/fair.c
+++ linux-pm/kernel/sched/fair.c
@@ -3030,7 +3030,14 @@ static int idle_balance(struct rq *this_
 
 #else /* CONFIG_SMP */
 
-static inline void update_load_avg(struct sched_entity *se, int update_tg) {}
+static inline void update_load_avg(struct sched_entity *se, int not_used)
+{
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	struct rq *rq = rq_of(cfs_rq);
+
+	cpufreq_trigger_update(rq_clock(rq));
+}
+
 static inline void
 enqueue_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
 static inline void

Re: cpufreq governors broken with !CONFIG_SMP?

[Steve Muckle]

On Fri, May 06, 2016 at 02:09:07AM +0200, Rafael J. Wysocki wrote:
> In turn, schedutil should probably depend on CONFIG_SMP.

In the long term I wonder if it's worth putting PELT under its own
separate feature or just removing #ifdef CONFIG_SMP.

Aside from task migration CPU frequency updates there's also task
creation and deletion which would apply on UP. The tunable
infrastructure being created for scheduler-guided frequency may be of
interest on UP also.

Re: cpufreq governors broken with !CONFIG_SMP?

[Rafael J. Wysocki]

On Thursday, May 05, 2016 05:25:19 PM Steve Muckle wrote:
> On Fri, May 06, 2016 at 02:09:07AM +0200, Rafael J. Wysocki wrote:
> > In turn, schedutil should probably depend on CONFIG_SMP.
> 
> In the long term I wonder if it's worth putting PELT under its own
> separate feature or just removing #ifdef CONFIG_SMP.
> 
> Aside from task migration CPU frequency updates there's also task
> creation and deletion which would apply on UP. The tunable
> infrastructure being created for scheduler-guided frequency may be of
> interest on UP also.

I agree, but I was talking short-term. :-)

We need to fix this for 4.6 (which most likely is 2 weeks away only) and
I don't think it hurts anyone if schedutil depends on CONFIG_SMP to start
with.

Re: cpufreq governors broken with !CONFIG_SMP?

[Peter Zijlstra]

On Fri, May 06, 2016 at 02:25:16AM +0200, Rafael J. Wysocki wrote:
> On Friday, May 06, 2016 02:09:07 AM Rafael J. Wysocki wrote:
> > On Thursday, May 05, 2016 04:49:22 PM Steve Muckle wrote:
> > > While working on a few patches for schedutil I noticed that the CFS
> > > cpufreq hooks depend on PELT, which depends on CONFIG_SMP.

> > For governors other than schedutil fixing that would be a matter of
> > adding a !CONFIG_SMP variant of update_load_avg() that will call
> > cpufreq_update_util() and do nothing else.  It doesn't matter what
> > is passed via util and max then.
> 
> Maybe something like the below, FWIW, as a quick fix for 4.6?

Right; I suppose that'll have to do for 4.6.

> ---
>  kernel/sched/fair.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> Index: linux-pm/kernel/sched/fair.c
> ===================================================================
> --- linux-pm.orig/kernel/sched/fair.c
> +++ linux-pm/kernel/sched/fair.c
> @@ -3030,7 +3030,14 @@ static int idle_balance(struct rq *this_
>  
>  #else /* CONFIG_SMP */
>  
> -static inline void update_load_avg(struct sched_entity *se, int update_tg) {}
> +static inline void update_load_avg(struct sched_entity *se, int not_used)
> +{
> +	struct cfs_rq *cfs_rq = cfs_rq_of(se);
> +	struct rq *rq = rq_of(cfs_rq);
> +
> +	cpufreq_trigger_update(rq_clock(rq));
> +}
> +
>  static inline void
>  enqueue_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
>  static inline void
> 

Re: cpufreq governors broken with !CONFIG_SMP?

[Peter Zijlstra]

On Thu, May 05, 2016 at 05:25:19PM -0700, Steve Muckle wrote:
> On Fri, May 06, 2016 at 02:09:07AM +0200, Rafael J. Wysocki wrote:
> > In turn, schedutil should probably depend on CONFIG_SMP.
> 
> In the long term I wonder if it's worth putting PELT under its own
> separate feature or just removing #ifdef CONFIG_SMP.

Probably should do that eventually yeah. It started out with being
smp&&cgroup only, then we started using it for regular balancing and it
became smp, and since we're now wanting to use it for pretty much
everything we should just remove that.

Re: cpufreq governors broken with !CONFIG_SMP?

[Rafael J. Wysocki]

On Fri, May 6, 2016 at 1:46 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, May 06, 2016 at 02:25:16AM +0200, Rafael J. Wysocki wrote:
>> On Friday, May 06, 2016 02:09:07 AM Rafael J. Wysocki wrote:
>> > On Thursday, May 05, 2016 04:49:22 PM Steve Muckle wrote:
>> > > While working on a few patches for schedutil I noticed that the CFS
>> > > cpufreq hooks depend on PELT, which depends on CONFIG_SMP.
>
>> > For governors other than schedutil fixing that would be a matter of
>> > adding a !CONFIG_SMP variant of update_load_avg() that will call
>> > cpufreq_update_util() and do nothing else.  It doesn't matter what
>> > is passed via util and max then.
>>
>> Maybe something like the below, FWIW, as a quick fix for 4.6?
>
> Right; I suppose that'll have to do for 4.6.

OK, thanks!

I'll resend it with a changelog and stuff then.