[v6,02/11] sched/pelt: remove blank line

Message ID 1528459794-13066-3-git-send-email-vincent.guittot@linaro.org
State New
Headers show
Series
  • track CPU utilization
Related show

Commit Message

Vincent Guittot June 8, 2018, 12:09 p.m.
remove some blank lines in __update_load_avg_se() and __update_load_avg_cfs_rq

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Reported-by: Patrick Bellasi <patrick.bellasi@arm.com>
Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>

---
 kernel/sched/pelt.c | 2 --
 1 file changed, 2 deletions(-)

-- 
2.7.4

Comments

Peter Zijlstra June 21, 2018, 2:33 p.m. | #1
On Fri, Jun 08, 2018 at 02:09:45PM +0200, Vincent Guittot wrote:
> diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c

> index e6ecbb2..4174582 100644

> --- a/kernel/sched/pelt.c

> +++ b/kernel/sched/pelt.c

> @@ -287,7 +287,6 @@ int __update_load_avg_se(u64 now, int cpu, struct cfs_rq *cfs_rq, struct sched_e

>  

>  	if (___update_load_sum(now, cpu, &se->avg, !!se->on_rq, !!se->on_rq,

>  				cfs_rq->curr == se)) {

> -

>  		___update_load_avg(&se->avg, se_weight(se), se_runnable(se));

>  		cfs_se_util_change(&se->avg);

>  		return 1;

> @@ -302,7 +301,6 @@ int __update_load_avg_cfs_rq(u64 now, int cpu, struct cfs_rq *cfs_rq)

>  				scale_load_down(cfs_rq->load.weight),

>  				scale_load_down(cfs_rq->runnable_weight),

>  				cfs_rq->curr != NULL)) {

> -

>  		___update_load_avg(&cfs_rq->avg, 1, 1);

>  		return 1;

>  	}


So I put them there on purpose, I find it easier to read when a
multi-line if statement and the body are separated. Makes it clearer
where the if ends and the block begins.

I mean, all whitespace in C is superfluous, and yet we keep adding it to
these files :-)
Vincent Guittot June 21, 2018, 6:42 p.m. | #2
On Thu, 21 Jun 2018 at 16:33, Peter Zijlstra <peterz@infradead.org> wrote:
>

> On Fri, Jun 08, 2018 at 02:09:45PM +0200, Vincent Guittot wrote:

> > diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c

> > index e6ecbb2..4174582 100644

> > --- a/kernel/sched/pelt.c

> > +++ b/kernel/sched/pelt.c

> > @@ -287,7 +287,6 @@ int __update_load_avg_se(u64 now, int cpu, struct cfs_rq *cfs_rq, struct sched_e

> >

> >       if (___update_load_sum(now, cpu, &se->avg, !!se->on_rq, !!se->on_rq,

> >                               cfs_rq->curr == se)) {

> > -

> >               ___update_load_avg(&se->avg, se_weight(se), se_runnable(se));

> >               cfs_se_util_change(&se->avg);

> >               return 1;

> > @@ -302,7 +301,6 @@ int __update_load_avg_cfs_rq(u64 now, int cpu, struct cfs_rq *cfs_rq)

> >                               scale_load_down(cfs_rq->load.weight),

> >                               scale_load_down(cfs_rq->runnable_weight),

> >                               cfs_rq->curr != NULL)) {

> > -

> >               ___update_load_avg(&cfs_rq->avg, 1, 1);

> >               return 1;

> >       }

>

> So I put them there on purpose, I find it easier to read when a

> multi-line if statement and the body are separated. Makes it clearer

> where the if ends and the block begins.

>

> I mean, all whitespace in C is superfluous, and yet we keep adding it to

> these files :-)


I'm fine with keeping it as well as for newly added ones; Patrick
raised the point on a similar empty line  for the newly added
update_rt_rq_load_avg() function on previous revision and just wanted
to keep them all aligned

Patch

diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c
index e6ecbb2..4174582 100644
--- a/kernel/sched/pelt.c
+++ b/kernel/sched/pelt.c
@@ -287,7 +287,6 @@  int __update_load_avg_se(u64 now, int cpu, struct cfs_rq *cfs_rq, struct sched_e
 
 	if (___update_load_sum(now, cpu, &se->avg, !!se->on_rq, !!se->on_rq,
 				cfs_rq->curr == se)) {
-
 		___update_load_avg(&se->avg, se_weight(se), se_runnable(se));
 		cfs_se_util_change(&se->avg);
 		return 1;
@@ -302,7 +301,6 @@  int __update_load_avg_cfs_rq(u64 now, int cpu, struct cfs_rq *cfs_rq)
 				scale_load_down(cfs_rq->load.weight),
 				scale_load_down(cfs_rq->runnable_weight),
 				cfs_rq->curr != NULL)) {
-
 		___update_load_avg(&cfs_rq->avg, 1, 1);
 		return 1;
 	}