sched/fair: vruntime should normalize when switching from fair

From: John Dias <[email protected]>

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.
Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.
The immediate result is inflation of the task's vruntime, giving it lower priority (starving it if there's enough available work). The longer-term effect is inflation of all vruntimes because the task's vruntime becomes the rq's min_vruntime when the higher priority tasks go idle. That leads to a vicious cycle, where the vruntime inflation repeatedly doubled.

The change here is to detect when vruntime_normalized is being called when the task is waking but is waking in another class, and to conclude that this is a case where vruntime has not been normalized.

Signed-off-by John Dias <[email protected]>

Signed-off-by Steve Muckle <[email protected]>

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

kernel/sched/fair.c +0 -0

index b39fb596f6c1..14011d7929d8 100644

Quoted 48 lines · Steve Muckle

From: John Dias <[email protected]>

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.
Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
   the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier. The immediate result is inflation of the task's vruntime, giving it lower priority (starving it if there's enough available work). The longer-term effect is inflation of all vruntimes because the task's vruntime becomes the rq's min_vruntime when the higher priority tasks go idle. That leads to a vicious cycle, where the vruntime inflation repeatedly doubled.

The change here is to detect when vruntime_normalized is being called when the task is waking but is waking in another class, and to conclude that this is a case where vruntime has not been normalized.

Signed-off-by John Dias <[email protected]>

Signed-off-by Steve Muckle <[email protected]>

---
  kernel/sched/fair.c | 3 ++-

  1 file changed, 2 insertions(+), 1 deletion(-)

kernel/sched/fair.c +2 -1

index b39fb596f6c1..14011d7929d8 100644
@@ -9638,7 +9638,8 @@ static inline bool vruntime_normalized(struct 
 task_struct *p)
        * - A task which has been woken up by try_to_wake_up() and
        *   waiting for actually being woken up by sched_ttwu_pending().
        */
-    if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+    if (!se->sum_exec_runtime ||
+        (p->state == TASK_WAKING && p->sched_class == 
 &fair_sched_class))
           return true;

Quoted 1 line

      return false;

The normalization of vruntime used to exist in task_waking but it was removed and the normalization was moved into migrate_task_rq_fair. The reasoning being that task_waking_fair was only hit when a task is queued onto a different core and migrate_task_rq_fair should do the same work.

However, we're finding that there's one case which migrate_task_rq_fair doesn't hit: that being the case where rt_mutex_setprio changes a task's scheduling class to RT when its scheduled out. The task never hits migrate_task_rq_fair because it is switched to RT and migrates as an RT task. Because of this we're getting an unbounded addition of min_vruntime when the task is re-attached to the CFS runqueue when it loses the inherited priority. The patch above works because now the kernel specifically checks for this case and normalizes accordingly.

Here's the patch I was talking about: https://lore.kernel.org/patchwork/patch/677689/. In our testing we were seeing vruntimes nearly double every time after rt_mutex_setprio boosts the task to RT.

Signed-off-by Miguel de Dios <[email protected]>

Tested-by Miguel de Dios <[email protected]>

Hi,

Quoted 65 lines · Miguel de Dios

Quoted 49 lines · Steve Muckle

From: John Dias <[email protected]>

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.
Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING,
   which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier. The immediate result is inflation of the task's vruntime, giving it lower priority (starving it if there's enough available work). The longer-term effect is inflation of all vruntimes because the task's vruntime becomes the rq's min_vruntime when the higher priority tasks go idle. That leads to a vicious cycle, where the vruntime inflation repeatedly doubled.

The change here is to detect when vruntime_normalized is being called when the task is waking but is waking in another class, and to conclude that this is a case where vruntime has not been normalized.

Signed-off-by John Dias <[email protected]>

Signed-off-by Steve Muckle <[email protected]>

---
  kernel/sched/fair.c | 3 ++-

  1 file changed, 2 insertions(+), 1 deletion(-)

kernel/sched/fair.c +2 -1

index b39fb596f6c1..14011d7929d8 100644
@@ -9638,7 +9638,8 @@ static inline bool vruntime_normalized(struct
 task_struct *p)
        * - A task which has been woken up by try_to_wake_up() and
        *   waiting for actually being woken up by sched_ttwu_pending().
        */
-    if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+    if (!se->sum_exec_runtime ||
+        (p->state == TASK_WAKING && p->sched_class ==
 &fair_sched_class))
           return true;

Quoted 1 line

      return false;

The normalization of vruntime used to exist in task_waking but it was removed and the normalization was moved into migrate_task_rq_fair. The reasoning being that task_waking_fair was only hit when a task is queued onto a different core and migrate_task_rq_fair should do the same work.

However, we're finding that there's one case which migrate_task_rq_fair doesn't hit: that being the case where rt_mutex_setprio changes a task's scheduling class to RT when its scheduled out. The task never hits migrate_task_rq_fair because it is switched to RT and migrates as an RT task. Because of this we're getting an unbounded addition of min_vruntime when the task is re-attached to the CFS runqueue when it loses the inherited priority. The patch above works because now the kernel specifically checks for this case and normalizes accordingly.

