From: Peter Zijlstra <peterz@infradead.org>
To: PierceGriffiths <pierceagriffiths@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scheduler: conditional statement cleanup
Date: Wed, 26 Sep 2018 09:46:09 +0200 [thread overview]
Message-ID: <20180926074609.GA1654@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20180921202205.11769-1-pierceagriffiths@gmail.com>
On Fri, Sep 21, 2018 at 03:22:03PM -0500, PierceGriffiths wrote:
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 625bc9897f62..443a1f235cfd 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -617,12 +617,8 @@ bool sched_can_stop_tick(struct rq *rq)
> * If there are more than one RR tasks, we need the tick to effect the
> * actual RR behaviour.
> */
> - if (rq->rt.rr_nr_running) {
> - if (rq->rt.rr_nr_running == 1)
> - return true;
> - else
> - return false;
> - }
> + if (rq->rt.rr_nr_running)
> + return rq->rt.rr_nr_running == 1;
>
> /*
> * If there's no RR tasks, but FIFO tasks, we can skip the tick, no
> diff --git a/kernel/sched/cpupri.c b/kernel/sched/cpupri.c
> index daaadf939ccb..152c133e8247 100644
> --- a/kernel/sched/cpupri.c
> +++ b/kernel/sched/cpupri.c
> @@ -29,20 +29,16 @@
> #include "sched.h"
>
> /* Convert between a 140 based task->prio, and our 102 based cpupri */
> -static int convert_prio(int prio)
> +static int convert_prio(const int prio)
> {
> - int cpupri;
> -
> if (prio == CPUPRI_INVALID)
> - cpupri = CPUPRI_INVALID;
> + return CPUPRI_INVALID;
> else if (prio == MAX_PRIO)
> - cpupri = CPUPRI_IDLE;
> + return CPUPRI_IDLE;
> else if (prio >= MAX_RT_PRIO)
> - cpupri = CPUPRI_NORMAL;
> + return CPUPRI_NORMAL;
> else
> - cpupri = MAX_RT_PRIO - prio + 1;
> -
> - return cpupri;
> + return MAX_RT_PRIO - prio + 1;
Code improves if you leave out the last else.
> }
>
> /**
> @@ -222,7 +216,7 @@ int cpupri_init(struct cpupri *cp)
> return 0;
>
> cleanup:
> - for (i--; i >= 0; i--)
> + while (--i >= 0)
> free_cpumask_var(cp->pri_to_cpu[i].mask);
> return -ENOMEM;
> }
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 2e2955a8cf8f..acf1b94669ad 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -142,10 +142,12 @@ void free_rt_sched_group(struct task_group *tg)
> destroy_rt_bandwidth(&tg->rt_bandwidth);
>
> for_each_possible_cpu(i) {
> - if (tg->rt_rq)
> - kfree(tg->rt_rq[i]);
> - if (tg->rt_se)
> - kfree(tg->rt_se[i]);
> + /* Don't need to check if tg->rt_rq[i]
> + * or tg->rt_se[i] are NULL, since kfree(NULL)
> + * simply performs no operation
> + */
Don't bother with the comment tho (but if you do, know this is the wrong
comment style).
> + kfree(tg->rt_rq[i]);
> + kfree(tg->rt_se[i]);
> }
>
> kfree(tg->rt_rq);
> @@ -1393,7 +1389,7 @@ select_task_rq_rt(struct task_struct *p, int cpu, int sd_flag, int flags)
>
> /* For anything but wake ups, just return the task_cpu */
> if (sd_flag != SD_BALANCE_WAKE && sd_flag != SD_BALANCE_FORK)
> - goto out;
> + return cpu;
>
> rq = cpu_rq(cpu);
>
> @@ -1437,7 +1433,6 @@ select_task_rq_rt(struct task_struct *p, int cpu, int sd_flag, int flags)
> }
> rcu_read_unlock();
>
> -out:
> return cpu;
> }
>
These changes are OK with minor edits, the rest just makes the code
harder to read.
prev parent reply other threads:[~2018-09-26 7:46 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-21 20:22 [PATCH] scheduler: conditional statement cleanup PierceGriffiths
2018-09-22 7:08 ` kbuild test robot
2018-09-22 10:26 ` Peter Zijlstra
2018-09-25 20:07 ` Pierce Griffiths
2018-09-26 7:46 ` Peter Zijlstra [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180926074609.GA1654@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pierceagriffiths@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).