Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

Frequency locked with ondemand governor #5

Open
raykzhao opened this issue Nov 8, 2021 · 11 comments
Open

Frequency locked with ondemand governor #5

raykzhao opened this issue Nov 8, 2021 · 11 comments
Labels
bug Something isn't working

Comments

@raykzhao
Copy link

raykzhao commented Nov 8, 2021

Hi @hamadmarri,

I notice that with the tt-scheduler on 5.15 kernel, the lowest frequency of some CPU cores might be locked at a random frequency with the ondemand scaling governor (both with and w/o intel_pstate). There is no such issue with the cfs scheduler on my machine.

This could be easily solved with cpupower frequency-set -g powersave and then cpupower frequency-set -g ondemand. However, since I have noticed that sometimes the lowest frequency of a CPU core might be locked at e.g. turbo frequency, it would be nice to make sure that the ondemand governor can work correctly for laptops during boot without intervention. I remember one earlier version of MuQSS/PDS had the same issue so I guess this might be related to the stats.

Another issue I have noticed is that, with nohz_full enabled for all cores except CPU0, some random CPU cores other than CPU0 might be locked at a low frequency i.e. no frequency scaling at all with the ondemand governor even under load. It seems that cfs scheduler does not have the frequency scaling issue with nohz_full. Also I can confirm via /proc/sched_debug that there is actually a task running on the frequency locked cores under load with the tt-scheduler.