Quoted 7 lines

Here's the patch I was talking about:
https://lore.kernel.org/patchwork/patch/677689/. In our testing we were seeing vruntimes nearly double every time after rt_mutex_setprio boosts the task to RT.

Signed-off-by Miguel de Dios <[email protected]>

Tested-by Miguel de Dios <[email protected]>

I tried to catch this issue on my Arm64 Juno board using pi_test (and a slightly adapted pip_test (usleep_val = 1500 and keep low as cfs)) from rt-tests but wasn't able to do so.

# pi_stress --inversions=1 --duration=1 --groups=1 --sched id=low,policy=cfs

Starting PI Stress Test
Number of thread groups: 1
Duration of test run: 1 seconds
Number of inversions per group: 1
Admin thread SCHED_FIFO priority 4
1 groups of 3 threads will be created
High thread SCHED_FIFO priority 3
Med thread SCHED_FIFO priority 2
Low thread SCHED_OTHER nice 0

# ./pip_stress

In both cases, the cfs task entering rt_mutex_setprio() is queued, so dequeue_task_fair()->dequeue_entity(), which subtracts cfs_rq->min_vruntime from se->vruntime, is called on it before it gets the rt prio.

Maybe it requires a very specific use of the pthread library to provoke this issue by making sure that the cfs tasks really blocks/sleeps?

Quoted 22 lines

I tried to catch this issue on my Arm64 Juno board using pi_test (and a slightly adapted pip_test (usleep_val = 1500 and keep low as cfs)) from rt-tests but wasn't able to do so.

# pi_stress --inversions=1 --duration=1 --groups=1 --sched id=low,policy=cfs

Starting PI Stress Test
Number of thread groups: 1
Duration of test run: 1 seconds
Number of inversions per group: 1
Admin thread SCHED_FIFO priority 4
1 groups of 3 threads will be created
High thread SCHED_FIFO priority 3
Med thread SCHED_FIFO priority 2
Low thread SCHED_OTHER nice 0

# ./pip_stress

In both cases, the cfs task entering rt_mutex_setprio() is queued, so dequeue_task_fair()->dequeue_entity(), which subtracts cfs_rq->min_vruntime from se->vruntime, is called on it before it gets the rt prio.

Quoted 2 lines

Maybe it requires a very specific use of the pthread library to provoke this issue by making sure that the cfs tasks really blocks/sleeps?

Maybe one could play with rt-app to recreate such specific use case?

https://github.com/scheduler-tools/rt-app/blob/master/doc/tutorial.txt#L459

Quoted 5 lines · Dietmar Eggemann

Hi,

Quoted 2 lines · Miguel de Dios

Quoted 1 line · Steve Muckle

From: John Dias <[email protected]>

Quoted 23 lines

I tried to catch this issue on my Arm64 Juno board using pi_test (and a slightly adapted pip_test (usleep_val = 1500 and keep low as cfs)) from rt-tests but wasn't able to do so.

# pi_stress --inversions=1 --duration=1 --groups=1 --sched id=low,policy=cfs

Starting PI Stress Test
Number of thread groups: 1
Duration of test run: 1 seconds
Number of inversions per group: 1
Admin thread SCHED_FIFO priority 4
1 groups of 3 threads will be created
High thread SCHED_FIFO priority 3
Med thread SCHED_FIFO priority 2
Low thread SCHED_OTHER nice 0

# ./pip_stress

In both cases, the cfs task entering rt_mutex_setprio() is queued, so dequeue_task_fair()->dequeue_entity(), which subtracts cfs_rq->min_vruntime from se->vruntime, is called on it before it gets the rt prio.

Quoted 2 lines

Maybe it requires a very specific use of the pthread library to provoke this issue by making sure that the cfs tasks really blocks/sleeps?

Maybe one could play with rt-app to recreate such specific use case?

https://github.com/scheduler-tools/rt-app/blob/master/doc/tutorial.txt#L459

Hi Juri,

Quoted 6 lines · Juri Lelli

Quoted 5 lines · Dietmar Eggemann

Hi,

Quoted 2 lines · Miguel de Dios

Quoted 1 line · Steve Muckle

From: John Dias <[email protected]>

[...]

Quoted 26 lines

Quoted 25 lines

I tried to catch this issue on my Arm64 Juno board using pi_test (and a slightly adapted pip_test (usleep_val = 1500 and keep low as cfs)) from rt-tests but wasn't able to do so.

# pi_stress --inversions=1 --duration=1 --groups=1 --sched id=low,policy=cfs

Starting PI Stress Test
Number of thread groups: 1
Duration of test run: 1 seconds
Number of inversions per group: 1
Admin thread SCHED_FIFO priority 4
1 groups of 3 threads will be created
High thread SCHED_FIFO priority 3
Med thread SCHED_FIFO priority 2
Low thread SCHED_OTHER nice 0

