2024-06-03 12:52:31

by Yajun Deng

Subject: [PATCH 0/2] sched/rt: Move sched_rt_entity::back to CONFIG_RT_GROUP_SCHED

The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
it should not place out of RT_GROUP_SCHED, move back to RT_GROUP_SCHED.
It will save a few bytes.

Yajun Deng (2):
sched/rt: Use 'back' only if CONFIG_RT_GROUP_SCHED is enabled
sched/headers: Move sched_rt_entity::back to under the
CONFIG_RT_GROUP_SCHED block

include/linux/sched.h | 2 +-
kernel/sched/rt.c | 15 ++++++++++-----
2 files changed, 11 insertions(+), 6 deletions(-)

--
2.25.1

2024-06-03 12:52:42

by Yajun Deng

Subject: [PATCH 1/2] sched/rt: Use 'back' only if CONFIG_RT_GROUP_SCHED is enabled

The 'back' member in struct sched_rt_entity only related to
CONFIG_RT_GROUP_SCHED, so there is no need to init it in dequeue_rt_stack.

Init the 'back' member in init_tg_rt_entry and use it only if
CONFIG_RT_GROUP_SCHED is enabled.

Signed-off-by: Yajun Deng <[email protected]>
---
kernel/sched/rt.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 63e49c8ffc4d..44bb9087cfe5 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -232,8 +232,10 @@ void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,

if (!parent)
rt_se->rt_rq = &rq->rt;
- else
+ else {
rt_se->rt_rq = parent->my_q;
+ parent->back = rt_se;
+ }

rt_se->my_q = rt_rq;
rt_se->parent = parent;
@@ -1428,16 +1430,19 @@ static void dequeue_rt_stack(struct sched_rt_entity *rt_se, unsigned int flags)
struct sched_rt_entity *back = NULL;
unsigned int rt_nr_running;

- for_each_sched_rt_entity(rt_se) {
- rt_se->back = back;
+ for_each_sched_rt_entity(rt_se)
back = rt_se;
- }

rt_nr_running = rt_rq_of_se(back)->rt_nr_running;

- for (rt_se = back; rt_se; rt_se = rt_se->back) {
+ for (rt_se = back; rt_se; ) {
if (on_rt_rq(rt_se))
__dequeue_rt_entity(rt_se, flags);
+#ifdef CONFIG_RT_GROUP_SCHED
+ rt_se = rt_se->back;
+#else
+ rt_se = NULL;
+#endif
}

dequeue_top_rt_rq(rt_rq_of_se(back), rt_nr_running);
--
2.25.1

2024-06-04 13:22:16

by Steven Rostedt

Subject: Re: [PATCH 1/2] sched/rt: Use 'back' only if CONFIG_RT_GROUP_SCHED is enabled

On Mon, 3 Jun 2024 20:50:01 +0800
Yajun Deng <[email protected]> wrote:

> The 'back' member in struct sched_rt_entity only related to
> CONFIG_RT_GROUP_SCHED, so there is no need to init it in dequeue_rt_stack.
>
> Init the 'back' member in init_tg_rt_entry and use it only if
> CONFIG_RT_GROUP_SCHED is enabled.
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
> kernel/sched/rt.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 63e49c8ffc4d..44bb9087cfe5 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -232,8 +232,10 @@ void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,
>
> if (!parent)
> rt_se->rt_rq = &rq->rt;
> - else
> + else {
> rt_se->rt_rq = parent->my_q;
> + parent->back = rt_se;
> + }

If one block of an if/else requires brackets, then both do.


>
> rt_se->my_q = rt_rq;
> rt_se->parent = parent;
> @@ -1428,16 +1430,19 @@ static void dequeue_rt_stack(struct sched_rt_entity *rt_se, unsigned int flags)
> struct sched_rt_entity *back = NULL;
> unsigned int rt_nr_running;
>
> - for_each_sched_rt_entity(rt_se) {
> - rt_se->back = back;
> + for_each_sched_rt_entity(rt_se)
> back = rt_se;
> - }
>
> rt_nr_running = rt_rq_of_se(back)->rt_nr_running;
>
> - for (rt_se = back; rt_se; rt_se = rt_se->back) {
> + for (rt_se = back; rt_se; ) {
> if (on_rt_rq(rt_se))
> __dequeue_rt_entity(rt_se, flags);
> +#ifdef CONFIG_RT_GROUP_SCHED
> + rt_se = rt_se->back;
> +#else
> + rt_se = NULL;
> +#endif

Perhaps make a helper function to get rid of the #ifdef in the code here.

for (rt_se = back, rt_se; rt_se = rt_se_back(rt_se)) {

Where rt_se_back() above can be defined in the #ifdef blocks for
CONFIG_RT_GROUP_SCHED as either:

static struct sched_rt_entity *rt_se_back(struct sched_rt_entity *se)
{
return se->back;
}

Or

static inline struct sched_rt_entity *rt_se_back(struct sched_rt_entity *se)
{
return NULL;
}

-- Steve


> }
>
> dequeue_top_rt_rq(rt_rq_of_se(back), rt_nr_running);

2024-06-05 01:52:16

by Yajun Deng

Subject: Re: [PATCH 1/2] sched/rt: Use 'back' only if CONFIG_RT_GROUP_SCHED is enabled

June 4, 2024 at 9:22 PM, "Steven Rostedt" <[email protected]> wrote:



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

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

Okay, thanks.

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