[PATCH V2] sched,psi: fix the 'int' underflow for psi

[PATCH V2] sched,psi: fix the 'int' underflow for psi

[Charan Teja Reddy]

psi_group_cpu->tasks, represented by the unsigned int, stores the number
of tasks that could be stalled on a psi resource(io/mem/cpu).
Decrementing these counters at zero leads to wrapping which further
leads to the psi_group_cpu->state_mask is being set with the respective
pressure state. This could result into the unnecessary time sampling for
the pressure state thus cause the spurious psi events. This can further
lead to wrong actions being taken at the user land based on these psi
events.
Though psi_bug is set under these conditions but that just for debug
purpose. Fix it by decrementing the ->tasks count only when it is
non-zero.

Signed-off-by: Charan Teja Reddy <charante@codeaurora.org>
---
 
Changes in V1: https://lore.kernel.org/patchwork/patch/1413894/

 kernel/sched/psi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index 967732c..651218d 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -711,14 +711,15 @@ static void psi_group_change(struct psi_group *group, int cpu,
 	for (t = 0, m = clear; m; m &= ~(1 << t), t++) {
 		if (!(m & (1 << t)))
 			continue;
-		if (groupc->tasks[t] == 0 && !psi_bug) {
+		if (groupc->tasks[t]) {
+			groupc->tasks[t]--;
+		} else if (!psi_bug) {
 			printk_deferred(KERN_ERR "psi: task underflow! cpu=%d t=%d tasks=[%u %u %u %u] clear=%x set=%x\n",
 					cpu, t, groupc->tasks[0],
 					groupc->tasks[1], groupc->tasks[2],
 					groupc->tasks[3], clear, set);
 			psi_bug = 1;
 		}
-		groupc->tasks[t]--;
 	}
 
 	for (t = 0; set; set &= ~(1 << t), t++)
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V2] sched,psi: fix the 'int' underflow for psi

[Johannes Weiner]

On Fri, Apr 16, 2021 at 08:32:16PM +0530, Charan Teja Reddy wrote:
> psi_group_cpu->tasks, represented by the unsigned int, stores the number
> of tasks that could be stalled on a psi resource(io/mem/cpu).
> Decrementing these counters at zero leads to wrapping which further
> leads to the psi_group_cpu->state_mask is being set with the respective
> pressure state. This could result into the unnecessary time sampling for
> the pressure state thus cause the spurious psi events. This can further
> lead to wrong actions being taken at the user land based on these psi
> events.
> Though psi_bug is set under these conditions but that just for debug
> purpose. Fix it by decrementing the ->tasks count only when it is
> non-zero.
> 
> Signed-off-by: Charan Teja Reddy <charante@codeaurora.org>

The subject should be

  psi: handle potential task count underflow bugs more gracefully

since it's not fixing a known bug at this point. Otherwise,

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Re: [PATCH V2] sched,psi: fix the 'int' underflow for psi

[Peter Zijlstra]

On Fri, Apr 16, 2021 at 11:11:04AM -0400, Johannes Weiner wrote:
> On Fri, Apr 16, 2021 at 08:32:16PM +0530, Charan Teja Reddy wrote:
> > psi_group_cpu->tasks, represented by the unsigned int, stores the number
> > of tasks that could be stalled on a psi resource(io/mem/cpu).
> > Decrementing these counters at zero leads to wrapping which further
> > leads to the psi_group_cpu->state_mask is being set with the respective
> > pressure state. This could result into the unnecessary time sampling for
> > the pressure state thus cause the spurious psi events. This can further
> > lead to wrong actions being taken at the user land based on these psi
> > events.
> > Though psi_bug is set under these conditions but that just for debug
> > purpose. Fix it by decrementing the ->tasks count only when it is
> > non-zero.
> > 
> > Signed-off-by: Charan Teja Reddy <charante@codeaurora.org>
> 
> The subject should be
> 
>   psi: handle potential task count underflow bugs more gracefully

Done.

> since it's not fixing a known bug at this point. Otherwise,
> 
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>

Thanks!

[tip: sched/core] sched,psi: Handle potential task count underflow bugs more gracefully

[tip-bot2 for Charan Teja Reddy]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     9d10a13d1e4c349b76f1c675a874a7f981d6d3b4
Gitweb:        https://git.kernel.org/tip/9d10a13d1e4c349b76f1c675a874a7f981d6d3b4
Author:        Charan Teja Reddy <charante@codeaurora.org>
AuthorDate:    Fri, 16 Apr 2021 20:32:16 +05:30
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 21 Apr 2021 13:55:41 +02:00

sched,psi: Handle potential task count underflow bugs more gracefully

psi_group_cpu->tasks, represented by the unsigned int, stores the
number of tasks that could be stalled on a psi resource(io/mem/cpu).
Decrementing these counters at zero leads to wrapping which further
leads to the psi_group_cpu->state_mask is being set with the
respective pressure state. This could result into the unnecessary time
sampling for the pressure state thus cause the spurious psi events.
This can further lead to wrong actions being taken at the user land
based on these psi events.

Though psi_bug is set under these conditions but that just for debug
purpose. Fix it by decrementing the ->tasks count only when it is
non-zero.

Signed-off-by: Charan Teja Reddy <charante@codeaurora.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Link: https://lkml.kernel.org/r/1618585336-37219-1-git-send-email-charante@codeaurora.org
---
 kernel/sched/psi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
index d1212f1..db27b69 100644
--- a/kernel/sched/psi.c
+++ b/kernel/sched/psi.c
@@ -699,14 +699,15 @@ static void psi_group_change(struct psi_group *group, int cpu,
 	for (t = 0, m = clear; m; m &= ~(1 << t), t++) {
 		if (!(m & (1 << t)))
 			continue;
-		if (groupc->tasks[t] == 0 && !psi_bug) {
+		if (groupc->tasks[t]) {
+			groupc->tasks[t]--;
+		} else if (!psi_bug) {
 			printk_deferred(KERN_ERR "psi: task underflow! cpu=%d t=%d tasks=[%u %u %u %u] clear=%x set=%x\n",
 					cpu, t, groupc->tasks[0],
 					groupc->tasks[1], groupc->tasks[2],
 					groupc->tasks[3], clear, set);
 			psi_bug = 1;
 		}
-		groupc->tasks[t]--;
 	}
 
 	for (t = 0; set; set &= ~(1 << t), t++)