# ./pip_stress

In both cases, the cfs task entering rt_mutex_setprio() is queued, so dequeue_task_fair()->dequeue_entity(), which subtracts cfs_rq->min_vruntime from se->vruntime, is called on it before it gets the rt prio.

Maybe it requires a very specific use of the pthread library to provoke this issue by making sure that the cfs tasks really blocks/sleeps?

Quoted 3 lines

Maybe one could play with rt-app to recreate such specific use case?

https://github.com/scheduler-tools/rt-app/blob/master/doc/tutorial.txt#L459

I played a little bit with rt-app on hikey960 to re-create Steve's test program.

Since there is no semaphore support (sem_wait(), sem_post()) I used condition variables (wait: pthread_cond_wait() , signal:
pthread_cond_signal()). It's not really the same since this is stateless but sleeps before the signals help to maintain the state in this easy example.

This provokes the vruntime issue e.g. for cpus 0,4 and it doesn't for 0,1:

    "global": {
        "calibration" : 130,
	"pi_enabled" : true
    },
    "tasks": {
        "rt_task": {
	    "loop" : 100,
	    "policy" : "SCHED_FIFO",
	    "cpus" : [0],

	    "lock" : "b_mutex",
	    "wait" : { "ref" : "b_cond", "mutex" : "b_mutex" },
	    "unlock" : "b_mutex",
	    "sleep" : 3000,
	    "lock1" : "a_mutex",
	    "signal" : "a_cond",
	    "unlock1" : "a_mutex",
	    "lock2" : "pi-mutex",
	    "unlock2" : "pi-mutex"
        },
	"cfs_task": {
	    "loop" : 100,
	    "policy" : "SCHED_OTHER",
	    "cpus" : [4],

	    "lock" : "pi-mutex",
	    "sleep" : 3000,
	    "lock1" : "b_mutex",
	    "signal" : "b_cond",
	    "unlock" : "b_mutex",
	    "lock2" : "a_mutex",
	    "wait" : { "ref" : "a_cond", "mutex" : "a_mutex" },
	    "unlock1" : "a_mutex",
	    "unlock2" : "pi-mutex"
	}
    }
}

Adding semaphores is possible but rt-app has no easy way to initialize individual objects, e.g. sem_init(..., value). The only way I see is via the global section, like "pi_enabled". But then, this is true for all objects of this kind (in this case mutexes)?

So the following couple of lines extension to rt-app works because both semaphores can be initialized to 0:

 {
    "global": {
        "calibration" : 130,
	"pi_enabled" : true
    },
    "tasks": {
        "rt_task": {
	    "loop" : 100,
	    "policy" : "SCHED_FIFO",
	    "cpus" : [0],

	    "sem_wait" : "b_sem",
	    "sleep" : 1000,
	    "sem_post" : "a_sem",

	    "lock" : "pi-mutex",
	    "unlock" : "pi-mutex"
        },
	"cfs_task": {
	    "loop" : 100,
	    "policy" : "SCHED_OTHER",
	    "cpus" : [4],

	    "lock" : "pi-mutex",
	    "sleep" : 1000,
	    "sem_post" : "b_sem",
	    "sem_wait" : "a_sem",
	    "unlock" : "pi-mutex"
	}
    }
}

Any thoughts on that? I can see something like this as infrastructure to create a regression test case based on rt-app and standard ftrace.

Quoted 38 lines · Dietmar Eggemann

Hi Juri,

Quoted 6 lines · Juri Lelli

Quoted 5 lines · Dietmar Eggemann

Hi,

Quoted 2 lines · Miguel de Dios

Quoted 1 line · Steve Muckle

From: John Dias <[email protected]>

[...]

Quoted 26 lines

Quoted 25 lines

I tried to catch this issue on my Arm64 Juno board using pi_test (and a slightly adapted pip_test (usleep_val = 1500 and keep low as cfs)) from rt-tests but wasn't able to do so.

# pi_stress --inversions=1 --duration=1 --groups=1 --sched id=low,policy=cfs

Starting PI Stress Test
Number of thread groups: 1
Duration of test run: 1 seconds
Number of inversions per group: 1
Admin thread SCHED_FIFO priority 4
1 groups of 3 threads will be created
High thread SCHED_FIFO priority 3
Med thread SCHED_FIFO priority 2
Low thread SCHED_OTHER nice 0

# ./pip_stress

In both cases, the cfs task entering rt_mutex_setprio() is queued, so dequeue_task_fair()->dequeue_entity(), which subtracts cfs_rq->min_vruntime from se->vruntime, is called on it before it gets the rt prio.

Maybe it requires a very specific use of the pthread library to provoke this issue by making sure that the cfs tasks really blocks/sleeps?

Quoted 6 lines

Quoted 3 lines

Maybe one could play with rt-app to recreate such specific use case?

https://github.com/scheduler-tools/rt-app/blob/master/doc/tutorial.txt#L459

