linux-kernel - Re: [PATCH 07/24] sched/fair: Re-organize dequeue_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: <xhsmhed6uxdmo.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Mon, 12 Aug 2024 12:02:39 +0200
From: Valentin Schneider <vschneid@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...hat.com, juri.lelli@...hat.com, vincent.guittot@...aro.org,
 dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com,
 mgorman@...e.de, linux-kernel@...r.kernel.org, kprateek.nayak@....com,
 wuyun.abel@...edance.com, youssefesmat@...omium.org, tglx@...utronix.de,
 efault@....de
Subject: Re: [PATCH 07/24] sched/fair: Re-organize dequeue_task_fair()

On 11/08/24 00:17, Peter Zijlstra wrote:
> On Fri, Aug 09, 2024 at 06:53:30PM +0200, Valentin Schneider wrote:
>> On 27/07/24 12:27, Peter Zijlstra wrote:
>> > -static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>> > +static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
>> >  {
>> > -	struct cfs_rq *cfs_rq;
>> > -	struct sched_entity *se = &p->se;
>> > -	int task_sleep = flags & DEQUEUE_SLEEP;
>> > -	int idle_h_nr_running = task_has_idle_policy(p);
>> >       bool was_sched_idle = sched_idle_rq(rq);
>> >       int rq_h_nr_running = rq->cfs.h_nr_running;
>> > +	bool task_sleep = flags & DEQUEUE_SLEEP;
>> > +	struct task_struct *p = NULL;
>> > +	int idle_h_nr_running = 0;
>> > +	int h_nr_running = 0;
>> > +	struct cfs_rq *cfs_rq;
>> >
>> > -	util_est_dequeue(&rq->cfs, p);
>> > +	if (entity_is_task(se)) {
>> > +		p = task_of(se);
>> > +		h_nr_running = 1;
>> > +		idle_h_nr_running = task_has_idle_policy(p);
>> > +	}
>> >
>>
>> This leaves the *h_nr_running to 0 for non-task entities. IIUC this makes
>> sense for ->sched_delayed entities (they should be empty of tasks), not so
>> sure for the other case. However, this only ends up being used for non-task
>> entities in:
>> - pick_next_entity(), if se->sched_delayed
>> - unregister_fair_sched_group()
>>
>> IIRC unregister_fair_sched_group() can only happen after the group has been
>> drained, so it would then indeed be empty of tasks, but I reckon this could
>> do with a comment/assert in dequeue_entities(), no? Or did I get too
>> confused by cgroups again?
>>
>
> Yeah, so I did have me a patch that made all this work for cfs bandwidth
> control as well. And then we need all this for throttled cgroup entries
> as well.
>
> Anyway... I had the patch, it worked, but then I remembered you were
> going to rewrite all that anyway and I was making a terrible mess of
> things, so I made it go away again.

Heh, sounds like someone needs to get back to it then :-)