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;
> + }
