Re: [PATCH v0.8 3/6] sched/umcg: implement UMCG syscalls
flat view
From: Thierry Delisle <hidden>
Date: 2021-11-05 23:48:29
Also in:
linux-mm, lkml
On 2021-11-04 3:58 p.m., Peter Oskolkov wrote:
quoted hunk ↗ jump to hunk
+/* + * 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.
quoted hunk ↗ jump to hunk
+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.