linux-kernel - Re: [PATCH] sched/fair: Make SCHED_IDLE se be preempted in strict hierarchy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 24 Jun 2024 12:37:11 -0700
From: Josh Don <joshdon@...gle.com>
To: Tianchen Ding <dtcccc@...ux.alibaba.com>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...hat.com>, 
	Peter Zijlstra <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>, 
	Vincent Guittot <vincent.guittot@...aro.org>, Dietmar Eggemann <dietmar.eggemann@....com>, 
	Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, 
	Daniel Bristot de Oliveira <bristot@...hat.com>, Valentin Schneider <vschneid@...hat.com>
Subject: Re: [PATCH] sched/fair: Make SCHED_IDLE se be preempted in strict hierarchy

Hi Tianchen,

Thanks for these fixes.

>  #endif /* CONFIG_FAIR_GROUP_SCHED */
> @@ -8382,16 +8382,7 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>         if (test_tsk_need_resched(curr))
>                 return;
>
> -       /* Idle tasks are by definition preempted by non-idle tasks. */
> -       if (unlikely(task_has_idle_policy(curr)) &&
> -           likely(!task_has_idle_policy(p)))
> -               goto preempt;
> -
> -       /*
> -        * Batch and idle tasks do not preempt non-idle tasks (their preemption
> -        * is driven by the tick):
> -        */
> -       if (unlikely(p->policy != SCHED_NORMAL) || !sched_feat(WAKEUP_PREEMPTION))
> +       if (!sched_feat(WAKEUP_PREEMPTION))
>                 return;
>
>         find_matching_se(&se, &pse);
> @@ -8408,6 +8399,12 @@ static void check_preempt_wakeup_fair(struct rq *rq, struct task_struct *p, int
>                 goto preempt;
>         if (cse_is_idle != pse_is_idle)
>                 return;

nit: add newline

> +       /*
> +        * Batch tasks do not preempt non-idle tasks (their preemption
> +        * is driven by the tick):
> +        */
> +       if (unlikely(pse == &p->se && p->policy == SCHED_BATCH))
> +               return;

I think it is worth extending that comment to explain why we don't
also check policy for SCHED_IDLE here (since the cse_is_idle checks
above boil down to checking for task idle policy in this special case
where pse is a task entity). The comment above the pse_is_idle checks
only talks about idle group preemption, when here we're relying on it
for idle task preemption.

>
>         cfs_rq = cfs_rq_of(se);
>         update_curr(cfs_rq);
> --
> 2.39.3
>

Reviewed-by: Josh Don <joshdon@...gle.com>