* [PATCH] sched/vtime: Fix guest/system mis-accounting on task switch
@ 2019-09-25 21:42 Frederic Weisbecker
2019-09-26 8:14 ` Peter Zijlstra
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Frederic Weisbecker @ 2019-09-25 21:42 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar
Cc: LKML, Frederic Weisbecker, Wanpeng Li, Thomas Gleixner, Rik van Riel
vtime_account_system() assumes that the target task to account cputime
to is always the current task. This is most often true indeed except on
task switch where we call:
vtime_common_task_switch(prev)
vtime_account_system(prev)
Here prev is the scheduling-out task where we account the cputime to. It
doesn't match current that is already the scheduling-in task at this
stage of the context switch.
So we end up checking the wrong task flags to determine if we are
accounting guest or system time to the previous task.
As a result the wrong task is used to check if the target is running in
guest mode. We may then spuriously account or leak either system or
guest time on task switch.
Fix this assumption and also turn vtime_guest_enter/exit() to use the
task passed in parameter as well to avoid future similar issues.
Fixes: 2a42eb9594a1 ("sched/cputime: Accumulate vtime on top of nsec clocksource")
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/cputime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 2305ce89a26c..46ed4e1383e2 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -740,7 +740,7 @@ void vtime_account_system(struct task_struct *tsk)
write_seqcount_begin(&vtime->seqcount);
/* We might have scheduled out from guest path */
- if (current->flags & PF_VCPU)
+ if (tsk->flags & PF_VCPU)
vtime_account_guest(tsk, vtime);
else
__vtime_account_system(tsk, vtime);
@@ -783,7 +783,7 @@ void vtime_guest_enter(struct task_struct *tsk)
*/
write_seqcount_begin(&vtime->seqcount);
__vtime_account_system(tsk, vtime);
- current->flags |= PF_VCPU;
+ tsk->flags |= PF_VCPU;
write_seqcount_end(&vtime->seqcount);
}
EXPORT_SYMBOL_GPL(vtime_guest_enter);
@@ -794,7 +794,7 @@ void vtime_guest_exit(struct task_struct *tsk)
write_seqcount_begin(&vtime->seqcount);
vtime_account_guest(tsk, vtime);
- current->flags &= ~PF_VCPU;
+ tsk->flags &= ~PF_VCPU;
write_seqcount_end(&vtime->seqcount);
}
EXPORT_SYMBOL_GPL(vtime_guest_exit);
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] sched/vtime: Fix guest/system mis-accounting on task switch
2019-09-25 21:42 [PATCH] sched/vtime: Fix guest/system mis-accounting on task switch Frederic Weisbecker
@ 2019-09-26 8:14 ` Peter Zijlstra
2019-10-09 12:59 ` [tip: sched/urgent] " tip-bot2 for Frederic Weisbecker
2019-10-09 12:59 ` [tip: sched/core] " tip-bot2 for Frederic Weisbecker
2 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2019-09-26 8:14 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Ingo Molnar, LKML, Wanpeng Li, Thomas Gleixner, Rik van Riel
On Wed, Sep 25, 2019 at 11:42:42PM +0200, Frederic Weisbecker wrote:
> vtime_account_system() assumes that the target task to account cputime
> to is always the current task. This is most often true indeed except on
> task switch where we call:
>
> vtime_common_task_switch(prev)
> vtime_account_system(prev)
>
> Here prev is the scheduling-out task where we account the cputime to. It
> doesn't match current that is already the scheduling-in task at this
> stage of the context switch.
>
> So we end up checking the wrong task flags to determine if we are
> accounting guest or system time to the previous task.
>
> As a result the wrong task is used to check if the target is running in
> guest mode. We may then spuriously account or leak either system or
> guest time on task switch.
>
> Fix this assumption and also turn vtime_guest_enter/exit() to use the
> task passed in parameter as well to avoid future similar issues.
>
> Fixes: 2a42eb9594a1 ("sched/cputime: Accumulate vtime on top of nsec clocksource")
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Cc: Ingo Molnar <mingo@kernel.org>
Thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread* [tip: sched/urgent] sched/vtime: Fix guest/system mis-accounting on task switch
2019-09-25 21:42 [PATCH] sched/vtime: Fix guest/system mis-accounting on task switch Frederic Weisbecker
2019-09-26 8:14 ` Peter Zijlstra
@ 2019-10-09 12:59 ` tip-bot2 for Frederic Weisbecker
2019-10-09 12:59 ` [tip: sched/core] " tip-bot2 for Frederic Weisbecker
2 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Frederic Weisbecker @ 2019-10-09 12:59 UTC (permalink / raw)
To: linux-tip-commits
Cc: Frederic Weisbecker, Peter Zijlstra (Intel),
Linus Torvalds, Rik van Riel, Thomas Gleixner, Wanpeng Li,
Ingo Molnar, Borislav Petkov, linux-kernel
The following commit has been merged into the sched/urgent branch of tip:
Commit-ID: 68e7a4d66b0ce04bf18ff2ffded5596ab3618585
Gitweb: https://git.kernel.org/tip/68e7a4d66b0ce04bf18ff2ffded5596ab3618585
Author: Frederic Weisbecker <frederic@kernel.org>
AuthorDate: Wed, 25 Sep 2019 23:42:42 +02:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 09 Oct 2019 12:38:03 +02:00
sched/vtime: Fix guest/system mis-accounting on task switch
vtime_account_system() assumes that the target task to account cputime
to is always the current task. This is most often true indeed except on
task switch where we call:
vtime_common_task_switch(prev)
vtime_account_system(prev)
Here prev is the scheduling-out task where we account the cputime to. It
doesn't match current that is already the scheduling-in task at this
stage of the context switch.
So we end up checking the wrong task flags to determine if we are
accounting guest or system time to the previous task.
As a result the wrong task is used to check if the target is running in
guest mode. We may then spuriously account or leak either system or
guest time on task switch.
Fix this assumption and also turn vtime_guest_enter/exit() to use the
task passed in parameter as well to avoid future similar issues.
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpengli@tencent.com>
Fixes: 2a42eb9594a1 ("sched/cputime: Accumulate vtime on top of nsec clocksource")
Link: https://lkml.kernel.org/r/20190925214242.21873-1-frederic@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/cputime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 2305ce8..46ed4e1 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -740,7 +740,7 @@ void vtime_account_system(struct task_struct *tsk)
write_seqcount_begin(&vtime->seqcount);
/* We might have scheduled out from guest path */
- if (current->flags & PF_VCPU)
+ if (tsk->flags & PF_VCPU)
vtime_account_guest(tsk, vtime);
else
__vtime_account_system(tsk, vtime);
@@ -783,7 +783,7 @@ void vtime_guest_enter(struct task_struct *tsk)
*/
write_seqcount_begin(&vtime->seqcount);
__vtime_account_system(tsk, vtime);
- current->flags |= PF_VCPU;
+ tsk->flags |= PF_VCPU;
write_seqcount_end(&vtime->seqcount);
}
EXPORT_SYMBOL_GPL(vtime_guest_enter);
@@ -794,7 +794,7 @@ void vtime_guest_exit(struct task_struct *tsk)
write_seqcount_begin(&vtime->seqcount);
vtime_account_guest(tsk, vtime);
- current->flags &= ~PF_VCPU;
+ tsk->flags &= ~PF_VCPU;
write_seqcount_end(&vtime->seqcount);
}
EXPORT_SYMBOL_GPL(vtime_guest_exit);
^ permalink raw reply related [flat|nested] 4+ messages in thread* [tip: sched/core] sched/vtime: Fix guest/system mis-accounting on task switch
2019-09-25 21:42 [PATCH] sched/vtime: Fix guest/system mis-accounting on task switch Frederic Weisbecker
2019-09-26 8:14 ` Peter Zijlstra
2019-10-09 12:59 ` [tip: sched/urgent] " tip-bot2 for Frederic Weisbecker
@ 2019-10-09 12:59 ` tip-bot2 for Frederic Weisbecker
2 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Frederic Weisbecker @ 2019-10-09 12:59 UTC (permalink / raw)
To: linux-tip-commits
Cc: Frederic Weisbecker, Peter Zijlstra (Intel),
Linus Torvalds, Rik van Riel, Thomas Gleixner, Wanpeng Li,
Ingo Molnar, Borislav Petkov, linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 68e7a4d66b0ce04bf18ff2ffded5596ab3618585
Gitweb: https://git.kernel.org/tip/68e7a4d66b0ce04bf18ff2ffded5596ab3618585
Author: Frederic Weisbecker <frederic@kernel.org>
AuthorDate: Wed, 25 Sep 2019 23:42:42 +02:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 09 Oct 2019 12:38:03 +02:00
sched/vtime: Fix guest/system mis-accounting on task switch
vtime_account_system() assumes that the target task to account cputime
to is always the current task. This is most often true indeed except on
task switch where we call:
vtime_common_task_switch(prev)
vtime_account_system(prev)
Here prev is the scheduling-out task where we account the cputime to. It
doesn't match current that is already the scheduling-in task at this
stage of the context switch.
So we end up checking the wrong task flags to determine if we are
accounting guest or system time to the previous task.
As a result the wrong task is used to check if the target is running in
guest mode. We may then spuriously account or leak either system or
guest time on task switch.
Fix this assumption and also turn vtime_guest_enter/exit() to use the
task passed in parameter as well to avoid future similar issues.
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Wanpeng Li <wanpengli@tencent.com>
Fixes: 2a42eb9594a1 ("sched/cputime: Accumulate vtime on top of nsec clocksource")
Link: https://lkml.kernel.org/r/20190925214242.21873-1-frederic@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/cputime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 2305ce8..46ed4e1 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -740,7 +740,7 @@ void vtime_account_system(struct task_struct *tsk)
write_seqcount_begin(&vtime->seqcount);
/* We might have scheduled out from guest path */
- if (current->flags & PF_VCPU)
+ if (tsk->flags & PF_VCPU)
vtime_account_guest(tsk, vtime);
else
__vtime_account_system(tsk, vtime);
@@ -783,7 +783,7 @@ void vtime_guest_enter(struct task_struct *tsk)
*/
write_seqcount_begin(&vtime->seqcount);
__vtime_account_system(tsk, vtime);
- current->flags |= PF_VCPU;
+ tsk->flags |= PF_VCPU;
write_seqcount_end(&vtime->seqcount);
}
EXPORT_SYMBOL_GPL(vtime_guest_enter);
@@ -794,7 +794,7 @@ void vtime_guest_exit(struct task_struct *tsk)
write_seqcount_begin(&vtime->seqcount);
vtime_account_guest(tsk, vtime);
- current->flags &= ~PF_VCPU;
+ tsk->flags &= ~PF_VCPU;
write_seqcount_end(&vtime->seqcount);
}
EXPORT_SYMBOL_GPL(vtime_guest_exit);
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-09 13:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 21:42 [PATCH] sched/vtime: Fix guest/system mis-accounting on task switch Frederic Weisbecker
2019-09-26 8:14 ` Peter Zijlstra
2019-10-09 12:59 ` [tip: sched/urgent] " tip-bot2 for Frederic Weisbecker
2019-10-09 12:59 ` [tip: sched/core] " tip-bot2 for Frederic Weisbecker
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.