I played a little bit with rt-app on hikey960 to re-create Steve's test program.

Oh, nice! Thanks for sharing what you have got.

Quoted 46 lines

Since there is no semaphore support (sem_wait(), sem_post()) I used condition variables (wait: pthread_cond_wait() , signal:
pthread_cond_signal()). It's not really the same since this is stateless but sleeps before the signals help to maintain the state in this easy example.

This provokes the vruntime issue e.g. for cpus 0,4 and it doesn't for 0,1:

    "global": {
        "calibration" : 130,
      "pi_enabled" : true
    },
    "tasks": {
        "rt_task": {
          "loop" : 100,
          "policy" : "SCHED_FIFO",
          "cpus" : [0],

          "lock" : "b_mutex",
          "wait" : { "ref" : "b_cond", "mutex" : "b_mutex" },
          "unlock" : "b_mutex",
          "sleep" : 3000,
          "lock1" : "a_mutex",
          "signal" : "a_cond",
          "unlock1" : "a_mutex",
          "lock2" : "pi-mutex",
          "unlock2" : "pi-mutex"
        },
      "cfs_task": {
          "loop" : 100,
          "policy" : "SCHED_OTHER",
          "cpus" : [4],

          "lock" : "pi-mutex",
          "sleep" : 3000,
          "lock1" : "b_mutex",
          "signal" : "b_cond",
          "unlock" : "b_mutex",
          "lock2" : "a_mutex",
          "wait" : { "ref" : "a_cond", "mutex" : "a_mutex" },
          "unlock1" : "a_mutex",
          "unlock2" : "pi-mutex"
      }
    }
}

Quoted 4 lines

Adding semaphores is possible but rt-app has no easy way to initialize individual objects, e.g. sem_init(..., value). The only way I see is via the global section, like "pi_enabled". But then, this is true for all objects of this kind (in this case mutexes)?

Right, global section should work fine. Why do you think this is a problem/limitation?

Quoted 36 lines

So the following couple of lines extension to rt-app works because both semaphores can be initialized to 0:

 {
    "global": {
        "calibration" : 130,
      "pi_enabled" : true
    },
    "tasks": {
        "rt_task": {
          "loop" : 100,
          "policy" : "SCHED_FIFO",
          "cpus" : [0],

          "sem_wait" : "b_sem",
          "sleep" : 1000,
          "sem_post" : "a_sem",

          "lock" : "pi-mutex",
          "unlock" : "pi-mutex"
        },
      "cfs_task": {
          "loop" : 100,
          "policy" : "SCHED_OTHER",
          "cpus" : [4],

          "lock" : "pi-mutex",
          "sleep" : 1000,
          "sem_post" : "b_sem",
          "sem_wait" : "a_sem",
          "unlock" : "pi-mutex"
      }
    }
}

Quoted 2 lines

Any thoughts on that? I can see something like this as infrastructure to create a regression test case based on rt-app and standard ftrace.

Agree. I guess we should add your first example to the repo (you'd be very welcome to create a PR) already and then work to support the second?

Quoted 11 lines · Juri Lelli

Quoted 9 lines · Dietmar Eggemann

Hi Juri,

Quoted 6 lines · Juri Lelli

Quoted 5 lines · Dietmar Eggemann

Hi,

Quoted 2 lines · Miguel de Dios

Quoted 1 line · Steve Muckle

From: John Dias <[email protected]>

Quoted 5 lines

Quoted 4 lines

Adding semaphores is possible but rt-app has no easy way to initialize individual objects, e.g. sem_init(..., value). The only way I see is via the global section, like "pi_enabled". But then, this is true for all objects of this kind (in this case mutexes)?

Quoted 2 lines

Right, global section should work fine. Why do you think this is a problem/limitation?

keep in mind that rt-app still have "ressources" section. This one is optional and almost never used as resources can be created on the fly but it's still there and can be used to initialize resources if needed like semaphore

Quoted 39 lines · Steve Muckle

From: John Dias <[email protected]>

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.
Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.
The immediate result is inflation of the task's vruntime, giving it lower priority (starving it if there's enough available work). The longer-term effect is inflation of all vruntimes because the task's vruntime becomes the rq's min_vruntime when the higher priority tasks go idle. That leads to a vicious cycle, where the vruntime inflation repeatedly doubled.

The change here is to detect when vruntime_normalized is being called when the task is waking but is waking in another class, and to conclude that this is a case where vruntime has not been normalized.

Signed-off-by John Dias <[email protected]>

Signed-off-by Steve Muckle <[email protected]>

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

kernel/sched/fair.c +0 -0

index b39fb596f6c1..14011d7929d8 100644

kernel/sched/fair.c static inline bool vruntime_normalized(struct task_struct *p) · line 9644

Quoted 8 lines

kernel/sched/fair.c +2 -1

