When starting a deadline inactive_timer, the task_struct refs will
be incremented only if dl_server is not set. But when canceling the
inactive_timer, the task refs will be decremented whether dl_server is
set or not, leading to a task reference imbalance issue.

This patch fixes the imbalanced reference by adding a '!dl_server()'
checker before calling put_task_struct().

Fixes: 63ba8422f876 ("sched/deadline: Introduce deadline servers")
Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com>
---
 kernel/sched/deadline.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
index f59e5c19d944..40aafc27f02b 100644
--- a/kernel/sched/deadline.c
+++ b/kernel/sched/deadline.c
@@ -320,6 +320,15 @@ void sub_running_bw(struct sched_dl_entity *dl_se, struct dl_rq *dl_rq)
 		__sub_running_bw(dl_se->dl_bw, dl_rq);
 }
 
+static inline
+void dl_cancel_inactive_timer(struct task_struct *p)
+{
+	struct sched_dl_entity *dl_se = &p->dl;
+
+	if (hrtimer_try_to_cancel(&dl_se->inactive_timer) == 1 && !dl_server(dl_se))
+		put_task_struct(p);
+}
+
 static void dl_change_utilization(struct task_struct *p, u64 new_bw)
 {
 	struct rq *rq;
@@ -340,8 +349,7 @@ static void dl_change_utilization(struct task_struct *p, u64 new_bw)
 		 * will not touch the rq's active utilization,
 		 * so we are still safe.
 		 */
-		if (hrtimer_try_to_cancel(&p->dl.inactive_timer) == 1)
-			put_task_struct(p);
+		dl_cancel_inactive_timer(p);
 	}
 	__sub_rq_bw(p->dl.dl_bw, &rq->dl);
 	__add_rq_bw(new_bw, &rq->dl);
@@ -490,10 +498,7 @@ static void task_contending(struct sched_dl_entity *dl_se, int flags)
 		 * will not touch the rq's active utilization,
 		 * so we are still safe.
 		 */
-		if (hrtimer_try_to_cancel(&dl_se->inactive_timer) == 1) {
-			if (!dl_server(dl_se))
-				put_task_struct(dl_task_of(dl_se));
-		}
+		dl_cancel_inactive_timer(dl_task_of(dl_se));
 	} else {
 		/*
 		 * Since "dl_non_contending" is not set, the
@@ -1975,8 +1980,7 @@ static void migrate_task_rq_dl(struct task_struct *p, int new_cpu __maybe_unused
 		 * will not touch the rq's active utilization,
 		 * so we are still safe.
 		 */
-		if (hrtimer_try_to_cancel(&p->dl.inactive_timer) == 1)
-			put_task_struct(p);
+		dl_cancel_inactive_timer(p);
 	}
 	sub_rq_bw(&p->dl, &rq->dl);
 	rq_unlock(rq, &rf);
@@ -2731,8 +2735,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p)
  */
 static void switched_to_dl(struct rq *rq, struct task_struct *p)
 {
-	if (hrtimer_try_to_cancel(&p->dl.inactive_timer) == 1)
-		put_task_struct(p);
+	dl_cancel_inactive_timer(p);
 
 	/*
 	 * In case a task is setscheduled to SCHED_DEADLINE we need to keep
-- 
2.33.0

Hi,

On 07/08/24 16:30, Zhang Qiao wrote:
> When starting a deadline inactive_timer, the task_struct refs will
> be incremented only if dl_server is not set. But when canceling the
> inactive_timer, the task refs will be decremented whether dl_server is
> set or not, leading to a task reference imbalance issue.
> 
> This patch fixes the imbalanced reference by adding a '!dl_server()'
> checker before calling put_task_struct().
> 
> Fixes: 63ba8422f876 ("sched/deadline: Introduce deadline servers")
> Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com>

Isn't this equivalent to Wander's patch below?

https://lore.kernel.org/lkml/20240724142253.27145-3-wander@redhat.com/

Thanks,
Juri

Hi,
在 2024/8/7 17:00, Juri Lelli 写道:
> Hi,
> 
> On 07/08/24 16:30, Zhang Qiao wrote:
>> When starting a deadline inactive_timer, the task_struct refs will
>> be incremented only if dl_server is not set. But when canceling the
>> inactive_timer, the task refs will be decremented whether dl_server is
>> set or not, leading to a task reference imbalance issue.
>>
>> This patch fixes the imbalanced reference by adding a '!dl_server()'
>> checker before calling put_task_struct().
>>
>> Fixes: 63ba8422f876 ("sched/deadline: Introduce deadline servers")
>> Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com>
> 
> Isn't this equivalent to Wander's patch below?
> 

Yeah, i didn't notice it.

> https://lore.kernel.org/lkml/20240724142253.27145-3-wander@redhat.com/
> 



Thanks,
Zhang Qiao.
> Thanks,
> Juri
> 
> 
> 

On 07/08/24 17:39, Zhang Qiao wrote:
> 
> 
> Hi,
> 在 2024/8/7 17:00, Juri Lelli 写道:
> > Hi,
> > 
> > On 07/08/24 16:30, Zhang Qiao wrote:
> >> When starting a deadline inactive_timer, the task_struct refs will
> >> be incremented only if dl_server is not set. But when canceling the
> >> inactive_timer, the task refs will be decremented whether dl_server is
> >> set or not, leading to a task reference imbalance issue.
> >>
> >> This patch fixes the imbalanced reference by adding a '!dl_server()'
> >> checker before calling put_task_struct().
> >>
> >> Fixes: 63ba8422f876 ("sched/deadline: Introduce deadline servers")
> >> Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com>
> > 
> > Isn't this equivalent to Wander's patch below?
> > 
> 
> Yeah, i didn't notice it.

Thanks for confirming!