linux-kernel - Re: [PATCH 1/2] sched/fair: Record the average duration of a task

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]

Message-ID: <d922f7bf3965f4eaef5028177b886e2e1861742d.camel@gmx.de>
Date: Wed, 26 Jun 2024 06:21:43 +0200
From: Mike Galbraith <efault@....de>
To: Chen Yu <yu.c.chen@...el.com>, Peter Zijlstra <peterz@...radead.org>, 
 Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>, Vincent
 Guittot <vincent.guittot@...aro.org>
Cc: Tim Chen <tim.c.chen@...el.com>, Yujie Liu <yujie.liu@...el.com>, K
 Prateek Nayak <kprateek.nayak@....com>, "Gautham R . Shenoy"
 <gautham.shenoy@....com>, Chen Yu <yu.chen.surf@...il.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] sched/fair: Record the average duration of a task

On Tue, 2024-06-25 at 15:22 +0800, Chen Yu wrote:
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 0935f9d4bb7b..7399c4143528 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4359,6 +4359,8 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p)
>         p->migration_pending = NULL;
>  #endif
>         init_sched_mm_cid(p);
> +       p->prev_sleep_sum_runtime = 0;
> +       p->duration_avg = 0;
>  }

Beginning life biased toward stacking?

>  DEFINE_STATIC_KEY_FALSE(sched_numa_balancing);
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 41b58387023d..445877069fbf 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
>
> @@ -6905,6 +6914,9 @@ static void dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  
>  dequeue_throttle:
>         util_est_update(&rq->cfs, p, task_sleep);
> +       if (task_sleep)
> +               dur_avg_update(p);
> +
>         hrtick_update(rq);
>  }
>

That qualifier looks a bit dangerous.  Microbench components tend to
have only one behavior, but the real world goes through all kinds of
nutty gyrations, intentional and otherwise.

The heuristics in the next patch seem to exhibit a healthy level of
paranoia, but these bits could perhaps use a tad more.  Bad experiences
springs to mind when I stare at that - sleepers going hog, hogs meet
sleeping lock contention, preemption, sync hint not meaning much...

	-Mike