@@ -9638,7 +9638,8 @@ static inline bool vruntime_normalized(struct task_struct *p)
  	 * - A task which has been woken up by try_to_wake_up() and
  	 *   waiting for actually being woken up by sched_ttwu_pending().
  	 */
-	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+	if (!se->sum_exec_runtime ||
+	    (p->state == TASK_WAKING && p->sched_class == &fair_sched_class))
  		return true;
  	return false;

The normalization of vruntime used to exist in task_waking but it was removed and the normalization was moved into migrate_task_rq_fair. The reasoning being that task_waking_fair was only hit when a task is queued onto a different core and migrate_task_rq_fair should do the same work.

However, we're finding that there's one case which migrate_task_rq_fair doesn't hit: that being the case where rt_mutex_setprio changes a task's scheduling class to RT when its scheduled out. The task never hits migrate_task_rq_fair because it is switched to RT and migrates as an RT task. Because of this we're getting an unbounded addition of min_vruntime when the task is re-attached to the CFS runqueue when it loses the inherited priority. The patch above works because now the kernel specifically checks for this case and normalizes accordingly.

Here's the patch I was talking about:
https://lore.kernel.org/patchwork/patch/677689/. In our testing we were seeing vruntimes nearly double every time after rt_mutex_setprio boosts the task to RT.

Quoted 5 lines · Steve Muckle

Quoted 4 lines

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.

Quoted 11 lines

Quoted 11 lines

Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 6 lines

Quoted 5 lines · Steve Muckle

Quoted 4 lines

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.

Quoted 11 lines

Quoted 11 lines

Quoted 11 lines

Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

I'm thinking that is an incomplete scenario; where do we get to TASK_WAKING.

Quoted 18 lines · Peter Zijlstra

Quoted 17 lines

Quoted 17 lines · Steve Muckle

Quoted 16 lines

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.

Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 2 lines

I'm thinking that is an incomplete scenario; where do we get to TASK_WAKING.

Yes there's a missing bit of context here at the beginning that the task to be boosted had already been put into TASK_WAKING.

Quoted 19 lines · Steve Muckle

Quoted 18 lines · Peter Zijlstra

Quoted 17 lines

Quoted 17 lines · Steve Muckle

Quoted 16 lines

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.

Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back
to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 5 lines

Quoted 2 lines

I'm thinking that is an incomplete scenario; where do we get to TASK_WAKING.

Yes there's a missing bit of context here at the beginning that the task to be boosted had already been put into TASK_WAKING.

See, I'm confused...

The only time TASK_WAKING is visible, is if we've done a remote wakeup and it's 'stuck' on the remote wake_list. And in that case we've done migrate_task_rq_fair() on it.

So by the time either rt_mutex_setprio() or __sched_setscheduler() get to calling check_class_changed(), under both pi_lock and rq->lock, the vruntime_normalized() thing should be right.

So please detail the exact scenario. Because I'm not seeing it.

Quoted 36 lines · Peter Zijlstra

Quoted 19 lines · Steve Muckle

Quoted 18 lines · Peter Zijlstra

Quoted 17 lines

Quoted 17 lines · Steve Muckle

Quoted 16 lines

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.

Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime
- later, when the prio is deboosted and the task is moved back
to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 5 lines

Quoted 2 lines

I'm thinking that is an incomplete scenario; where do we get to TASK_WAKING.

Yes there's a missing bit of context here at the beginning that the task to be boosted had already been put into TASK_WAKING.

See, I'm confused...

The only time TASK_WAKING is visible, is if we've done a remote wakeup and it's 'stuck' on the remote wake_list. And in that case we've done migrate_task_rq_fair() on it.

So by the time either rt_mutex_setprio() or __sched_setscheduler() get to calling check_class_changed(), under both pi_lock and rq->lock, the vruntime_normalized() thing should be right.

Quoted 1 line

So please detail the exact scenario. Because I'm not seeing it.

Using Steve's test program (https://lkml.org/lkml/2018/8/24/686) I see the issue but only if the two tasks (rt_task, fair_task) run on 2 cpus which don't share LLC (e.g. CPU0 and CPU4 on hikey960).

So the wakeup goes the TTWU_QUEUE && !share_cache (ttwu_queue_remote) path.

I forgot to mention that since fair_task's cpu affinity is restricted to CPU4, there is no call to set_task_cpu()->migrate_task_rq_fair() since if (task_cpu(p) != cpu) fails.

I think the combination of cpu affinity of the fair_task to CPU4 and the fact that the scheduler runs on CPU1 when waking fair_task (with the two cpus not sharing LLC) while TTWU_QUEUE is enabled is the situation in which this vruntime issue can happen.

Quoted 8 lines · Dietmar Eggemann

I forgot to mention that since fair_task's cpu affinity is restricted to CPU4, there is no call to set_task_cpu()->migrate_task_rq_fair() since if (task_cpu(p) != cpu) fails.

I think the combination of cpu affinity of the fair_task to CPU4 and the fact that the scheduler runs on CPU1 when waking fair_task (with the two cpus not sharing LLC) while TTWU_QUEUE is enabled is the situation in which this vruntime issue can happen.

