Received-SPF: pass (google.com: domain of linux-kernel+bounces-201658-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
Date: Wed, 05 Jun 2024 01:51:56 +0000
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
From: "Yajun Deng" <yajun.deng@linux.dev>
Message-ID: <e72e7b6cca0ee56b39e9f28fc66441f3366ba7a1@linux.dev>
TLS-Required: No
Subject: Re: [PATCH 1/2] sched/rt: Use 'back' only if CONFIG_RT_GROUP_SCHED
 is enabled
To: "Steven Rostedt" <rostedt@goodmis.org>
Cc: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
 vincent.guittot@linaro.org, dietmar.eggemann@arm.com, bsegall@google.com,
 mgorman@suse.de, bristot@redhat.com, vschneid@redhat.com,
 linux-kernel@vger.kernel.org
In-Reply-To: <20240604092203.7c31b7b5@gandalf.local.home>
References: <20240603125002.3086-1-yajun.deng@linux.dev>
 <20240603125002.3086-2-yajun.deng@linux.dev>
 <20240604092203.7c31b7b5@gandalf.local.home>

June 4, 2024 at 9:22 PM, "Steven Rostedt" <rostedt@goodmis.org> wrote:


>=20
>=20On Mon, 3 Jun 2024 20:50:01 +0800
>=20
>=20Yajun Deng <yajun.deng@linux.dev> wrote:
>=20
>=20>=20
>=20> The 'back' member in struct sched_rt_entity only related to
> >=20
>=20>  CONFIG_RT_GROUP_SCHED, so there is no need to init it in dequeue_r=
t_stack.
> >=20
>=20>=20=20
>=20>=20
>=20>  Init the 'back' member in init_tg_rt_entry and use it only if
> >=20
>=20>  CONFIG_RT_GROUP_SCHED is enabled.
> >=20
>=20>=20=20
>=20>=20
>=20>  Signed-off-by: Yajun Deng <yajun.deng@linux.dev>
> >=20
>=20>  ---
> >=20
>=20>  kernel/sched/rt.c | 15 ++++++++++-----
> >=20
>=20>  1 file changed, 10 insertions(+), 5 deletions(-)
> >=20
>=20>=20=20
>=20>=20
>=20>  diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> >=20
>=20>  index 63e49c8ffc4d..44bb9087cfe5 100644
> >=20
>=20>  --- a/kernel/sched/rt.c
> >=20
>=20>  +++ b/kernel/sched/rt.c
> >=20
>=20>  @@ -232,8 +232,10 @@ void init_tg_rt_entry(struct task_group *tg, =
struct rt_rq *rt_rq,
> >=20
>=20>=20=20
>=20>=20
>=20>  if (!parent)
> >=20
>=20>  rt_se->rt_rq =3D &rq->rt;
> >=20
>=20>  - else
> >=20
>=20>  + else {
> >=20
>=20>  rt_se->rt_rq =3D parent->my_q;
> >=20
>=20>  + parent->back =3D rt_se;
> >=20
>=20>  + }
> >=20
>=20
> If one block of an if/else requires brackets, then both do.
>=20

Got=20it.
> >=20
>=20> rt_se->my_q =3D rt_rq;
> >=20
>=20>  rt_se->parent =3D parent;
> >=20
>=20>  @@ -1428,16 +1430,19 @@ static void dequeue_rt_stack(struct sched_=
rt_entity *rt_se, unsigned int flags)
> >=20
>=20>  struct sched_rt_entity *back =3D NULL;
> >=20
>=20>  unsigned int rt_nr_running;
> >=20
>=20>=20=20
>=20>=20
>=20>  - for_each_sched_rt_entity(rt_se) {
> >=20
>=20>  - rt_se->back =3D back;
> >=20
>=20>  + for_each_sched_rt_entity(rt_se)
> >=20
>=20>  back =3D rt_se;
> >=20
>=20>  - }
> >=20
>=20>=20=20
>=20>=20
>=20>  rt_nr_running =3D rt_rq_of_se(back)->rt_nr_running;
> >=20
>=20>=20=20
>=20>=20
>=20>  - for (rt_se =3D back; rt_se; rt_se =3D rt_se->back) {
> >=20
>=20>  + for (rt_se =3D back; rt_se; ) {
> >=20
>=20>  if (on_rt_rq(rt_se))
> >=20
>=20>  __dequeue_rt_entity(rt_se, flags);
> >=20
>=20>  +#ifdef CONFIG_RT_GROUP_SCHED
> >=20
>=20>  + rt_se =3D rt_se->back;
> >=20
>=20>  +#else
> >=20
>=20>  + rt_se =3D NULL;
> >=20
>=20>  +#endif
> >=20
>=20
> Perhaps make a helper function to get rid of the #ifdef in the code her=
e.
>=20
>=20 for (rt_se =3D back, rt_se; rt_se =3D rt_se_back(rt_se)) {
>=20
>=20Where rt_se_back() above can be defined in the #ifdef blocks for
>=20
>=20CONFIG_RT_GROUP_SCHED as either:
>=20
>=20static struct sched_rt_entity *rt_se_back(struct sched_rt_entity *se)
>=20
>=20{
>=20
>=20 return se->back;
>=20
>=20}
>=20
>=20Or
>=20
>=20static inline struct sched_rt_entity *rt_se_back(struct sched_rt_enti=
ty *se)
>=20
>=20{
>=20
>=20 return NULL;
>=20
>=20}
>=20

Okay,=20thanks.

> -- Steve
>=20
>=20>=20
>=20> }
> >=20
>=20>=20=20
>=20>=20
>=20>  dequeue_top_rt_rq(rt_rq_of_se(back), rt_nr_running);
> >
>