On 2021-11-04 3:58 p.m., Peter Oskolkov wrote:
> +/*
> + * Try to wake up. May be called with preempt_disable set. May be called
> + * cross-process.
> + *
> + * Note: umcg_ttwu succeeds even if ttwu fails: see wait/wake state
> + * ordering logic.
> + */
> +static int umcg_ttwu(u32 next_tid, int wake_flags)
> +{
> + struct task_struct *next;
> +
> + rcu_read_lock();
> + next = find_task_by_vpid(next_tid);
> + if (!next || !umcg_wakeup_allowed(next)) {
> + rcu_read_unlock();
> + return -ESRCH;
> + }
> +
> + /* The result of ttwu below is ignored. */
> + try_to_wake_up(next, TASK_NORMAL, wake_flags);
> + rcu_read_unlock();
> +
> + return 0;
> +}
Doesn't try_to_wake_up return different values based on whether or not a
task
was woken up? I think it could be useful to propagate that result instead of
always returning zero. Even if it only helps for debugging.
> +static bool enqueue_idle_worker(struct umcg_task __user *ut_worker)
> +{
> + u64 __user *node = &ut_worker->idle_workers_ptr;
> + u64 __user *head_ptr;
> + u64 first = (u64)node;
> + u64 head;
> +
> + if (get_user(head, node) || !head)
> + return false;
> +
> + head_ptr = (u64 __user *)head;
> +
> + /* Mark the worker as pending. */
> + if (put_user(UMCG_IDLE_NODE_PENDING, node))
> + return false;
> +
> + /* Make the head point to the worker. */
> + if (xchg_user_64(head_ptr, &first))
> + return false;
> +
> + /* Make the worker point to the previous head. */
> + if (put_user(first, node))
> + return false;
> +
> + return true;
> +}
If the last two operation return false, whichever task tries to consume the
list could deadlock, depending on whether or not the ensuing
force_sig(SIGKILL); reaches the consuming task. Does the force_sig kill
the task or the entire process. Is it possible to consume this list from a
different process that shares the memory? I'm wondering if the last
two "return false" should attempt to retract the
UMCG_IDLE_NODE_PENDING.
