Re: [RFC][PATCH] SCHED_EDF scheduling class

[roel kluin]

>  static int __sched_setscheduler(struct task_struct *p, int policy,
> -                               struct sched_param *param, bool user)
> +                               struct sched_param *param,
> +                               struct sched_param_ex *param_ex,
> +                               bool user)
...

> @@ -6200,6 +6364,13 @@ recheck:
>            (p->mm && param->sched_priority > MAX_USER_RT_PRIO-1) ||
>            (!p->mm && param->sched_priority > MAX_RT_PRIO-1))
>                return -EINVAL;
> +       if (edf_policy(policy) && param_ex->sched_priority != 0)
> +               return -EINVAL;
> +       if (edf_policy(policy) && (param_ex == NULL ||
> +           timespec_to_ns(&param_ex->sched_period) == 0 ||
> +           timespec_to_ns(&param_ex->sched_period) <
> +           timespec_to_ns(&param_ex->sched_runtime)))
> +               return -EINVAL;

shouldn't the NULL test be moved upwards, to prevent a dereference of
a NULL pointer? Also I notice that `timespec_to_ns(&param_ex->sched_period)'
is called twice, maybe gcc does this but can't we do something like

        if (edf_policy(policy)) {
                if (param_ex == NULL || param_ex->sched_priority != 0)
                        return -EINVAL;
                s64 psp = timespec_to_ns(&param_ex->sched_period);
                if (psp == 0 || psp < timespec_to_ns(&param_ex->sched_runtime))
                        return -EINVAL;
        }

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