From: Andrei Vagin <avagin@gmail.com>
To: Peter Oskolkov <posk@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
Paul Turner <pjt@google.com>, Ben Segall <bsegall@google.com>,
Peter Oskolkov <posk@posk.io>,
Joel Fernandes <joel@joelfernandes.org>,
Andrew Morton <akpm@linux-foundation.org>,
Andrei Vagin <avagin@google.com>,
Jim Newsome <jnewsome@torproject.org>
Subject: Re: [RFC PATCH v0.1 7/9] sched/umcg: add UMCG server/worker API (early RFC)
Date: Fri, 21 May 2021 13:17:11 -0700 [thread overview]
Message-ID: <YKgVR5dM9RTZmCjh@gmail.com> (raw)
In-Reply-To: <20210520183614.1227046-8-posk@google.com>
On Thu, May 20, 2021 at 11:36:12AM -0700, Peter Oskolkov wrote:
> Implement UMCG server/worker API.
>
> This is an early RFC patch - the code seems working, but
> more testing is needed. Gaps I plan to address before this
> is ready for a detailed review:
>
> - preemption/interrupt handling;
> - better documentation/comments;
> - tracing;
> - additional testing;
> - corner cases like abnormal process/task termination;
> - in some cases where I kill the task (umcg_segv), returning
> an error may be more appropriate.
>
> All in all, please focus more on the high-level approach
> and less on things like variable names, (doc) comments, or indentation.
>
> Signed-off-by: Peter Oskolkov <posk@google.com>
> ---
> include/linux/mm_types.h | 5 +
> include/linux/syscalls.h | 5 +
> kernel/fork.c | 11 +
> kernel/sched/core.c | 11 +
> kernel/sched/umcg.c | 764 ++++++++++++++++++++++++++++++++++++++-
> kernel/sched/umcg.h | 54 +++
> mm/init-mm.c | 4 +
> 7 files changed, 845 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 6613b26a8894..5ca7b7d55775 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -562,6 +562,11 @@ struct mm_struct {
> #ifdef CONFIG_IOMMU_SUPPORT
> u32 pasid;
> #endif
> +
> +#ifdef CONFIG_UMCG
> + spinlock_t umcg_lock;
> + struct list_head umcg_groups;
> +#endif
> } __randomize_layout;
>
> /*
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 15de3e34ccee..2781659daaf1 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -1059,6 +1059,11 @@ asmlinkage long umcg_wait(u32 flags, const struct __kernel_timespec __user *time
> asmlinkage long umcg_wake(u32 flags, u32 next_tid);
> asmlinkage long umcg_swap(u32 wake_flags, u32 next_tid, u32 wait_flags,
> const struct __kernel_timespec __user *timeout);
> +asmlinkage long umcg_create_group(u32 api_version, u64, flags);
> +asmlinkage long umcg_destroy_group(u32 group_id);
> +asmlinkage long umcg_poll_worker(u32 flags, struct umcg_task __user **ut);
> +asmlinkage long umcg_run_worker(u32 flags, u32 worker_tid,
> + struct umcg_task __user **ut);
>
> /*
> * Architecture-specific system calls
> diff --git a/kernel/fork.c b/kernel/fork.c
> index ace4631b5b54..3a2a7950df8e 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1026,6 +1026,10 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
> seqcount_init(&mm->write_protect_seq);
> mmap_init_lock(mm);
> INIT_LIST_HEAD(&mm->mmlist);
> +#ifdef CONFIG_UMCG
> + spin_lock_init(&mm->umcg_lock);
> + INIT_LIST_HEAD(&mm->umcg_groups);
> +#endif
> mm->core_state = NULL;
> mm_pgtables_bytes_init(mm);
> mm->map_count = 0;
> @@ -1102,6 +1106,13 @@ static inline void __mmput(struct mm_struct *mm)
> list_del(&mm->mmlist);
> spin_unlock(&mmlist_lock);
> }
> +#ifdef CONFIG_UMCG
> + if (!list_empty(&mm->umcg_groups)) {
> + spin_lock(&mm->umcg_lock);
> + list_del(&mm->umcg_groups);
I am not sure that I understand what is going on here. umsg_groups is
the head of a group list. list_del is usually called on list entries.
Should we enumirate all groups here and destroy them?
> + spin_unlock(&mm->umcg_lock);
> + }
> +#endif
> if (mm->binfmt)
> module_put(mm->binfmt->module);
> mmdrop(mm);
...
> +/**
> + * sys_umcg_create_group - create a UMCG group
> + * @api_version: Requested API version.
> + * @flags: Reserved.
> + *
> + * Return:
> + * >= 0 - the group ID
> + * -EOPNOTSUPP - @api_version is not supported
> + * -EINVAL - @flags is not valid
> + * -ENOMEM - not enough memory
> + */
> +SYSCALL_DEFINE2(umcg_create_group, u32, api_version, u64, flags)
> +{
> + int ret;
> + struct umcg_group *group;
> + struct umcg_group *list_entry;
> + struct mm_struct *mm = current->mm;
> +
> + if (flags)
> + return -EINVAL;
> +
> + if (__api_version(api_version))
> + return -EOPNOTSUPP;
> +
> + group = kzalloc(sizeof(struct umcg_group), GFP_KERNEL);
> + if (!group)
> + return -ENOMEM;
> +
> + spin_lock_init(&group->lock);
> + INIT_LIST_HEAD(&group->list);
> + INIT_LIST_HEAD(&group->waiters);
> + group->flags = flags;
> + group->api_version = api_version;
> +
> + spin_lock(&mm->umcg_lock);
> +
> + list_for_each_entry_rcu(list_entry, &mm->umcg_groups, list) {
> + if (list_entry->group_id >= group->group_id)
> + group->group_id = list_entry->group_id + 1;
> + }
pls take into account that we need to be able to save and restore umcg
groups from user-space. There is the CRIU project that allows to
checkpoint/restore processes.
> +
> + list_add_rcu(&mm->umcg_groups, &group->list);
I think it should be:
list_add_rcu(&group->list, &mm->umcg_groups);
> +
> + ret = group->group_id;
> + spin_unlock(&mm->umcg_lock);
> +
> + return ret;
> +}
> +
Thanks,
Andrei
next prev parent reply other threads:[~2021-05-21 20:20 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-20 18:36 [RFC PATCH v0.1 0/9] UMCG early preview/RFC patchset Peter Oskolkov
2021-05-20 18:36 ` [RFC PATCH v0.1 1/9] sched/umcg: add UMCG syscall stubs and CONFIG_UMCG Peter Oskolkov
2021-05-22 18:40 ` kernel test robot
2021-05-22 21:49 ` kernel test robot
2021-05-20 18:36 ` [RFC PATCH v0.1 2/9] sched/umcg: add uapi/linux/umcg.h and sched/umcg.c Peter Oskolkov
2021-05-20 18:36 ` [RFC PATCH v0.1 3/9] sched: add WF_CURRENT_CPU and externise ttwu Peter Oskolkov
2021-05-20 18:36 ` [RFC PATCH v0.1 4/9] sched/umcg: implement core UMCG API Peter Oskolkov
2021-05-21 19:06 ` Andrei Vagin
2021-05-21 21:31 ` Jann Horn
2021-05-21 22:03 ` Peter Oskolkov
2021-05-21 19:32 ` Andy Lutomirski
2021-05-21 22:01 ` Peter Oskolkov
2021-05-21 21:33 ` Jann Horn
2021-06-09 13:01 ` Peter Zijlstra
2021-05-20 18:36 ` [RFC PATCH v0.1 5/9] lib/umcg: implement UMCG core API for userspace Peter Oskolkov
2021-05-20 18:36 ` [RFC PATCH v0.1 6/9] selftests/umcg: add UMCG core API selftest Peter Oskolkov
2021-05-20 18:36 ` [RFC PATCH v0.1 7/9] sched/umcg: add UMCG server/worker API (early RFC) Peter Oskolkov
2021-05-21 20:17 ` Andrei Vagin [this message]
2021-05-22 18:29 ` kernel test robot
2021-05-22 19:34 ` kernel test robot
2021-05-22 20:19 ` kernel test robot
2021-05-20 18:36 ` [RFC PATCH v0.1 8/9] lib/umcg: " Peter Oskolkov
2021-05-20 18:36 ` [RFC PATCH v0.1 9/9] selftests/umcg: add UMCG server/worker API selftest Peter Oskolkov
2021-05-20 21:17 ` [RFC PATCH v0.1 0/9] UMCG early preview/RFC patchset Jonathan Corbet
2021-05-20 21:38 ` Peter Oskolkov
2021-05-21 0:15 ` Randy Dunlap
2021-05-21 8:04 ` Peter Zijlstra
2021-05-21 15:08 ` Jonathan Corbet
2021-05-21 16:03 ` Peter Oskolkov
2021-05-21 19:17 ` Jonathan Corbet
2021-05-27 0:06 ` Peter Oskolkov
2021-05-27 15:41 ` Jonathan Corbet
[not found] ` <CAEWA0a72SvpcuN4ov=98T3uWtExPCr7BQePOgjkqD1ofWKEASw@mail.gmail.com>
2021-05-21 19:13 ` Peter Oskolkov
2021-05-21 23:08 ` Jann Horn
2021-06-09 12:54 ` Peter Zijlstra
2021-06-09 20:18 ` Peter Oskolkov
2021-06-10 18:02 ` Peter Zijlstra
2021-06-10 20:06 ` Peter Oskolkov
2021-07-07 17:45 ` Thierry Delisle
2021-07-08 21:44 ` Peter Oskolkov
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=YKgVR5dM9RTZmCjh@gmail.com \
--to=avagin@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=avagin@google.com \
--cc=bsegall@google.com \
--cc=jnewsome@torproject.org \
--cc=joel@joelfernandes.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=posk@google.com \
--cc=posk@posk.io \
--cc=tglx@linutronix.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.