2023-10-05 16:42:35

by Yajun Deng

[permalink] [raw]
Subject: [PATCH 0/2] Move sched_rt_entity::back to 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.

The 1st patch is introduce for_each_sched_rt_entity_back() & use it.
it no functional changes.
The 2nd patch is move sched_rt_entity::back to under the
CONFIG_RT_GROUP_SCHED block, it will save a few bytes.

Yajun Deng (2):
sched/rt: Introduce for_each_sched_rt_entity_back() & use it
sched/rt: Move sched_rt_entity::back to under the
CONFIG_RT_GROUP_SCHED block

include/linux/sched.h | 2 +-
kernel/sched/rt.c | 25 ++++++++++++++++---------
2 files changed, 17 insertions(+), 10 deletions(-)

--
2.25.1


2023-10-05 16:42:37

by Yajun Deng

[permalink] [raw]
Subject: [PATCH 2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block

The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
So move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block. It
will save a few bytes.

Also, init child when parent isn't NULL in init_tg_rt_entry().

Signed-off-by: Yajun Deng <[email protected]>
---
include/linux/sched.h | 2 +-
kernel/sched/rt.c | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 292c31697248..d0fe56603e60 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -597,8 +597,8 @@ struct sched_rt_entity {
unsigned short on_rq;
unsigned short on_list;

- struct sched_rt_entity *back;
#ifdef CONFIG_RT_GROUP_SCHED
+ struct sched_rt_entity *back;
struct sched_rt_entity *parent;
/* rq on which this entity is (to be) queued: */
struct rt_rq *rt_rq;
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 642edbd24ffb..7b3105b875f1 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -233,8 +233,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;
@@ -1441,23 +1443,21 @@ static void __dequeue_rt_entity(struct sched_rt_entity *rt_se, unsigned int flag
*/
static void dequeue_rt_stack(struct sched_rt_entity *rt_se, unsigned int flags)
{
- struct sched_rt_entity *back = NULL;
+ struct sched_rt_entity *root = NULL;
unsigned int rt_nr_running;

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

- rt_nr_running = rt_rq_of_se(back)->rt_nr_running;
+ rt_nr_running = rt_rq_of_se(root)->rt_nr_running;

- rt_se = back;
+ rt_se = root;
for_each_sched_rt_entity_back(rt_se) {
if (on_rt_rq(rt_se))
__dequeue_rt_entity(rt_se, flags);
}

- dequeue_top_rt_rq(rt_rq_of_se(back), rt_nr_running);
+ dequeue_top_rt_rq(rt_rq_of_se(root), rt_nr_running);
}

static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int flags)
--
2.25.1

2023-10-05 16:42:39

by Yajun Deng

[permalink] [raw]
Subject: [PATCH 1/2] sched/rt: Introduce for_each_sched_rt_entity_back() & use it

The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
it should not place out of RT_GROUP_SCHED.

Introduce for_each_sched_rt_entity_back() & use it, it safe move back to
under the CONFIG_RT_GROUP_SCHED in next patch.

No functional changes.

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

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 88fc98601413..642edbd24ffb 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -520,6 +520,9 @@ static inline struct task_group *next_task_group(struct task_group *tg)
#define for_each_sched_rt_entity(rt_se) \
for (; rt_se; rt_se = rt_se->parent)

+#define for_each_sched_rt_entity_back(rt_se) \
+ for (; rt_se; rt_se = rt_se->back)
+
static inline struct rt_rq *group_rt_rq(struct sched_rt_entity *rt_se)
{
return rt_se->my_q;
@@ -625,6 +628,9 @@ typedef struct rt_rq *rt_rq_iter_t;
#define for_each_sched_rt_entity(rt_se) \
for (; rt_se; rt_se = NULL)

+#define for_each_sched_rt_entity_back(rt_se) \
+ for_each_sched_rt_entity(rt_se)
+
static inline struct rt_rq *group_rt_rq(struct sched_rt_entity *rt_se)
{
return NULL;
@@ -1445,7 +1451,8 @@ static void dequeue_rt_stack(struct sched_rt_entity *rt_se, unsigned int flags)

rt_nr_running = rt_rq_of_se(back)->rt_nr_running;

- for (rt_se = back; rt_se; rt_se = rt_se->back) {
+ rt_se = back;
+ for_each_sched_rt_entity_back(rt_se) {
if (on_rt_rq(rt_se))
__dequeue_rt_entity(rt_se, flags);
}
--
2.25.1

2023-10-09 10:16:51

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block


* Yajun Deng <[email protected]> wrote:

> The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
> So move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block. It
> will save a few bytes.
>
> Also, init child when parent isn't NULL in init_tg_rt_entry().
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
> include/linux/sched.h | 2 +-
> kernel/sched/rt.c | 18 +++++++++---------
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 292c31697248..d0fe56603e60 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -597,8 +597,8 @@ struct sched_rt_entity {
> unsigned short on_rq;
> unsigned short on_list;
>
> - struct sched_rt_entity *back;
> #ifdef CONFIG_RT_GROUP_SCHED
> + struct sched_rt_entity *back;
> struct sched_rt_entity *parent;
> /* rq on which this entity is (to be) queued: */
> struct rt_rq *rt_rq;

Title claims this change - the rest of the changes should be in a separate
patch:

> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 642edbd24ffb..7b3105b875f1 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -233,8 +233,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;
> @@ -1441,23 +1443,21 @@ static void __dequeue_rt_entity(struct sched_rt_entity *rt_se, unsigned int flag
> */
> static void dequeue_rt_stack(struct sched_rt_entity *rt_se, unsigned int flags)
> {
> - struct sched_rt_entity *back = NULL;
> + struct sched_rt_entity *root = NULL;
> unsigned int rt_nr_running;
>
> - for_each_sched_rt_entity(rt_se) {
> - rt_se->back = back;
> - back = rt_se;
> - }
> + for_each_sched_rt_entity(rt_se)
> + root = rt_se;
>
> - rt_nr_running = rt_rq_of_se(back)->rt_nr_running;
> + rt_nr_running = rt_rq_of_se(root)->rt_nr_running;
>
> - rt_se = back;
> + rt_se = root;
> for_each_sched_rt_entity_back(rt_se) {
> if (on_rt_rq(rt_se))
> __dequeue_rt_entity(rt_se, flags);
> }
>
> - dequeue_top_rt_rq(rt_rq_of_se(back), rt_nr_running);
> + dequeue_top_rt_rq(rt_rq_of_se(root), rt_nr_running);
> }
>
> static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int flags)
> --
> 2.25.1

Thanks,

Ingo

2023-10-09 11:13:45

by Yajun Deng

[permalink] [raw]
Subject: Re: [PATCH 2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block


On 2023/10/9 18:16, Ingo Molnar wrote:
> * Yajun Deng <[email protected]> wrote:
>
>> The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
>> So move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block. It
>> will save a few bytes.
>>
>> Also, init child when parent isn't NULL in init_tg_rt_entry().
>>
>> Signed-off-by: Yajun Deng <[email protected]>
>> ---
>> include/linux/sched.h | 2 +-
>> kernel/sched/rt.c | 18 +++++++++---------
>> 2 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 292c31697248..d0fe56603e60 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -597,8 +597,8 @@ struct sched_rt_entity {
>> unsigned short on_rq;
>> unsigned short on_list;
>>
>> - struct sched_rt_entity *back;
>> #ifdef CONFIG_RT_GROUP_SCHED
>> + struct sched_rt_entity *back;
>> struct sched_rt_entity *parent;
>> /* rq on which this entity is (to be) queued: */
>> struct rt_rq *rt_rq;
> Title claims this change - the rest of the changes should be in a separate
> patch:


Okay. I will send v2.

>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index 642edbd24ffb..7b3105b875f1 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -233,8 +233,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;
>> @@ -1441,23 +1443,21 @@ static void __dequeue_rt_entity(struct sched_rt_entity *rt_se, unsigned int flag
>> */
>> static void dequeue_rt_stack(struct sched_rt_entity *rt_se, unsigned int flags)
>> {
>> - struct sched_rt_entity *back = NULL;
>> + struct sched_rt_entity *root = NULL;
>> unsigned int rt_nr_running;
>>
>> - for_each_sched_rt_entity(rt_se) {
>> - rt_se->back = back;
>> - back = rt_se;
>> - }
>> + for_each_sched_rt_entity(rt_se)
>> + root = rt_se;
>>
>> - rt_nr_running = rt_rq_of_se(back)->rt_nr_running;
>> + rt_nr_running = rt_rq_of_se(root)->rt_nr_running;
>>
>> - rt_se = back;
>> + rt_se = root;
>> for_each_sched_rt_entity_back(rt_se) {
>> if (on_rt_rq(rt_se))
>> __dequeue_rt_entity(rt_se, flags);
>> }
>>
>> - dequeue_top_rt_rq(rt_rq_of_se(back), rt_nr_running);
>> + dequeue_top_rt_rq(rt_rq_of_se(root), rt_nr_running);
>> }
>>
>> static void enqueue_rt_entity(struct sched_rt_entity *rt_se, unsigned int flags)
>> --
>> 2.25.1
> Thanks,
>
> Ingo

2023-10-09 11:28:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block


* Yajun Deng <[email protected]> wrote:

>
> On 2023/10/9 18:16, Ingo Molnar wrote:
> > * Yajun Deng <[email protected]> wrote:
> >
> > > The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
> > > So move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block. It
> > > will save a few bytes.
> > >
> > > Also, init child when parent isn't NULL in init_tg_rt_entry().
> > >
> > > Signed-off-by: Yajun Deng <[email protected]>
> > > ---
> > > include/linux/sched.h | 2 +-
> > > kernel/sched/rt.c | 18 +++++++++---------
> > > 2 files changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > index 292c31697248..d0fe56603e60 100644
> > > --- a/include/linux/sched.h
> > > +++ b/include/linux/sched.h
> > > @@ -597,8 +597,8 @@ struct sched_rt_entity {
> > > unsigned short on_rq;
> > > unsigned short on_list;
> > > - struct sched_rt_entity *back;
> > > #ifdef CONFIG_RT_GROUP_SCHED
> > > + struct sched_rt_entity *back;
> > > struct sched_rt_entity *parent;
> > > /* rq on which this entity is (to be) queued: */
> > > struct rt_rq *rt_rq;
> > Title claims this change - the rest of the changes should be in a separate
> > patch:
>
>
> Okay. I will send v2.

It's ~v7 already by my count, isn't it?

Thanks,

Ingo

2023-10-09 11:32:15

by Yajun Deng

[permalink] [raw]
Subject: Re: [PATCH 2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block


On 2023/10/9 19:26, Ingo Molnar wrote:
> * Yajun Deng <[email protected]> wrote:
>
>> On 2023/10/9 18:16, Ingo Molnar wrote:
>>> * Yajun Deng <[email protected]> wrote:
>>>
>>>> The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
>>>> So move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block. It
>>>> will save a few bytes.
>>>>
>>>> Also, init child when parent isn't NULL in init_tg_rt_entry().
>>>>
>>>> Signed-off-by: Yajun Deng <[email protected]>
>>>> ---
>>>> include/linux/sched.h | 2 +-
>>>> kernel/sched/rt.c | 18 +++++++++---------
>>>> 2 files changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>>>> index 292c31697248..d0fe56603e60 100644
>>>> --- a/include/linux/sched.h
>>>> +++ b/include/linux/sched.h
>>>> @@ -597,8 +597,8 @@ struct sched_rt_entity {
>>>> unsigned short on_rq;
>>>> unsigned short on_list;
>>>> - struct sched_rt_entity *back;
>>>> #ifdef CONFIG_RT_GROUP_SCHED
>>>> + struct sched_rt_entity *back;
>>>> struct sched_rt_entity *parent;
>>>> /* rq on which this entity is (to be) queued: */
>>>> struct rt_rq *rt_rq;
>>> Title claims this change - the rest of the changes should be in a separate
>>> patch:
>>
>> Okay. I will send v2.
> It's ~v7 already by my count, isn't it?


May be. If we count from the earliest.

>
> Thanks,
>
> Ingo

2023-10-09 11:54:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block


* Yajun Deng <[email protected]> wrote:

>
> On 2023/10/9 19:26, Ingo Molnar wrote:
> > * Yajun Deng <[email protected]> wrote:
> >
> > > On 2023/10/9 18:16, Ingo Molnar wrote:
> > > > * Yajun Deng <[email protected]> wrote:
> > > >
> > > > > The member back in struct sched_rt_entity only related to RT_GROUP_SCHED,
> > > > > So move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block. It
> > > > > will save a few bytes.
> > > > >
> > > > > Also, init child when parent isn't NULL in init_tg_rt_entry().
> > > > >
> > > > > Signed-off-by: Yajun Deng <[email protected]>
> > > > > ---
> > > > > include/linux/sched.h | 2 +-
> > > > > kernel/sched/rt.c | 18 +++++++++---------
> > > > > 2 files changed, 10 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > > index 292c31697248..d0fe56603e60 100644
> > > > > --- a/include/linux/sched.h
> > > > > +++ b/include/linux/sched.h
> > > > > @@ -597,8 +597,8 @@ struct sched_rt_entity {
> > > > > unsigned short on_rq;
> > > > > unsigned short on_list;
> > > > > - struct sched_rt_entity *back;
> > > > > #ifdef CONFIG_RT_GROUP_SCHED
> > > > > + struct sched_rt_entity *back;
> > > > > struct sched_rt_entity *parent;
> > > > > /* rq on which this entity is (to be) queued: */
> > > > > struct rt_rq *rt_rq;
> > > > Title claims this change - the rest of the changes should be in a separate
> > > > patch:
> > >
> > > Okay. I will send v2.
> > It's ~v7 already by my count, isn't it?
>
>
> May be. If we count from the earliest.

Yes, of course we count from the earliest this series was sent, why
wouldn't we? Having new patches or removing patches doesn't really reset
the counter.

Thanks,

Ingo