Messages in this thread |  | | Date | Wed, 25 Nov 2020 14:03:22 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH -tip 22/32] sched: Split the cookie and setup per-task cookie on fork |
| |
On Tue, Nov 17, 2020 at 06:19:52PM -0500, Joel Fernandes (Google) wrote: > +static bool sched_core_get_task_cookie(unsigned long cookie) > +{ > + struct sched_core_cookie *ptr = (struct sched_core_cookie *)cookie; > + > + /* > + * NOTE: sched_core_put() is not done by put_task_cookie(). Instead, it > + * is done after the stopper runs. > + */ > + sched_core_get(); > + return refcount_inc_not_zero(&ptr->refcnt);
See below, but afaict this should be refcount_inc().
> +}
> + /* > + * t1 joining t2 > + * CASE 1: > + * before 0 0 > + * after new cookie new cookie > + * > + * CASE 2: > + * before X (non-zero) 0 > + * after 0 0 > + * > + * CASE 3: > + * before 0 X (non-zero) > + * after X X > + * > + * CASE 4: > + * before Y (non-zero) X (non-zero) > + * after X X > + */ > + if (!t1->core_task_cookie && !t2->core_task_cookie) { > + /* CASE 1. */ > + cookie = sched_core_alloc_task_cookie(); > + if (!cookie) > + goto out_unlock; > + > + /* Add another reference for the other task. */ > + if (!sched_core_get_task_cookie(cookie)) {
afaict this should be refcount_inc(), as this can never fail and if it does, it's an error.
> + ret = -EINVAL; > + goto out_unlock; > + } > + > + wr.tasks[0] = t1; > + wr.tasks[1] = t2; > + wr.cookies[0] = wr.cookies[1] = cookie; > + > + } else if (t1->core_task_cookie && !t2->core_task_cookie) { > + /* CASE 2. */ > + sched_core_put_task_cookie(t1->core_task_cookie); > + sched_core_put_after_stopper = true; > + > + wr.tasks[0] = t1; /* Reset cookie for t1. */ > + > + } else if (!t1->core_task_cookie && t2->core_task_cookie) { > + /* CASE 3. */ > + if (!sched_core_get_task_cookie(t2->core_task_cookie)) {
afaict this can never fail either, because you're calling in here with a reference on t2
> + ret = -EINVAL; > + goto out_unlock; > + } > + > + wr.tasks[0] = t1; > + wr.cookies[0] = t2->core_task_cookie; > + > + } else { > + /* CASE 4. */ > + if (!sched_core_get_task_cookie(t2->core_task_cookie)) {
Same.
> + ret = -EINVAL; > + goto out_unlock; > + } > + sched_core_put_task_cookie(t1->core_task_cookie); > + sched_core_put_after_stopper = true; > + > + wr.tasks[0] = t1; > + wr.cookies[0] = t2->core_task_cookie; > + }
|  |