Messages in this thread | | | Subject | Re: [PATCH v0.8 3/6] sched/umcg: implement UMCG syscalls | From | Thierry Delisle <> | Date | Fri, 5 Nov 2021 19:48:21 -0400 |
| |
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.
| |