linux-kernel - Re: [PATCH] sched/rt: Fix double enqueue caused by rt_effective

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:   Tue, 6 Jul 2021 16:48:56 +0200
From:   Dietmar Eggemann <dietmar.eggemann@....com>
To:     Juri Lelli <juri.lelli@...hat.com>, peterz@...radead.org,
        mingo@...hat.com
Cc:     linux-kernel@...r.kernel.org, vincent.guittot@...aro.org,
        rostedt@...dmis.org, bristot@...hat.com, bsegall@...gle.com,
        mgorman@...e.de, Mark Simmons <msimmons@...hat.com>
Subject: Re: [PATCH] sched/rt: Fix double enqueue caused by rt_effective_prio

On 01/07/2021 11:14, Juri Lelli wrote:
> Double enqueues in rt runqueues (list) have been reported while running
> a simple test that spawns a number of threads doing a short sleep/run
> pattern while being concurrently setscheduled between rt and fair class.

I tried to recreate this in rt-app (with `pi-mutex` resource and
`pi_enabled=true` but I can't bring the system into hitting this warning.

[...]

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 0c22cd026440..c84ac1d675f4 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -6823,7 +6823,8 @@ static void __setscheduler_params(struct task_struct *p,
>  
>  /* Actually do priority change: must hold pi & rq lock. */
>  static void __setscheduler(struct rq *rq, struct task_struct *p,
> -			   const struct sched_attr *attr, bool keep_boost)
> +			   const struct sched_attr *attr, bool keep_boost,
> +			   int new_effective_prio)
>  {
>  	/*
>  	 * If params can't change scheduling class changes aren't allowed
> @@ -6840,7 +6841,7 @@ static void __setscheduler(struct rq *rq, struct task_struct *p,
>  	 */
>  	p->prio = normal_prio(p);
>  	if (keep_boost)
> -		p->prio = rt_effective_prio(p, p->prio);
> +		p->prio = new_effective_prio;

So in case __sched_setscheduler() is called for p (SCHED_NORMAL, NICE0)
you want to avoid that this 2. rt_effective_prio() call returns
p->prio=120 in case the 1. call (in __sched_setscheduler()) did return 0
(due to pi_task->prio=0 (FIFO rt_priority=99 task))?

>  
>  	if (dl_prio(p->prio))
>  		p->sched_class = &dl_sched_class;
> @@ -6873,7 +6874,7 @@ static int __sched_setscheduler(struct task_struct *p,
>  	int newprio = dl_policy(attr->sched_policy) ? MAX_DL_PRIO - 1 :
>  		      MAX_RT_PRIO - 1 - attr->sched_priority;
>  	int retval, oldprio, oldpolicy = -1, queued, running;
> -	int new_effective_prio, policy = attr->sched_policy;
> +	int new_effective_prio = newprio, policy = attr->sched_policy;
>  	const struct sched_class *prev_class;
>  	struct callback_head *head;
>  	struct rq_flags rf;
> @@ -7072,6 +7073,9 @@ static int __sched_setscheduler(struct task_struct *p,
>  	oldprio = p->prio;
>  
>  	if (pi) {
> +		newprio = fair_policy(attr->sched_policy) ?
> +			NICE_TO_PRIO(attr->sched_nice) : newprio;
> +

Why is this necessary? p (SCHED_NORMAL) would get newprio=99 now and
with this it gets [100...120...139] which is still greater than any RT
(0-98)/DL (-1) prio?

[...]