search

hamadmarri / TT-CPU-Scheduler

Task Type (TT) is an alternative CPU Scheduler for linux.
110 stars 12 forks source link

Frequency locked with `ondemand` governor #5

Open raykzhao opened 3 years ago

raykzhao commented 3 years ago

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 commented 2 years ago

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 :+1:

hamadmarri commented 2 years ago

Hi @raykzhao

Could you please try this fix accounting.patch.zip

raykzhao commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

accounting.patch.zip @raykzhao

raykzhao commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@raykzhao

I am not sure if this is still relevant (Feb. 25, 2016) https://patchwork.kernel.org/project/linux-pm/patch/1825489.pc33SqXSIB@vostro.rjw.lan/

> 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 commented 2 years ago

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 commented 2 years ago

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.