Hey Will,

On Friday 13 Nov 2020 at 09:37:12 (+0000), Will Deacon wrote:
> -static int __set_cpus_allowed_ptr(struct task_struct *p,
> -                               const struct cpumask *new_mask, bool check)
> +static int __set_cpus_allowed_ptr_locked(struct task_struct *p,
> +                                      const struct cpumask *new_mask,
> +                                      bool check,
> +                                      struct rq *rq,
> +                                      struct rq_flags *rf)
>  {
>       const struct cpumask *cpu_valid_mask = cpu_active_mask;
>       unsigned int dest_cpu;
> -     struct rq_flags rf;
> -     struct rq *rq;
>       int ret = 0;

Should we have a lockdep assertion here?

> -     rq = task_rq_lock(p, &rf);
>       update_rq_clock(rq);
>  
>       if (p->flags & PF_KTHREAD) {
> @@ -1929,7 +1923,7 @@ static int __set_cpus_allowed_ptr(struct task_struct *p,
>       if (task_running(rq, p) || p->state == TASK_WAKING) {
>               struct migration_arg arg = { p, dest_cpu };
>               /* Need help from migration thread: drop lock and wait. */
> -             task_rq_unlock(rq, p, &rf);
> +             task_rq_unlock(rq, p, rf);
>               stop_one_cpu(cpu_of(rq), migration_cpu_stop, &arg);
>               return 0;
>       } else if (task_on_rq_queued(p)) {
> @@ -1937,20 +1931,69 @@ static int __set_cpus_allowed_ptr(struct task_struct 
> *p,
>                * OK, since we're going to drop the lock immediately
>                * afterwards anyway.
>                */
> -             rq = move_queued_task(rq, &rf, p, dest_cpu);
> +             rq = move_queued_task(rq, rf, p, dest_cpu);
>       }
>  out:
> -     task_rq_unlock(rq, p, &rf);
> +     task_rq_unlock(rq, p, rf);

And that's a little odd to have here no? Can we move it back on the
caller's side?

>       return ret;
>  }

Thanks,
Quentin

Reply via email to