From: Tao Zhou <tao.zhou@linux.dev>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@redhat.com, tglx@linutronix.de, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
bristot@redhat.com, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, linux-api@vger.kernel.org, x86@kernel.org,
pjt@google.com, posk@google.com, avagin@google.com,
jannh@google.com, tdelisle@uwaterloo.ca, mark.rutland@arm.com,
posk@posk.io, Tao Zhou <tao.zhou@linux.dev>
Subject: Re: [RFC][PATCH v2 5/5] sched: User Mode Concurency Groups
Date: Fri, 28 Jan 2022 02:31:24 +0800 [thread overview]
Message-ID: <YfLk/BlerNaAFMLL@geo.homenetwork> (raw)
In-Reply-To: <20220120160822.914418096@infradead.org>
On Thu, Jan 20, 2022 at 04:55:22PM +0100, Peter Zijlstra wrote:
Another iterator of the patch, some nits below.
[...]
> +/*
> + * Pinning a page inhibits rmap based unmap for Anon pages. Doing a load
> + * through the user mapping ensures the user mapping exists.
> + */
> +#define umcg_pin_and_load(_self, _pagep, _member) \
> +({ \
> + __label__ __out; \
> + int __ret = -EFAULT; \
> + \
> + if (pin_user_pages_fast((unsigned long)(_self), 1, 0, &(_pagep)) != 1) \
> + goto __out; \
> + \
> + if (!PageAnon(_pagep) || \
> + get_user(_member, &(_self)->_member)) { \
Here should be 'get_user(_member, &(_self)->##_member))'
if I am not wrong.
> + unpin_user_page(_pagep); \
> + goto __out; \
> + } \
> + __ret = 0; \
> +__out: __ret; \
> +})
[...]
> +
> +/*
> + * Enqueue @tsk on it's server's runnable list
> + *
> + * Must be called in umcg_pin_pages() context, relies on tsk->umcg_server.
> + *
> + * cmpxchg based single linked list add such that list integrity is never
> + * violated. Userspace *MUST* remove it from the list before changing ->state.
> + * As such, we must change state to RUNNABLE before enqueue.
> + *
> + * Returns:
> + * 0: success
> + * -EFAULT
> + */
> +static int umcg_enqueue_runnable(struct task_struct *tsk)
> +{
> + struct umcg_task __user *server = tsk->umcg_server_task;
> + struct umcg_task __user *self = tsk->umcg_task;
> + u64 first_ptr, *head = &server->runnable_workers_ptr;
> + u64 self_ptr = (unsigned long)self;
Why not 'u64 self_ptr = (u64)self;' ?
> + /*
> + * umcg_pin_pages() did access_ok() on both pointers, use self here
> + * only because __user_access_begin() isn't available in generic code.
> + */
> + if (!user_access_begin(self, sizeof(*self)))
> + return -EFAULT;
> +
> + unsafe_get_user(first_ptr, head, Efault);
> + do {
> + unsafe_put_user(first_ptr, &self->runnable_workers_ptr, Efault);
> + } while (!unsafe_try_cmpxchg_user(head, &first_ptr, self_ptr, Efault));
> +
> + user_access_end();
> + return 0;
> +
> +Efault:
> + user_access_end();
> + return -EFAULT;
> +}
[...]
> +/*
> + * Handles ::next_tid as per sys_umcg_wait().
> + *
> + * ::next_tid - return
> + * -----------------------------
> + * 0 - 0 (success)
> + *
> + * tid - -ESRCH (no such task, or not of this UMCG)
> + * - -EAGAIN (next::state != RUNNABLE)
> + * - 0 (success, ::next_tid |= RUNNING)
> + *
> + * tid|RUNNING - -EAGAIN (next::state != RUNNING)
> + * - 0 (success)
> + *
> + * Returns:
> + * 0: success
> + * -EFAULT
> + * -ESRCH
> + * -EAGAIN
> + */
> +static int umcg_wake_next(struct task_struct *tsk, struct umcg_task __user *self)
@tsk is not used in function.
> +{
> + struct umcg_task __user *next_task;
> + struct task_struct *next;
> + u32 next_tid, state;
> + int ret;
> +
> + if (get_user(next_tid, &self->next_tid))
> + return -EFAULT;
> +
> + if (!next_tid)
> + return 0;
> +
> + next = umcg_get_task(next_tid);
> + if (!next)
> + return -ESRCH;
> +
> + next_task = READ_ONCE(next->umcg_task);
> +
> + if (next_tid & UMCG_TID_RUNNING) {
> + ret = -EFAULT;
> + if (get_user(state, &next_task->state))
> + goto put_next;
> +
> + ret = 0;
> + if ((state & UMCG_TASK_MASK) != UMCG_TASK_RUNNING)
> + ret = -EAGAIN;
> +
> + } else {
> + ret = umcg_wake_task(next, next_task);
> + if (ret)
> + goto put_next;
> +
> + ret = -EFAULT;
> + if (put_user(next_tid | UMCG_TID_RUNNING, &self->next_tid))
> + goto put_next;
> +
> + /*
> + * If this is a worker doing sys_umcg_wait() switching to
> + * another worker, userspace has the responsibility to update
> + * server::next_tid.
> + */
> +
> + ret = 0;
> + }
> +
> +put_next:
> + put_task_struct(next);
> + return ret;
> +}
> +
next prev parent reply other threads:[~2022-01-27 18:31 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-20 15:55 [RFC][PATCH v2 0/5] sched: User Managed Concurrency Groups Peter Zijlstra
2022-01-20 15:55 ` [RFC][PATCH v2 1/5] mm: Avoid unmapping pinned pages Peter Zijlstra
2022-01-20 18:03 ` Nadav Amit
2022-01-21 7:59 ` Peter Zijlstra
2022-01-20 18:25 ` David Hildenbrand
2022-01-21 7:51 ` Peter Zijlstra
2022-01-21 8:22 ` David Hildenbrand
2022-01-21 8:59 ` Peter Zijlstra
2022-01-21 9:04 ` David Hildenbrand
2022-01-21 11:40 ` Peter Zijlstra
2022-01-21 12:04 ` David Hildenbrand
2022-01-20 15:55 ` [RFC][PATCH v2 2/5] entry,x86: Create common IRQ operations for exceptions Peter Zijlstra
2022-01-21 16:34 ` Mark Rutland
2022-01-20 15:55 ` [RFC][PATCH v2 3/5] sched/umcg: add WF_CURRENT_CPU and externise ttwu Peter Zijlstra
2022-01-20 15:55 ` [RFC][PATCH v2 4/5] x86/uaccess: Implement unsafe_try_cmpxchg_user() Peter Zijlstra
2022-01-27 2:17 ` Sean Christopherson
2022-01-27 6:36 ` Sean Christopherson
2022-01-27 9:56 ` Peter Zijlstra
2022-01-27 23:33 ` Sean Christopherson
2022-01-28 0:17 ` Nick Desaulniers
2022-01-28 16:29 ` Sean Christopherson
2022-01-27 9:55 ` Peter Zijlstra
2022-01-20 15:55 ` [RFC][PATCH v2 5/5] sched: User Mode Concurency Groups Peter Zijlstra
2022-01-21 11:47 ` Peter Zijlstra
2022-01-21 15:18 ` Peter Zijlstra
2022-01-24 14:29 ` Peter Zijlstra
2022-01-24 16:44 ` Peter Zijlstra
2022-01-24 17:06 ` Peter Oskolkov
2022-01-25 14:59 ` Peter Zijlstra
2022-01-24 13:59 ` Peter Zijlstra
2022-01-21 12:26 ` Peter Zijlstra
2022-01-21 16:57 ` Mark Rutland
2022-01-24 9:48 ` Peter Zijlstra
2022-01-24 10:03 ` Peter Zijlstra
2022-01-24 10:07 ` Peter Zijlstra
2022-01-24 10:27 ` Mark Rutland
2022-01-24 14:46 ` Tao Zhou
2022-01-27 12:19 ` Peter Zijlstra
2022-01-27 18:33 ` Tao Zhou
2022-01-27 12:25 ` Peter Zijlstra
2022-01-27 18:47 ` Tao Zhou
2022-01-27 12:26 ` Peter Zijlstra
2022-01-27 18:31 ` Tao Zhou [this message]
2022-01-20 17:28 ` [RFC][PATCH v2 0/5] sched: User Managed Concurrency Groups Peter Oskolkov
2022-01-21 8:01 ` Peter Zijlstra
2022-01-21 18:01 ` Steven Rostedt
2022-01-24 8:20 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YfLk/BlerNaAFMLL@geo.homenetwork \
--to=tao.zhou@linux.dev \
--cc=avagin@google.com \
--cc=bristot@redhat.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=jannh@google.com \
--cc=juri.lelli@redhat.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mark.rutland@arm.com \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=posk@google.com \
--cc=posk@posk.io \
--cc=rostedt@goodmis.org \
--cc=tdelisle@uwaterloo.ca \
--cc=tglx@linutronix.de \
--cc=vincent.guittot@linaro.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).