From mboxrd@z Thu Jan 1 00:00:00 1970 From: Waiman Long Subject: Re: [PATCH v2 3/6] sched/cpuset: Keep track of SCHED_DEADLINE task in cpusets Date: Wed, 3 May 2023 13:56:09 -0400 Message-ID: <09fc4561-5ea9-8b5f-a784-37f578f152af@redhat.com> References: <20230503072228.115707-1-juri.lelli@redhat.com> <20230503072228.115707-4-juri.lelli@redhat.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1683136575; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=AQpEwS4JBjiBUmzDrp0hg9pwaawUbOZgQLeijVoHEYM=; b=JS3Jx/cp/pUCfR5irtpd1jY15FTzZaTxwc1DrZ/O1Vm5IdGNEI+4YcsKUlSmcLvDDTYSy0 hZiVVR/oZIuPEKISXmvr3i0+wzogdRuOBrMxZUep/6EalIVzUOuOETheM+TjHdQfJnSyGd JwTnWXD2DIBzbV6fmHnHsAmMFDrNWno= Content-Language: en-US In-Reply-To: <20230503072228.115707-4-juri.lelli-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Juri Lelli , Peter Zijlstra , Ingo Molnar , Qais Yousef , Tejun Heo , Zefan Li , Johannes Weiner , Hao Luo Cc: Dietmar Eggemann , Steven Rostedt , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, luca.abeni-5rdYK369eBLQB0XuIGIEkQ@public.gmane.org, claudio-YOzL5CV4y4YG1A2ADO40+w@public.gmane.org, tommaso.cucinotta-5rdYK369eBLQB0XuIGIEkQ@public.gmane.org, bristot-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Vincent Guittot , Wei Wang , Rick Yiu , Quentin Perret , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Sudeep Holla On 5/3/23 03:22, Juri Lelli wrote: > Qais reported that iterating over all tasks when rebuilding root domains > for finding out which ones are DEADLINE and need their bandwidth > correctly restored on such root domains can be a costly operation (10+ > ms delays on suspend-resume). > > To fix the problem keep track of the number of DEADLINE tasks belonging > to each cpuset and then use this information (followup patch) to only > perform the above iteration if DEADLINE tasks are actually present in > the cpuset for which a corresponding root domain is being rebuilt. > > Reported-by: Qais Yousef > Link: https://lore.kernel.org/lkml/20230206221428.2125324-1-qyousef-wp2msK0BRk8tq7phqP6ubQ@public.gmane.org/ > Signed-off-by: Juri Lelli > --- > include/linux/cpuset.h | 4 ++++ > kernel/cgroup/cgroup.c | 4 ++++ > kernel/cgroup/cpuset.c | 25 +++++++++++++++++++++++++ > kernel/sched/deadline.c | 14 ++++++++++++++ > 4 files changed, 47 insertions(+) > > diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h > index f90e6325d707..d629094fac6e 100644 > --- a/include/linux/cpuset.h > +++ b/include/linux/cpuset.h > @@ -71,6 +71,8 @@ extern void cpuset_init_smp(void); > extern void cpuset_force_rebuild(void); > extern void cpuset_update_active_cpus(void); > extern void cpuset_wait_for_hotplug(void); > +extern void inc_dl_tasks_cs(struct task_struct *task); > +extern void dec_dl_tasks_cs(struct task_struct *task); > extern void cpuset_lock(void); > extern void cpuset_unlock(void); > extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask); > @@ -189,6 +191,8 @@ static inline void cpuset_update_active_cpus(void) > > static inline void cpuset_wait_for_hotplug(void) { } > > +static inline void inc_dl_tasks_cs(struct task_struct *task) { } > +static inline void dec_dl_tasks_cs(struct task_struct *task) { } > static inline void cpuset_lock(void) { } > static inline void cpuset_unlock(void) { } > > diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c > index 625d7483951c..9d809191a54f 100644 > --- a/kernel/cgroup/cgroup.c > +++ b/kernel/cgroup/cgroup.c > @@ -57,6 +57,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -6683,6 +6684,9 @@ void cgroup_exit(struct task_struct *tsk) > list_add_tail(&tsk->cg_list, &cset->dying_tasks); > cset->nr_tasks--; > > + if (dl_task(tsk)) > + dec_dl_tasks_cs(tsk); > + > WARN_ON_ONCE(cgroup_task_frozen(tsk)); > if (unlikely(!(tsk->flags & PF_KTHREAD) && > test_bit(CGRP_FREEZE, &task_dfl_cgroup(tsk)->flags))) > diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c > index ee66be215fb9..b9f4d5602517 100644 > --- a/kernel/cgroup/cpuset.c > +++ b/kernel/cgroup/cpuset.c > @@ -193,6 +193,12 @@ struct cpuset { > int use_parent_ecpus; > int child_ecpus_count; > > + /* > + * number of SCHED_DEADLINE tasks attached to this cpuset, so that we > + * know when to rebuild associated root domain bandwidth information. > + */ > + int nr_deadline_tasks; > + > /* Invalid partition error code, not lock protected */ > enum prs_errcode prs_err; > > @@ -245,6 +251,20 @@ static inline struct cpuset *parent_cs(struct cpuset *cs) > return css_cs(cs->css.parent); > } > > +void inc_dl_tasks_cs(struct task_struct *p) > +{ > + struct cpuset *cs = task_cs(p); > + > + cs->nr_deadline_tasks++; > +} > + > +void dec_dl_tasks_cs(struct task_struct *p) > +{ > + struct cpuset *cs = task_cs(p); > + > + cs->nr_deadline_tasks--; > +} > + > /* bits in struct cpuset flags field */ > typedef enum { > CS_ONLINE, > @@ -2497,6 +2517,11 @@ static int cpuset_can_attach(struct cgroup_taskset *tset) > ret = security_task_setscheduler(task); > if (ret) > goto out_unlock; > + > + if (dl_task(task)) { > + cs->nr_deadline_tasks++; > + cpuset_attach_old_cs->nr_deadline_tasks--; > + } > } > > /* > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index 5a9a4b81c972..e11de074a6fd 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -16,6 +16,8 @@ > * Fabio Checconi > */ > > +#include > + > /* > * Default limits for DL period; on the top end we guard against small util > * tasks still getting ridiculously long effective runtimes, on the bottom end we > @@ -2596,6 +2598,12 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p) > if (task_on_rq_queued(p) && p->dl.dl_runtime) > task_non_contending(p); > > + /* > + * In case a task is setscheduled out from SCHED_DEADLINE we need to > + * keep track of that on its cpuset (for correct bandwidth tracking). > + */ > + dec_dl_tasks_cs(p); > + > if (!task_on_rq_queued(p)) { > /* > * Inactive timer is armed. However, p is leaving DEADLINE and > @@ -2636,6 +2644,12 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p) > if (hrtimer_try_to_cancel(&p->dl.inactive_timer) == 1) > put_task_struct(p); > > + /* > + * In case a task is setscheduled to SCHED_DEADLINE we need to keep > + * track of that on its cpuset (for correct bandwidth tracking). > + */ > + inc_dl_tasks_cs(p); > + > /* If p is not queued we will update its parameters at next wakeup. */ > if (!task_on_rq_queued(p)) { > add_rq_bw(&p->dl, &rq->dl); Reviewed-by: Waiman Long