Ohhh, D'0h. A remote wakeup that doesn't migrate.

That would suggest something like so:

kernel/sched/fair.c +0 -0

index b39fb596f6c1..b3b62cf37fb6 100644

Yes, this solves the issue for the case I described. Using 'p->sched_remote_wakeup' (WF_MIGRATED) looks more elegant than using 'p->sched_class == &fair_sched_class'.

Quoted 16 lines · Peter Zijlstra

Quoted 15 lines · Steve Muckle

Quoted 14 lines · Peter Zijlstra

Quoted 14 lines

Quoted 14 lines · Steve Muckle

Quoted 13 lines

When rt_mutex_setprio changes a task's scheduling class to RT, we're seeing cases where the task's vruntime is not updated correctly upon return to the fair class.

Specifically, the following is being observed:
- task is deactivated while still in the fair class
- task is boosted to RT via rt_mutex_setprio, which changes
the task to RT and calls check_class_changed.
- check_class_changed leads to detach_task_cfs_rq, at which point the vruntime_normalized check sees that the task's state is TASK_WAKING, which results in skipping the subtraction of the rq's min_vruntime from the task's vruntime

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 13 lines

Quoted 6 lines

Quoted 3 lines

I'm thinking that is an incomplete scenario; where do we get to TASK_WAKING.

Yes there's a missing bit of context here at the beginning that the task to be boosted had already been put into TASK_WAKING.

See, I'm confused...

The only time TASK_WAKING is visible, is if we've done a remote wakeup and it's 'stuck' on the remote wake_list. And in that case we've done migrate_task_rq_fair() on it.

Quoted 5 lines

So by the time either rt_mutex_setprio() or __sched_setscheduler() get to calling check_class_changed(), under both pi_lock and rq->lock, the vruntime_normalized() thing should be right.

So please detail the exact scenario. Because I'm not seeing it.

Using Steve's test program (https://lkml.org/lkml/2018/8/24/686) I see the issue but only if the two tasks (rt_task, fair_task) run on 2 cpus which don't share LLC (e.g. CPU0 and CPU4 on hikey960).

So the wakeup goes the TTWU_QUEUE && !share_cache (ttwu_queue_remote) path.

...

rt_task-3579  [000] 35.391573: sched_waking:     comm=fair_task pid=3580 prio=120 target_cpu=004
rt_task-3579  [000] 35.391580: bprint:           try_to_wake_up: try_to_wake_up: task=fair_task pid=3580 task_cpu(p)=4 p->on_rq=0
rt_task-3579  [000] 35.391584: bprint:           try_to_wake_up: ttwu_queue: task=fair_task pid=3580
rt_task-3579  [000] 35.391588: bprint:           try_to_wake_up: ttwu_queue_remote: task=fair_task pid=3580
rt_task-3579  [000] 35.391591: bprint:           try_to_wake_up: ttwu_queue_remote: cpu=4 smp_send_reschedule
rt_task-3579  [000] 35.391627: sched_pi_setprio: comm=fair_task pid=3580 oldprio=120 newprio=19
rt_task-3579  [000] 35.391635: bprint:           rt_mutex_setprio: task=fair_task pid=3580 prio=120->19 queued=0 running=0 state=0x200 vruntime=46922871 cpu=4 cfs_rq->min_vruntime=7807420844
rt_task-3579  [000] 35.391641: bprint:           rt_mutex_setprio: p->prio set: task=fair_task pid=3580 prio=19 queued=0 running=0 state=0x200 vruntime=46922871
rt_task-3579  [000] 35.391646: bprint:           rt_mutex_setprio: queued checked: task=fair_task pid=3580 prio=19 queued=0 running=0 state=0x200 vruntime=46922871
rt_task-3579  [000] 35.391652: bprint:           rt_mutex_setprio: running checked: task=fair_task pid=3580 prio=19 queued=0 running=0 state=0x200 vruntime=46922871
rt_task-3579  [000] 35.391657: bprint:           rt_mutex_setprio: fair_class=0xffff000008da2c80 rt_class=0xffff000008da2d70 prev_class=0xffff000008da2c80 p->sched_class=0xffff000008da2d70 oldprio=120 p->prio=19
rt_task-3579  [000] 35.391661: bprint:           detach_task_cfs_rq: task=fair_task pid=3580 cpu=4 vruntime_normalized=1
rt_task-3579  [000] 35.391706: sched_switch:     rt_task:3579 [19] D ==> swapper/0:0 [120]
 <idle>-0     [004] 35.391828: bprint:           ttwu_do_activate: ttwu_do_activate: task=fair_task pid=3580
 <idle>-0     [004] 35.391832: bprint:           ttwu_do_activate: ttwu_activate: task=fair_task pid=3580
 <idle>-0     [004] 35.391833: bprint:           ttwu_do_wakeup: ttwu_do_wakeup: task=fair_task pid=3580
 <idle>-0     [004] 35.391834: sched_wakeup:     fair_task:3580 [19] success=1 CPU:004

