diff mbox series

[2/2] sched/rt: Move sched_rt_entity::back to under the CONFIG_RT_GROUP_SCHED block

Message ID 20231005162317.3343678-3-yajun.deng@linux.dev
State New
Headers
Series Move sched_rt_entity::back to RT_GROUP_SCHED |

Commit Message

Yajun Deng Oct. 5, 2023, 4:23 p.m. UTC 
  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 <yajun.deng@linux.dev>
---
 include/linux/sched.h |  2 +-
 kernel/sched/rt.c     | 18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)

Comments

Ingo Molnar Oct. 9, 2023, 10:16 a.m. UTC | #1 
* Yajun Deng <yajun.deng@linux.dev> 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 <yajun.deng@linux.dev>
> ---
>  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
Yajun Deng Oct. 9, 2023, 11:13 a.m. UTC | #2 
On 2023/10/9 18:16, Ingo Molnar wrote:
> * Yajun Deng <yajun.deng@linux.dev> 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 <yajun.deng@linux.dev>
>> ---
>>   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
Ingo Molnar Oct. 9, 2023, 11:26 a.m. UTC | #3 
* Yajun Deng <yajun.deng@linux.dev> wrote:

> 
> On 2023/10/9 18:16, Ingo Molnar wrote:
> > * Yajun Deng <yajun.deng@linux.dev> 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 <yajun.deng@linux.dev>
> > > ---
> > >   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
Yajun Deng Oct. 9, 2023, 11:31 a.m. UTC | #4 
On 2023/10/9 19:26, Ingo Molnar wrote:
> * Yajun Deng <yajun.deng@linux.dev> wrote:
>
>> On 2023/10/9 18:16, Ingo Molnar wrote:
>>> * Yajun Deng <yajun.deng@linux.dev> 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 <yajun.deng@linux.dev>
>>>> ---
>>>>    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
Ingo Molnar Oct. 9, 2023, 11:54 a.m. UTC | #5 
* Yajun Deng <yajun.deng@linux.dev> wrote:

> 
> On 2023/10/9 19:26, Ingo Molnar wrote:
> > * Yajun Deng <yajun.deng@linux.dev> wrote:
> > 
> > > On 2023/10/9 18:16, Ingo Molnar wrote:
> > > > * Yajun Deng <yajun.deng@linux.dev> 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 <yajun.deng@linux.dev>
> > > > > ---
> > > > >    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
diff mbox series

Patch

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)