[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250414115509.GB3558904@bytedance>
Date: Mon, 14 Apr 2025 19:55:09 +0800
From: Aaron Lu <ziqianlu@...edance.com>
To: K Prateek Nayak <kprateek.nayak@....com>
Cc: Valentin Schneider <vschneid@...hat.com>,
Ben Segall <bsegall@...gle.com>,
Peter Zijlstra <peterz@...radead.org>,
Josh Don <joshdon@...gle.com>, Ingo Molnar <mingo@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
Xi Wang <xii@...gle.com>, linux-kernel@...r.kernel.org,
Juri Lelli <juri.lelli@...hat.com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Steven Rostedt <rostedt@...dmis.org>, Mel Gorman <mgorman@...e.de>,
Chengming Zhou <chengming.zhou@...ux.dev>,
Chuyi Zhou <zhouchuyi@...edance.com>,
Jan Kiszka <jan.kiszka@...mens.com>
Subject: Re: [RFC PATCH v2 1/7] sched/fair: Add related data structure for
task based throttle
On Mon, Apr 14, 2025 at 09:28:36AM +0530, K Prateek Nayak wrote:
> Hello Aaron,
>
> On 4/9/2025 5:37 PM, Aaron Lu wrote:
> > From: Valentin Schneider <vschneid@...hat.com>
> >
> > Add related data structures for this new throttle functionality.
> >
> > Signed-off-by: Valentin Schneider <vschneid@...hat.com>
> > Signed-off-by: Aaron Lu <ziqianlu@...edance.com>
> > ---
> > include/linux/sched.h | 4 ++++
> > kernel/sched/core.c | 3 +++
> > kernel/sched/fair.c | 12 ++++++++++++
> > kernel/sched/sched.h | 2 ++
> > 4 files changed, 21 insertions(+)
> >
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index f96ac19828934..0b55c79fee209 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -880,6 +880,10 @@ struct task_struct {
> > #ifdef CONFIG_CGROUP_SCHED
> > struct task_group *sched_task_group;
> > +#ifdef CONFIG_CFS_BANDWIDTH
> > + struct callback_head sched_throttle_work;
> > + struct list_head throttle_node;
>
> Since throttled tasks are fully dequeued before placing on the
> "throttled_limbo_list", is it possible to reuse "p->se.group_node"?
I think it might be possible.
> Currently, it is used to track the task on "rq->cfs_tasks" and during
> load-balancing when moving a bunch of tasks between CPUs but since a
> fully throttled task is not tracked by either, it should be safe to
> reuse this bit (CONFIG_DEBUG_LIST will scream if I'm wrong) and save
> up on some space in the task_struct.
>
> Thoughts?
Is it that adding throttle_node would cause task_struct to just cross a
cacheline boundary? :-)
Or it's mainly a concern that system could have many tasks and any saving
in task_struct is worth to try?
I can see reusing another field would cause task_is_throttled() more
obscure to digest and implement, but I think it is doable.
Thanks,
Aaron
Powered by blists - more mailing lists