It doesn't happen on hikey960 when I use two cpus of the same LLC or on my laptop (i7-4750HQ).

Hi Dietmar,

Quoted 5 lines · Dietmar Eggemann

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Steve Muckle

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Quoted 1 line · Steve Muckle

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? attach_task_cfs_rq will not do that the same reason as detach_task_cfs_rq. fair task's sched_remote_wakeup is false which results in vruntime will not be renormalized in enqueue_entity.

Hi,

Quoted 7 lines · Wanpeng Li

Hi Dietmar,

Quoted 5 lines · Dietmar Eggemann

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Steve Muckle

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Quoted 1 line · Steve Muckle

[...]

Quoted 4 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 5 lines

Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? attach_task_cfs_rq will not do that the same reason as detach_task_cfs_rq. fair task's sched_remote_wakeup is false which results in vruntime will not be renormalized in enqueue_entity.

The cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair().

Quoted 24 lines · Dietmar Eggemann

Hi,

Quoted 7 lines · Wanpeng Li

Hi Dietmar,

Quoted 5 lines · Dietmar Eggemann

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Steve Muckle

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Quoted 1 line · Steve Muckle

[...]

Quoted 9 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? attach_task_cfs_rq will not do that the same reason as detach_task_cfs_rq. fair task's sched_remote_wakeup is false which results in vruntime will not be renormalized in enqueue_entity.

Quoted 2 lines

The cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair().