By the way, thanks for your awesome scheduler! In my daily usage on my laptop, so far my system is quite smooth and I did not notice any freezes. I am using the tt-scheduler patch on top of the vanilla kernel with some other patches (most are picked from https://github.com/sirlucjan/kernel-patches/tree/master/5.15).

@hamadmarri
Copy link
Owner

hamadmarri commented Nov 9, 2021

Hi @raykzhao

Thank you for testing and debugging the scheduler. I believe that we had the same issue regarding cpus freq. with RDB before and the fix was related to the task stats and accounting. I will add a sub kconfig option under TT to allow for full tasks accounting (please see https://github.com/hamadmarri/TT-CPU-Scheduler#future-plan).

It is a serious issue since the performance of the task which is running on the locked cpu with low frequency will be affected (in case if cpu freq. is not just falsely reporting some numbers while the actual cpu freq is higher).

Thank you so much 👍

@hamadmarri
Copy link
Owner

hamadmarri commented Nov 13, 2021

Hi @raykzhao

Could you please try this fix
accounting.patch.zip

@raykzhao
Copy link
Author

Hi @hamadmarri,

It seems that with this patch, CONFIG_NUMA_BALANCING must be disabled now. Otherwise I got the following errors during compiling:

In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:232:22: error: redefinition of ‘capacity_of’
  232 | static unsigned long capacity_of(int cpu)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:593:22: note: previous definition of ‘capacity_of’ with type ‘long unsigned int(int)’
  593 | static unsigned long capacity_of(int cpu)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:520:29: error: redefinition of ‘cfs_rq_runnable_avg’
  520 | static inline unsigned long cfs_rq_runnable_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:499:29: note: previous definition of ‘cfs_rq_runnable_avg’ with type ‘long unsigned int(struct cfs_rq *)’
  499 | static inline unsigned long cfs_rq_runnable_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:525:29: error: redefinition of ‘cfs_rq_load_avg’
  525 | static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:489:29: note: previous definition of ‘cfs_rq_load_avg’ with type ‘long unsigned int(struct cfs_rq *)’
  489 | static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:699:22: error: redefinition of ‘task_h_load’
  699 | static unsigned long task_h_load(struct task_struct *p)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:719:22: note: previous definition of ‘task_h_load’ with type ‘long unsigned int(struct task_struct *)’
  719 | static unsigned long task_h_load(struct task_struct *p)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:783:29: error: redefinition of ‘cpu_util’
  783 | static inline unsigned long cpu_util(int cpu)
      |                             ^~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:509:29: note: previous definition of ‘cpu_util’ with type ‘long unsigned int(int)’
  509 | static inline unsigned long cpu_util(int cpu)
      |                             ^~~~~~~~

@hamadmarri
Copy link
Owner

Hi @hamadmarri,

It seems that with this patch, CONFIG_NUMA_BALANCING must be disabled now. Otherwise I got the following errors during compiling:

In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:232:22: error: redefinition of ‘capacity_of’
  232 | static unsigned long capacity_of(int cpu)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:593:22: note: previous definition of ‘capacity_of’ with type ‘long unsigned int(int)’
  593 | static unsigned long capacity_of(int cpu)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:520:29: error: redefinition of ‘cfs_rq_runnable_avg’
  520 | static inline unsigned long cfs_rq_runnable_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:499:29: note: previous definition of ‘cfs_rq_runnable_avg’ with type ‘long unsigned int(struct cfs_rq *)’
  499 | static inline unsigned long cfs_rq_runnable_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:525:29: error: redefinition of ‘cfs_rq_load_avg’
  525 | static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:489:29: note: previous definition of ‘cfs_rq_load_avg’ with type ‘long unsigned int(struct cfs_rq *)’
  489 | static inline unsigned long cfs_rq_load_avg(struct cfs_rq *cfs_rq)
      |                             ^~~~~~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:699:22: error: redefinition of ‘task_h_load’
  699 | static unsigned long task_h_load(struct task_struct *p)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:719:22: note: previous definition of ‘task_h_load’ with type ‘long unsigned int(struct task_struct *)’
  719 | static unsigned long task_h_load(struct task_struct *p)
      |                      ^~~~~~~~~~~
In file included from kernel/sched/bs.c:10:
kernel/sched/tt_stats.h:783:29: error: redefinition of ‘cpu_util’
  783 | static inline unsigned long cpu_util(int cpu)
      |                             ^~~~~~~~
In file included from kernel/sched/bs.c:9:
kernel/sched/fair_numa.h:509:29: note: previous definition of ‘cpu_util’ with type ‘long unsigned int(int)’
  509 | static inline unsigned long cpu_util(int cpu)
      |                             ^~~~~~~~

Ops, I am working on it.

@hamadmarri
Copy link
Owner

accounting.patch.zip
@raykzhao

@raykzhao
Copy link
Author

Hi @hamadmarri,

Thank you for the patch! Both CPU frequency scaling issues with ondemand governor have been fixed. I will also try the schedutil governor later.

@raykzhao
Copy link
Author

raykzhao commented Nov 13, 2021

Hi @hamadmarri,

Although schedutil seems working fine when CONFIG_NO_HZ_FULL=n, however, the frequency of some nohz_full enabled CPU cores are locked when using schedutil and intel_pstate (note that similar issue with ondemand governor has already been fixed by your patch). I will see if disabling intel_pstate will make any difference in this case.

@hamadmarri
Copy link
Owner

hamadmarri commented Nov 13, 2021

Hi @hamadmarri,

Although schedutil seems working fine when CONFIG_NO_HZ_FULL=n, however, the frequency of some nohz_full enabled CPU cores are still locked when using schedutil and intel_pstate. I will see if disabling intel_pstate will make any difference in this case.

Yes, I still have this problem. I think I am missing some load updates in one of the nohz function. I will check.

Please see this update

accounting.patch.zip

@hamadmarri
Copy link
Owner

@raykzhao

I am not sure if this is still relevant (Feb. 25, 2016)
https://patchwork.kernel.org/project/linux-pm/patch/[email protected]/

> There's also NO_HZ_FULL. I don't know that anyone using NO_HZ_FULL would
> want to use a governor other than performance, but I thought I'd mention
> it just in case.

Yup.  That's broken already with anything different from "performance".

I think we should fix it, but it can be taken care of later.  Plus that's one
of the things that need to be fixed in general rather than for a single
governor.

I forgot to update load in the most important functions :D
update_curr, set_next_task, put_prev_task, and others.

accounting.patch.zip

@raykzhao
Copy link
Author

Hi @hamadmarri,

With the latest patch, schedutil works fine with nohz_full on my laptop when intel_pstate is disabled. However, the frequencies are still locked when intel_pstate is enabled. I guess there could be issues with how intel_pstate interacts with schedutil.

@raykzhao
Copy link
Author

raykzhao commented Nov 14, 2021

Hi @hamadmarri,

I just tested the cfs with schedutil, nohz_full, and intel_pstate, and I still encounter the same issue. So I think this is an upstream bug.

Thank you for the fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants