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.