I understand what your patch done,

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

       -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
            -> ttwu_activate()
                 -> activate_task()
                      -> enqueue_task()
                           -> enqueue_task_fair()
                                -> enqueue_entity()
                                     bool renorm = !(flags &

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Quoted 25 lines · Wanpeng Li

Quoted 24 lines · Dietmar Eggemann

Hi,

Quoted 7 lines · Wanpeng Li

Hi Dietmar,

Quoted 5 lines · Dietmar Eggemann

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Steve Muckle

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Quoted 1 line · Steve Muckle

[...]

Quoted 9 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? attach_task_cfs_rq will not do that the same reason as detach_task_cfs_rq. fair task's sched_remote_wakeup is false which results in vruntime will not be renormalized in enqueue_entity.

Quoted 4 lines

Quoted 2 lines

The cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair().

I understand what your patch done,

It's not my patch ;-) I just helped to find out under which circumstances this issue can happen.

Quoted 11 lines

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

       -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
            -> ttwu_activate()
                 -> activate_task()
                      -> enqueue_task()
                           -> enqueue_task_fair()
                                -> enqueue_entity()
                                     bool renorm = !(flags &

Quoted 4 lines

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Maybe this is a misunderstanding on my side but didn't you asked me to '... Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? ...'

Quoted 16 lines

Quoted 15 lines

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

        -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
             -> ttwu_activate()
                  -> activate_task()
                       -> enqueue_task()
                            -> enqueue_task_fair()
                                 -> enqueue_entity()
                                      bool renorm = !(flags &

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Quoted 3 lines

Maybe this is a misunderstanding on my side but didn't you asked me to '... Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? ...'

Yeah, if the calltrace above and my analysis is correct, then the fair rq's min_vruntime will not be added to the task's vruntime in your *later* scenario, which means that your patch is not necessary.

Quoted 18 lines · Wanpeng Li

Quoted 18 lines

Quoted 16 lines

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

        -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
             -> ttwu_activate()
                  -> activate_task()
                       -> enqueue_task()
                            -> enqueue_task_fair()
                                 -> enqueue_entity()
                                      bool renorm = !(flags &

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Quoted 8 lines

Quoted 3 lines

Maybe this is a misunderstanding on my side but didn't you asked me to '... Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? ...'

Yeah, if the calltrace above and my analysis is correct, then the fair rq's min_vruntime will not be added to the task's vruntime in your *later* scenario, which means that your patch is not necessary.

In the scenario I observed, the task is not waking - it is running and being deboosted from priority inheritance, transitioning from RT to CFS.

Dietmar and I both were able to reproduce the issue with the testcase I posted earlier in this thread.

Quoted 20 lines · Wanpeng Li

Quoted 19 lines

Quoted 7 lines

On your CPU4:
scheduler_ipi()
   -> sched_ttwu_pending()
        -> ttwu_do_activate()    => p->sched_remote_wakeup should be false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not -> ttwu_activate()
                  -> activate_task()

Quoted 8 lines

                       -> enqueue_task()
                            -> enqueue_task_fair()
                                 -> enqueue_entity() bool renorm = !(flags & ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Maybe this is a misunderstanding on my side but didn't you asked me to '... Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? ...'

Quoted 3 lines

Yeah, if the calltrace above and my analysis is correct, then the fair rq's min_vruntime will not be added to the task's vruntime in your *later* scenario, which means that your patch is not necessary.

In the scenario I observed, the task is not waking - it is running and being deboosted from priority inheritance, transitioning from RT to CFS.

Dietmar and I both were able to reproduce the issue with the testcase I posted earlier in this thread.

Quoted 25 lines · Steve Muckle

Quoted 16 lines · Wanpeng Li

Quoted 16 lines

Quoted 15 lines

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

        -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
             -> ttwu_activate()
                  -> activate_task()
                       -> enqueue_task()
                            -> enqueue_task_fair()
                                 -> enqueue_entity()
                                      bool renorm = !(flags &

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Quoted 7 lines

Quoted 3 lines

Maybe this is a misunderstanding on my side but didn't you asked me to '... Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? ...'

Yeah, if the calltrace above and my analysis is correct, then the fair rq's min_vruntime will not be added to the task's vruntime in your *later* scenario, which means that your patch is not necessary.

Quoted 5 lines

In the scenario I observed, the task is not waking - it is running and being deboosted from priority inheritance, transitioning from RT to CFS.

Dietmar and I both were able to reproduce the issue with the testcase I posted earlier in this thread.

Correct, and with the same testcase I got this call stack in this scenario:

[   35.588509] CPU: 1 PID: 2926 Comm: fair_task Not tainted
4.18.0-rc6-00052-g11b7dafa2edb-dirty #5
[   35.597217] Hardware name: ARM Juno development board (r0) (DT)
[   35.603080] Call trace:
[   35.605509]  dump_backtrace+0x0/0x168
[   35.609138]  show_stack+0x24/0x30
[   35.612424]  dump_stack+0xac/0xe4
[   35.615710]  enqueue_task_fair+0xae0/0x11c0
[   35.619854]  rt_mutex_setprio+0x5a0/0x628
[   35.623827]  mark_wakeup_next_waiter+0x7c/0xc8
[   35.628228]  __rt_mutex_futex_unlock+0x30/0x50
[   35.632630]  do_futex+0x74c/0xb28
[   35.635912]  sys_futex+0x118/0x198
[   35.639280]  el0_svc_naked+0x30/0x34

Quoted 51 lines · Dietmar Eggemann

Quoted 25 lines · Wanpeng Li

Quoted 24 lines · Dietmar Eggemann

Hi,

Quoted 7 lines · Wanpeng Li

Hi Dietmar,

Quoted 5 lines · Dietmar Eggemann

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Steve Muckle

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Quoted 1 line · Steve Muckle

[...]

Quoted 9 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? attach_task_cfs_rq will not do that the same reason as detach_task_cfs_rq. fair task's sched_remote_wakeup is false which results in vruntime will not be renormalized in enqueue_entity.

Quoted 4 lines

Quoted 2 lines

The cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair().

I understand what your patch done,

It's not my patch ;-) I just helped to find out under which circumstances this issue can happen.

Quoted 15 lines

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

        -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
             -> ttwu_activate()
                  -> activate_task()
                       -> enqueue_task()
                            -> enqueue_task_fair()
                                 -> enqueue_entity()
                                      bool renorm = !(flags &

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?

Quoted 3 lines

Maybe this is a misunderstanding on my side but didn't you asked me to '... Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? ...'

Yeah, if the calltrace above and my analysis is correct, then the fair rq's min_vruntime will not be added to the task's vruntime in your *later* scenario, which means that your patch is not necessary.

Quoted 18 lines · Dietmar Eggemann

Hi,

Quoted 7 lines · Wanpeng Li

Hi Dietmar,

Quoted 5 lines · Dietmar Eggemann

Quoted 3 lines · Peter Zijlstra

Quoted 2 lines · Steve Muckle

Quoted 1 line · Peter Zijlstra

Quoted 1 line

Quoted 1 line · Steve Muckle

[...]

Quoted 4 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

- later, when the prio is deboosted and the task is moved back to the fair class, the fair rq's min_vruntime is added to the task's vruntime, even though it wasn't subtracted earlier.

Quoted 8 lines

Quoted 5 lines

Could you point out when the fair rq's min_vruntime is added to the task's vruntime in your *later* scenario? attach_task_cfs_rq will not do that the same reason as detach_task_cfs_rq. fair task's sched_remote_wakeup is false which results in vruntime will not be renormalized in enqueue_entity.

The cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair().

I understand what your patch done,

On your CPU4:
scheduler_ipi()
-> sched_ttwu_pending()

      -> ttwu_do_activate()    => p->sched_remote_wakeup should be
false, so ENQUEUE_WAKEUP is set, ENQUEUE_MIGRATED is not
           -> ttwu_activate()
                -> activate_task()
                     -> enqueue_task()
                          -> enqueue_task_fair()
                               -> enqueue_entity()
                                    bool renorm = !(flags &

ENQUEUE_WAKEUP) || (flags & ENQUEUE_MIGRATE)
so renorm is false in enqueue_entity(), why you mentioned that the cfs_rq->min_vruntime is still added to the se->vruntime in enqueue_task_fair()?