[PATCH 1/1] sched: apply ifdef CONFIG_SMP to meaningful code only on SMP system

[PATCH 1/1] sched: apply ifdef CONFIG_SMP to meaningful code only on SMP system

[byungchul.park]

From: Byungchul Park <byungchul.park@lge.com>

account_numa_dequeue and manipulating se->group_node are meaningful only on SMP

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 kernel/sched/fair.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 09456fc..93b8de6 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2337,15 +2337,17 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	update_load_sub(&cfs_rq->load, se->load.weight);
 	if (!parent_entity(se))
 		update_load_sub(&rq_of(cfs_rq)->load, se->load.weight);
+#ifdef CONFIG_SMP
 	if (entity_is_task(se)) {
 		account_numa_dequeue(rq_of(cfs_rq), task_of(se));
 		list_del_init(&se->group_node);
 	}
+#endif
 	cfs_rq->nr_running--;
 }
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
-# ifdef CONFIG_SMP
+#ifdef CONFIG_SMP
 static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
 {
 	long tg_weight;
-- 
1.7.9.5

Re: [PATCH 1/1] sched: apply ifdef CONFIG_SMP to meaningful code only on SMP system

[Peter Zijlstra]

On Sun, Jul 05, 2015 at 06:43:31PM +0900, byungchul.park@lge.com wrote:
> From: Byungchul Park <byungchul.park@lge.com>
> 
> account_numa_dequeue and manipulating se->group_node are meaningful only on SMP

Clearly the data element exists for !SMP and you didn't fix that.
Also, try and do this without adding more #ifdef.

Re: [PATCH 1/1] sched: apply ifdef CONFIG_SMP to meaningful code only on SMP system

[Byungchul Park]

On Mon, Jul 06, 2015 at 01:27:46PM +0200, Peter Zijlstra wrote:
> On Sun, Jul 05, 2015 at 06:43:31PM +0900, byungchul.park@lge.com wrote:
> > From: Byungchul Park <byungchul.park@lge.com>
> > 
> > account_numa_dequeue and manipulating se->group_node are meaningful only on SMP
> 
> Clearly the data element exists for !SMP and you didn't fix that.
> Also, try and do this without adding more #ifdef.

yes.

it will be better if it can be done without #ifdef.
i will try it.

thank you very much :)
byungchul

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] sched: apply ifdef CONFIG_SMP to meaningful code only on SMP system

[Byungchul Park]

On Sun, Jul 05, 2015 at 06:43:31PM +0900, byungchul.park@lge.com wrote:
> From: Byungchul Park <byungchul.park@lge.com>

Hello,

This patch was rejected and the next version having tried to apply what
peterz recommanded, was almost ignored last year. But now, exactly same
thing is already in the tree which was posted by Tim Chen. Yes. This is
a trivial one, I thus don't mind much. Commiting patches is not important.

However, I want to ask you to treat patches by new one like me with more
consideration, please. Sometimes I feel thankful while sometimes I feel
disappointed with a happening like this. It would be appriciated if you
do so.

Thank you,
Byungchul

> 
> account_numa_dequeue and manipulating se->group_node are meaningful only on SMP
> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  kernel/sched/fair.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 09456fc..93b8de6 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2337,15 +2337,17 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	update_load_sub(&cfs_rq->load, se->load.weight);
>  	if (!parent_entity(se))
>  		update_load_sub(&rq_of(cfs_rq)->load, se->load.weight);
> +#ifdef CONFIG_SMP
>  	if (entity_is_task(se)) {
>  		account_numa_dequeue(rq_of(cfs_rq), task_of(se));
>  		list_del_init(&se->group_node);
>  	}
> +#endif
>  	cfs_rq->nr_running--;
>  }
>  
>  #ifdef CONFIG_FAIR_GROUP_SCHED
> -# ifdef CONFIG_SMP
> +#ifdef CONFIG_SMP
>  static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>  {
>  	long tg_weight;
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] sched: apply ifdef CONFIG_SMP to meaningful code only on SMP system

[Byungchul Park]

On Thu, Aug 25, 2016 at 03:56:59PM +0900, Byungchul Park wrote:
> On Sun, Jul 05, 2015 at 06:43:31PM +0900, byungchul.park@lge.com wrote:
> > From: Byungchul Park <byungchul.park@lge.com>
> 
> Hello,
> 
> This patch was rejected and the next version having tried to apply what
> peterz recommanded, was almost ignored last year. But now, exactly same
> thing is already in the tree which was posted by Tim Chen. Yes. This is
> a trivial one, I thus don't mind much. Commiting patches is not important.

I meant that "Whether my patches are commited or not is not important".

> However, I want to ask you to treat patches by new one like me with more
> consideration, please. Sometimes I feel thankful while sometimes I feel
> disappointed with a happening like this. It would be appriciated if you
> do so.
> 
> Thank you,
> Byungchul
> 
> > 
> > account_numa_dequeue and manipulating se->group_node are meaningful only on SMP
> > 
> > Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> > ---
> >  kernel/sched/fair.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 09456fc..93b8de6 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -2337,15 +2337,17 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
> >  	update_load_sub(&cfs_rq->load, se->load.weight);
> >  	if (!parent_entity(se))
> >  		update_load_sub(&rq_of(cfs_rq)->load, se->load.weight);
> > +#ifdef CONFIG_SMP
> >  	if (entity_is_task(se)) {
> >  		account_numa_dequeue(rq_of(cfs_rq), task_of(se));
> >  		list_del_init(&se->group_node);
> >  	}
> > +#endif
> >  	cfs_rq->nr_running--;
> >  }
> >  
> >  #ifdef CONFIG_FAIR_GROUP_SCHED
> > -# ifdef CONFIG_SMP
> > +#ifdef CONFIG_SMP
> >  static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
> >  {
> >  	long tg_weight;
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/