* [PATCH v5 4/5] selftests: pidfd: add pidfd.h UAPI wrapper
From: Lorenzo Stoakes @ 2024-10-25 9:41 UTC (permalink / raw)
To: Christian Brauner
Cc: Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
In-Reply-To: <cover.1729848252.git.lorenzo.stoakes@oracle.com>
Conflicts can arise between system fcntl.h and linux/fcntl.h, imported by
the linux/pidfd.h UAPI header.
Work around this by adding a wrapper for linux/pidfd.h to
tools/include/ which sets the linux/fcntl.h header guard ahead of
importing the pidfd.h header file.
Adjust the pidfd selftests Makefile to reference this include directory and
put it at a higher precidence than any make header installed headers to
ensure the wrapper is preferred.
This way we can directly import the UAPI header file without issue, use the
latest system header file without having to duplicate anything.
Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
tools/include/linux/pidfd.h | 14 ++++++++++++++
tools/testing/selftests/pidfd/Makefile | 3 +--
2 files changed, 15 insertions(+), 2 deletions(-)
create mode 100644 tools/include/linux/pidfd.h
diff --git a/tools/include/linux/pidfd.h b/tools/include/linux/pidfd.h
new file mode 100644
index 000000000000..113c8023072d
--- /dev/null
+++ b/tools/include/linux/pidfd.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _TOOLS_LINUX_PIDFD_H
+#define _TOOLS_LINUX_PIDFD_H
+
+/*
+ * Some systems have issues with the linux/fcntl.h import in linux/pidfd.h, so
+ * work around this by setting the header guard.
+ */
+#define _LINUX_FCNTL_H
+#include "../../../include/uapi/linux/pidfd.h"
+#undef _LINUX_FCNTL_H
+
+#endif /* _TOOLS_LINUX_PIDFD_H */
diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile
index d731e3e76d5b..f5038c9dae14 100644
--- a/tools/testing/selftests/pidfd/Makefile
+++ b/tools/testing/selftests/pidfd/Makefile
@@ -1,8 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
-CFLAGS += -g $(KHDR_INCLUDES) -pthread -Wall
+CFLAGS += -g -isystem $(top_srcdir)/tools/include $(KHDR_INCLUDES) -pthread -Wall
TEST_GEN_PROGS := pidfd_test pidfd_fdinfo_test pidfd_open_test \
pidfd_poll_test pidfd_wait pidfd_getfd_test pidfd_setns_test
include ../lib.mk
-
--
2.47.0
^ permalink raw reply related
* [PATCH v5 3/5] tools: testing: separate out wait_for_pid() into helper header
From: Lorenzo Stoakes @ 2024-10-25 9:41 UTC (permalink / raw)
To: Christian Brauner
Cc: Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
In-Reply-To: <cover.1729848252.git.lorenzo.stoakes@oracle.com>
It seems tests other than the pidfd tests use the wait_for_pid() function
declared in pidfd.h.
Since we will shortly be modifying pidfd.h in a way that might clash with
other tests, separate this out and update tests accordingly.
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
tools/testing/selftests/cgroup/test_kill.c | 2 +-
.../pid_namespace/regression_enomem.c | 2 +-
tools/testing/selftests/pidfd/pidfd.h | 26 +------------
tools/testing/selftests/pidfd/pidfd_helpers.h | 39 +++++++++++++++++++
4 files changed, 42 insertions(+), 27 deletions(-)
create mode 100644 tools/testing/selftests/pidfd/pidfd_helpers.h
diff --git a/tools/testing/selftests/cgroup/test_kill.c b/tools/testing/selftests/cgroup/test_kill.c
index 0e5bb6c7307a..2367f645fe89 100644
--- a/tools/testing/selftests/cgroup/test_kill.c
+++ b/tools/testing/selftests/cgroup/test_kill.c
@@ -10,7 +10,7 @@
#include <unistd.h>
#include "../kselftest.h"
-#include "../pidfd/pidfd.h"
+#include "../pidfd/pidfd_helpers.h"
#include "cgroup_util.h"
/*
diff --git a/tools/testing/selftests/pid_namespace/regression_enomem.c b/tools/testing/selftests/pid_namespace/regression_enomem.c
index 7d84097ad45c..f3e6989c8069 100644
--- a/tools/testing/selftests/pid_namespace/regression_enomem.c
+++ b/tools/testing/selftests/pid_namespace/regression_enomem.c
@@ -12,7 +12,7 @@
#include <sys/wait.h>
#include "../kselftest_harness.h"
-#include "../pidfd/pidfd.h"
+#include "../pidfd/pidfd_helpers.h"
/*
* Regression test for:
diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h
index 88d6830ee004..0f3fc51cec73 100644
--- a/tools/testing/selftests/pidfd/pidfd.h
+++ b/tools/testing/selftests/pidfd/pidfd.h
@@ -17,6 +17,7 @@
#include <sys/wait.h>
#include "../kselftest.h"
+#include "pidfd_helpers.h"
#ifndef P_PIDFD
#define P_PIDFD 3
@@ -68,31 +69,6 @@
#define PIDFD_SKIP 3
#define PIDFD_XFAIL 4
-static inline int wait_for_pid(pid_t pid)
-{
- int status, ret;
-
-again:
- ret = waitpid(pid, &status, 0);
- if (ret == -1) {
- if (errno == EINTR)
- goto again;
-
- ksft_print_msg("waitpid returned -1, errno=%d\n", errno);
- return -1;
- }
-
- if (!WIFEXITED(status)) {
- ksft_print_msg(
- "waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n",
- WIFSIGNALED(status), WTERMSIG(status));
- return -1;
- }
-
- ret = WEXITSTATUS(status);
- return ret;
-}
-
static inline int sys_pidfd_open(pid_t pid, unsigned int flags)
{
return syscall(__NR_pidfd_open, pid, flags);
diff --git a/tools/testing/selftests/pidfd/pidfd_helpers.h b/tools/testing/selftests/pidfd/pidfd_helpers.h
new file mode 100644
index 000000000000..5637bfe888de
--- /dev/null
+++ b/tools/testing/selftests/pidfd/pidfd_helpers.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __PIDFD_HELPERS_H
+#define __PIDFD_HELPERS_H
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include "../kselftest.h"
+
+static inline int wait_for_pid(pid_t pid)
+{
+ int status, ret;
+
+again:
+ ret = waitpid(pid, &status, 0);
+ if (ret == -1) {
+ if (errno == EINTR)
+ goto again;
+
+ ksft_print_msg("waitpid returned -1, errno=%d\n", errno);
+ return -1;
+ }
+
+ if (!WIFEXITED(status)) {
+ ksft_print_msg(
+ "waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n",
+ WIFSIGNALED(status), WTERMSIG(status));
+ return -1;
+ }
+
+ ret = WEXITSTATUS(status);
+ return ret;
+}
+
+#endif /* __PIDFD_HELPERS_H */
--
2.47.0
^ permalink raw reply related
* [PATCH v5 2/5] pidfd: add PIDFD_SELF_* sentinels to refer to own thread/process
From: Lorenzo Stoakes @ 2024-10-25 9:41 UTC (permalink / raw)
To: Christian Brauner
Cc: Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
In-Reply-To: <cover.1729848252.git.lorenzo.stoakes@oracle.com>
It is useful to be able to utilise the pidfd mechanism to reference the
current thread or process (from a userland point of view - thread group
leader from the kernel's point of view).
Therefore introduce PIDFD_SELF_THREAD to refer to the current thread, and
PIDFD_SELF_THREAD_GROUP to refer to the current thread group leader.
For convenience and to avoid confusion from userland's perspective we alias
these:
* PIDFD_SELF is an alias for PIDFD_SELF_THREAD - This is nearly always what
the user will want to use, as they would find it surprising if for
instance fd's were unshared()'d and they wanted to invoke pidfd_getfd()
and that failed.
* PIDFD_SELF_PROCESS is an alias for PIDFD_SELF_THREAD_GROUP - Most users
have no concept of thread groups or what a thread group leader is, and
from userland's perspective and nomenclature this is what userland
considers to be a process.
Due to the refactoring of the central __pidfd_get_pid() function we can
implement this functionality centrally, providing the use of this sentinel
in most functionality which utilises pidfd's.
We need to explicitly adjust kernel_waitid_prepare() to permit this (though
it wouldn't really make sense to use this there, we provide the ability for
consistency).
We explicitly disallow use of this in setns(), which would otherwise have
required explicit custom handling, as it doesn't make sense to set the
current calling thread to join the namespace of itself.
As the callers of pidfd_get_pid() expect an increased reference count on
the pid we do so in the self case, reducing churn and avoiding any breakage
from existing logic which decrements this reference count.
This change implicitly provides PIDFD_SELF_* support in the waitid(P_PIDFS,
...), process_madvise(), process_mrelease(), pidfd_send_signal(), and
pidfd_getfd() system calls.
Things such as polling a pidfs and general fd operations are not supported,
this strictly provides the sentinel for APIs which explicitly accept a
pidfd.
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
include/linux/pid.h | 8 ++++--
include/uapi/linux/pidfd.h | 15 +++++++++++
kernel/exit.c | 3 ++-
kernel/nsproxy.c | 1 +
kernel/pid.c | 51 ++++++++++++++++++++++++--------------
5 files changed, 57 insertions(+), 21 deletions(-)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index d466890e1b35..3b2ac7567a88 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -78,11 +78,15 @@ struct file;
* __pidfd_get_pid() - Retrieve a pid associated with the specified pidfd.
*
* @pidfd: The pidfd whose pid we want, or the fd of a /proc/<pid> file if
- * @alloc_proc is also set.
+ * @alloc_proc is also set, or PIDFD_SELF_* to refer to the current
+ * thread or thread group leader.
* @allow_proc: If set, then an fd of a /proc/<pid> file can be passed instead
* of a pidfd, and this will be used to determine the pid.
+
* @flags: Output variable, if non-NULL, then the file->f_flags of the
- * pidfd will be set here.
+ * pidfd will be set here or If PIDFD_SELF_THREAD is set, this is
+ * set to PIDFD_THREAD, otherwise if PIDFD_SELF_THREAD_GROUP then
+ * this is set to zero.
*
* Returns: If successful, the pid associated with the pidfd, otherwise an
* error.
diff --git a/include/uapi/linux/pidfd.h b/include/uapi/linux/pidfd.h
index 565fc0629fff..0ca2ebf906fd 100644
--- a/include/uapi/linux/pidfd.h
+++ b/include/uapi/linux/pidfd.h
@@ -29,4 +29,19 @@
#define PIDFD_GET_USER_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 9)
#define PIDFD_GET_UTS_NAMESPACE _IO(PIDFS_IOCTL_MAGIC, 10)
+/*
+ * Special sentinel values which can be used to refer to the current thread or
+ * thread group leader (which from a userland perspective is the process).
+ */
+#define PIDFD_SELF PIDFD_SELF_THREAD
+#define PIDFD_SELF_PROCESS PIDFD_SELF_THREAD_GROUP
+
+#define PIDFD_SELF_THREAD -100 /* Current thread. */
+#define PIDFD_SELF_THREAD_GROUP -200 /* Current thread group leader. */
+
+static inline int pidfd_is_self_sentinel(pid_t pid)
+{
+ return pid == PIDFD_SELF_THREAD || pid == PIDFD_SELF_THREAD_GROUP;
+}
+
#endif /* _UAPI_LINUX_PIDFD_H */
diff --git a/kernel/exit.c b/kernel/exit.c
index 619f0014c33b..3eb20f8252ee 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -71,6 +71,7 @@
#include <linux/user_events.h>
#include <linux/uaccess.h>
+#include <uapi/linux/pidfd.h>
#include <uapi/linux/wait.h>
#include <asm/unistd.h>
@@ -1739,7 +1740,7 @@ int kernel_waitid_prepare(struct wait_opts *wo, int which, pid_t upid,
break;
case P_PIDFD:
type = PIDTYPE_PID;
- if (upid < 0)
+ if (upid < 0 && !pidfd_is_self_sentinel(upid))
return -EINVAL;
pid = pidfd_get_pid(upid, &f_flags);
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index dc952c3b05af..d239f7eeaa1f 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -550,6 +550,7 @@ SYSCALL_DEFINE2(setns, int, fd, int, flags)
struct nsset nsset = {};
int err = 0;
+ /* If fd is PIDFD_SELF_*, implicitly fail here, as invalid. */
if (!fd_file(f))
return -EBADF;
diff --git a/kernel/pid.c b/kernel/pid.c
index 94c97559e5c5..8742157b36f8 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -535,33 +535,48 @@ struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
}
EXPORT_SYMBOL_GPL(find_ge_pid);
+static struct pid *pidfd_get_pid_self(unsigned int pidfd, unsigned int *flags)
+{
+ bool is_thread = pidfd == PIDFD_SELF_THREAD;
+ enum pid_type type = is_thread ? PIDTYPE_PID : PIDTYPE_TGID;
+ struct pid *pid = *task_pid_ptr(current, type);
+
+ /* The caller expects an elevated reference count. */
+ get_pid(pid);
+ return pid;
+}
+
struct pid *__pidfd_get_pid(unsigned int pidfd, bool allow_proc,
unsigned int *flags)
{
- struct pid *pid;
- struct fd f = fdget(pidfd);
- struct file *file = fd_file(f);
+ if (pidfd_is_self_sentinel(pidfd)) {
+ return pidfd_get_pid_self(pidfd, flags);
+ } else {
+ struct pid *pid;
+ struct fd f = fdget(pidfd);
+ struct file *file = fd_file(f);
- if (!file)
- return ERR_PTR(-EBADF);
+ if (!file)
+ return ERR_PTR(-EBADF);
- pid = pidfd_pid(file);
- /* If we allow opening a pidfd via /proc/<pid>, do so. */
- if (IS_ERR(pid) && allow_proc)
- pid = tgid_pidfd_to_pid(file);
+ pid = pidfd_pid(file);
+ /* If we allow opening a pidfd via /proc/<pid>, do so. */
+ if (IS_ERR(pid) && allow_proc)
+ pid = tgid_pidfd_to_pid(file);
- if (IS_ERR(pid)) {
+ if (IS_ERR(pid)) {
+ fdput(f);
+ return pid;
+ }
+
+ /* Pin pid before we release fd. */
+ get_pid(pid);
+ if (flags)
+ *flags = file->f_flags;
fdput(f);
+
return pid;
}
-
- /* Pin pid before we release fd. */
- get_pid(pid);
- if (flags)
- *flags = file->f_flags;
- fdput(f);
-
- return pid;
}
/**
--
2.47.0
^ permalink raw reply related
* [PATCH v5 1/5] pidfd: extend pidfd_get_pid() and de-duplicate pid lookup
From: Lorenzo Stoakes @ 2024-10-25 9:41 UTC (permalink / raw)
To: Christian Brauner
Cc: Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
In-Reply-To: <cover.1729848252.git.lorenzo.stoakes@oracle.com>
The means by which a pid is determined from a pidfd is duplicated, with
some callers holding a reference to the (pid)fd, and others explicitly
pinning the pid.
Introduce __pidfd_get_pid() which narrows this to one approach of pinning
the pid, with an optional output parameters for file->f_flags to avoid the
need to hold onto a file to retrieve this.
Additionally, allow the ability to open a pidfd by opening a /proc/<pid>
directory, utilised by the pidfd_send_signal() system call, providing a
pidfd_get_pid_proc() helper function to do so.
Doing this allows us to eliminate open-coded pidfd pid lookup and to
consistently handle this in one place.
This lays the groundwork for a subsequent patch which adds a new sentinel
pidfd to explicitly reference the current process (i.e. thread group
leader) without the need for a pidfd.
Reviewed-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
include/linux/pid.h | 30 +++++++++++++++++++++++++++++-
kernel/pid.c | 42 ++++++++++++++++++++++++------------------
kernel/signal.c | 29 ++++++-----------------------
3 files changed, 59 insertions(+), 42 deletions(-)
diff --git a/include/linux/pid.h b/include/linux/pid.h
index a3aad9b4074c..d466890e1b35 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -2,6 +2,7 @@
#ifndef _LINUX_PID_H
#define _LINUX_PID_H
+#include <linux/file.h>
#include <linux/pid_types.h>
#include <linux/rculist.h>
#include <linux/rcupdate.h>
@@ -72,8 +73,35 @@ extern struct pid init_struct_pid;
struct file;
+
+/**
+ * __pidfd_get_pid() - Retrieve a pid associated with the specified pidfd.
+ *
+ * @pidfd: The pidfd whose pid we want, or the fd of a /proc/<pid> file if
+ * @alloc_proc is also set.
+ * @allow_proc: If set, then an fd of a /proc/<pid> file can be passed instead
+ * of a pidfd, and this will be used to determine the pid.
+ * @flags: Output variable, if non-NULL, then the file->f_flags of the
+ * pidfd will be set here.
+ *
+ * Returns: If successful, the pid associated with the pidfd, otherwise an
+ * error.
+ */
+struct pid *__pidfd_get_pid(unsigned int pidfd, bool allow_proc,
+ unsigned int *flags);
+
+static inline struct pid *pidfd_get_pid(unsigned int pidfd, unsigned int *flags)
+{
+ return __pidfd_get_pid(pidfd, /* allow_proc = */ false, flags);
+}
+
+static inline struct pid *pidfd_get_pid_proc(unsigned int pidfd,
+ unsigned int *flags)
+{
+ return __pidfd_get_pid(pidfd, /* allow_proc = */ true, flags);
+}
+
struct pid *pidfd_pid(const struct file *file);
-struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags);
struct task_struct *pidfd_get_task(int pidfd, unsigned int *flags);
int pidfd_prepare(struct pid *pid, unsigned int flags, struct file **ret);
void do_notify_pidfd(struct task_struct *task);
diff --git a/kernel/pid.c b/kernel/pid.c
index 2715afb77eab..94c97559e5c5 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -36,6 +36,7 @@
#include <linux/pid_namespace.h>
#include <linux/init_task.h>
#include <linux/syscalls.h>
+#include <linux/proc_fs.h>
#include <linux/proc_ns.h>
#include <linux/refcount.h>
#include <linux/anon_inodes.h>
@@ -534,22 +535,32 @@ struct pid *find_ge_pid(int nr, struct pid_namespace *ns)
}
EXPORT_SYMBOL_GPL(find_ge_pid);
-struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags)
+struct pid *__pidfd_get_pid(unsigned int pidfd, bool allow_proc,
+ unsigned int *flags)
{
- struct fd f;
struct pid *pid;
+ struct fd f = fdget(pidfd);
+ struct file *file = fd_file(f);
- f = fdget(fd);
- if (!fd_file(f))
+ if (!file)
return ERR_PTR(-EBADF);
- pid = pidfd_pid(fd_file(f));
- if (!IS_ERR(pid)) {
- get_pid(pid);
- *flags = fd_file(f)->f_flags;
+ pid = pidfd_pid(file);
+ /* If we allow opening a pidfd via /proc/<pid>, do so. */
+ if (IS_ERR(pid) && allow_proc)
+ pid = tgid_pidfd_to_pid(file);
+
+ if (IS_ERR(pid)) {
+ fdput(f);
+ return pid;
}
+ /* Pin pid before we release fd. */
+ get_pid(pid);
+ if (flags)
+ *flags = file->f_flags;
fdput(f);
+
return pid;
}
@@ -747,23 +758,18 @@ SYSCALL_DEFINE3(pidfd_getfd, int, pidfd, int, fd,
unsigned int, flags)
{
struct pid *pid;
- struct fd f;
int ret;
/* flags is currently unused - make sure it's unset */
if (flags)
return -EINVAL;
- f = fdget(pidfd);
- if (!fd_file(f))
- return -EBADF;
-
- pid = pidfd_pid(fd_file(f));
+ pid = pidfd_get_pid(pidfd, NULL);
if (IS_ERR(pid))
- ret = PTR_ERR(pid);
- else
- ret = pidfd_getfd(pid, fd);
+ return PTR_ERR(pid);
- fdput(f);
+ ret = pidfd_getfd(pid, fd);
+
+ put_pid(pid);
return ret;
}
diff --git a/kernel/signal.c b/kernel/signal.c
index 4344860ffcac..9a35b1cf40ad 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3875,17 +3875,6 @@ static int copy_siginfo_from_user_any(kernel_siginfo_t *kinfo,
return copy_siginfo_from_user(kinfo, info);
}
-static struct pid *pidfd_to_pid(const struct file *file)
-{
- struct pid *pid;
-
- pid = pidfd_pid(file);
- if (!IS_ERR(pid))
- return pid;
-
- return tgid_pidfd_to_pid(file);
-}
-
#define PIDFD_SEND_SIGNAL_FLAGS \
(PIDFD_SIGNAL_THREAD | PIDFD_SIGNAL_THREAD_GROUP | \
PIDFD_SIGNAL_PROCESS_GROUP)
@@ -3908,10 +3897,10 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
siginfo_t __user *, info, unsigned int, flags)
{
int ret;
- struct fd f;
struct pid *pid;
kernel_siginfo_t kinfo;
enum pid_type type;
+ unsigned int f_flags;
/* Enforce flags be set to 0 until we add an extension. */
if (flags & ~PIDFD_SEND_SIGNAL_FLAGS)
@@ -3921,16 +3910,10 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
if (hweight32(flags & PIDFD_SEND_SIGNAL_FLAGS) > 1)
return -EINVAL;
- f = fdget(pidfd);
- if (!fd_file(f))
- return -EBADF;
-
/* Is this a pidfd? */
- pid = pidfd_to_pid(fd_file(f));
- if (IS_ERR(pid)) {
- ret = PTR_ERR(pid);
- goto err;
- }
+ pid = pidfd_get_pid_proc(pidfd, &f_flags);
+ if (IS_ERR(pid))
+ return PTR_ERR(pid);
ret = -EINVAL;
if (!access_pidfd_pidns(pid))
@@ -3939,7 +3922,7 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
switch (flags) {
case 0:
/* Infer scope from the type of pidfd. */
- if (fd_file(f)->f_flags & PIDFD_THREAD)
+ if (f_flags & PIDFD_THREAD)
type = PIDTYPE_PID;
else
type = PIDTYPE_TGID;
@@ -3978,7 +3961,7 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
else
ret = kill_pid_info_type(sig, &kinfo, pid, type);
err:
- fdput(f);
+ put_pid(pid);
return ret;
}
--
2.47.0
^ permalink raw reply related
* [PATCH v5 0/5] introduce PIDFD_SELF* sentinels
From: Lorenzo Stoakes @ 2024-10-25 9:41 UTC (permalink / raw)
To: Christian Brauner
Cc: Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, linux-kernel, Oliver Sang, John Hubbard
If you wish to utilise a pidfd interface to refer to the current process or
thread it is rather cumbersome, requiring something like:
int pidfd = pidfd_open(getpid(), 0 or PIDFD_THREAD);
...
close(pidfd);
Or the equivalent call opening /proc/self. It is more convenient to use a
sentinel value to indicate to an interface that accepts a pidfd that we
simply wish to refer to the current process thread.
This series introduces sentinels for this purposes which can be passed as
the pidfd in this instance rather than having to establish a dummy fd for
this purpose.
It is useful to refer to both the current thread from the userland's
perspective for which we use PIDFD_SELF, and the current process from the
userland's perspective, for which we use PIDFD_SELF_PROCESS.
There is unfortunately some confusion between the kernel and userland as to
what constitutes a process - a thread from the userland perspective is a
process in userland, and a userland process is a thread group (more
specifically the thread group leader from the kernel perspective). We
therefore alias things thusly:
* PIDFD_SELF_THREAD aliased by PIDFD_SELF - use PIDTYPE_PID.
* PIDFD_SELF_THREAD_GROUP alised by PIDFD_SELF_PROCESS - use PIDTYPE_TGID.
In all of the kernel code we refer to PIDFD_SELF_THREAD and
PIDFD_SELF_THREAD_GROUP. However we expect users to use PIDFD_SELF and
PIDFD_SELF_PROCESS.
This matters for cases where, for instance, a user unshare()'s FDs or does
thread-specific signal handling and where the user would be hugely confused
if the FDs referenced or signal processed referred to the thread group
leader rather than the individual thread.
We ensure that pidfd_send_signal() and pidfd_getfd() work correctly, and
assert as much in selftests. All other interfaces except setns() will work
implicitly with this new interface, however it doesn't make sense to test
waitid(P_PIDFD, ...) as waiting on ourselves is a blocking operation.
In the case of setns() we explicitly disallow use of PIDFD_SELF* as it
doesn't make sense to obtain the namespaces of our own process, and it
would require work to implement this functionality there that would be of
no use.
We also do not provide the ability to utilise PIDFD_SELF* in ordinary fd
operations such as open() or poll(), as this would require extensive work
and be of no real use.
v5:
* Fixup self test dependencies on pidfd/pidfd.h.
v4:
* Avoid returning an fd in the __pidfd_get_pid() function as pointed out by
Christian, instead simply always pin the pid and maintain fd scope in the
helper alone.
* Add wrapper header file in tools/include/linux to allow for import of
UAPI pidfd.h header without encountering the collision between system
fcntl.h and linux/fcntl.h as discussed with Shuah and John.
* Fixup tests to import the UAPI pidfd.h header working around conflicts
between system fcntl.h and linux/fcntl.h which the UAPI pidfd.h imports,
as reported by Shuah.
* Use an int for pidfd_is_self_sentinel() to avoid any dependency on
stdbool.h in userland.
https://lore.kernel.org/linux-mm/cover.1729198898.git.lorenzo.stoakes@oracle.com/
v3:
* Do not fput() an invalid fd as reported by kernel test bot.
* Fix unintended churn from moving variable declaration.
https://lore.kernel.org/linux-mm/cover.1729073310.git.lorenzo.stoakes@oracle.com/
v2:
* Fix tests as reported by Shuah.
* Correct RFC version lore link.
https://lore.kernel.org/linux-mm/cover.1728643714.git.lorenzo.stoakes@oracle.com/
Non-RFC v1:
* Removed RFC tag - there seems to be general consensus that this change is
a good idea, but perhaps some debate to be had on implementation. It
seems sensible then to move forward with the RFC flag removed.
* Introduced PIDFD_SELF_THREAD, PIDFD_SELF_THREAD_GROUP and their aliases
PIDFD_SELF and PIDFD_SELF_PROCESS respectively.
* Updated testing accordingly.
https://lore.kernel.org/linux-mm/cover.1728578231.git.lorenzo.stoakes@oracle.com/
RFC version:
https://lore.kernel.org/linux-mm/cover.1727644404.git.lorenzo.stoakes@oracle.com/
Lorenzo Stoakes (5):
pidfd: extend pidfd_get_pid() and de-duplicate pid lookup
pidfd: add PIDFD_SELF_* sentinels to refer to own thread/process
tools: testing: separate out wait_for_pid() into helper header
selftests: pidfd: add pidfd.h UAPI wrapper
selftests: pidfd: add tests for PIDFD_SELF_*
include/linux/pid.h | 34 ++++-
include/uapi/linux/pidfd.h | 15 ++
kernel/exit.c | 3 +-
kernel/nsproxy.c | 1 +
kernel/pid.c | 65 +++++---
kernel/signal.c | 29 +---
tools/include/linux/pidfd.h | 14 ++
tools/testing/selftests/cgroup/test_kill.c | 2 +-
.../pid_namespace/regression_enomem.c | 2 +-
tools/testing/selftests/pidfd/Makefile | 3 +-
tools/testing/selftests/pidfd/pidfd.h | 28 +---
.../selftests/pidfd/pidfd_getfd_test.c | 141 ++++++++++++++++++
tools/testing/selftests/pidfd/pidfd_helpers.h | 39 +++++
.../selftests/pidfd/pidfd_setns_test.c | 11 ++
tools/testing/selftests/pidfd/pidfd_test.c | 76 ++++++++--
15 files changed, 375 insertions(+), 88 deletions(-)
create mode 100644 tools/include/linux/pidfd.h
create mode 100644 tools/testing/selftests/pidfd/pidfd_helpers.h
--
2.47.0
^ permalink raw reply
* [PATCH 3/6] futex: Propagate flags into futex_get_value_locked()
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong
In-Reply-To: <20241025090347.244183920@infradead.org>
In order to facilitate variable sized futexes propagate the flags into
futex_get_value_locked().
No functional change intended.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
---
kernel/futex/core.c | 4 ++--
kernel/futex/futex.h | 2 +-
kernel/futex/pi.c | 8 ++++----
kernel/futex/requeue.c | 4 ++--
kernel/futex/waitwake.c | 4 ++--
5 files changed, 11 insertions(+), 11 deletions(-)
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -528,12 +528,12 @@ int futex_cmpxchg_value_locked(u32 *curv
return ret;
}
-int futex_get_value_locked(u32 *dest, u32 __user *from)
+int futex_get_value_locked(u32 *dest, u32 __user *from, unsigned int flags)
{
int ret;
pagefault_disable();
- ret = __get_user(*dest, from);
+ ret = futex_get_value(dest, from, flags);
pagefault_enable();
return ret ? -EFAULT : 0;
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -239,7 +239,7 @@ extern void futex_wake_mark(struct wake_
extern int fault_in_user_writeable(u32 __user *uaddr);
extern int futex_cmpxchg_value_locked(u32 *curval, u32 __user *uaddr, u32 uval, u32 newval);
-extern int futex_get_value_locked(u32 *dest, u32 __user *from);
+extern int futex_get_value_locked(u32 *dest, u32 __user *from, unsigned int flags);
extern struct futex_q *futex_top_waiter(struct futex_hash_bucket *hb, union futex_key *key);
extern void __futex_unqueue(struct futex_q *q);
--- a/kernel/futex/pi.c
+++ b/kernel/futex/pi.c
@@ -240,7 +240,7 @@ static int attach_to_pi_state(u32 __user
* still is what we expect it to be, otherwise retry the entire
* operation.
*/
- if (futex_get_value_locked(&uval2, uaddr))
+ if (futex_get_value_locked(&uval2, uaddr, FLAGS_SIZE_32))
goto out_efault;
if (uval != uval2)
@@ -359,7 +359,7 @@ static int handle_exit_race(u32 __user *
* The same logic applies to the case where the exiting task is
* already gone.
*/
- if (futex_get_value_locked(&uval2, uaddr))
+ if (futex_get_value_locked(&uval2, uaddr, FLAGS_SIZE_32))
return -EFAULT;
/* If the user space value has changed, try again. */
@@ -527,7 +527,7 @@ int futex_lock_pi_atomic(u32 __user *uad
* Read the user space value first so we can validate a few
* things before proceeding further.
*/
- if (futex_get_value_locked(&uval, uaddr))
+ if (futex_get_value_locked(&uval, uaddr, FLAGS_SIZE_32))
return -EFAULT;
if (unlikely(should_fail_futex(true)))
@@ -750,7 +750,7 @@ static int __fixup_pi_state_owner(u32 __
if (!pi_state->owner)
newtid |= FUTEX_OWNER_DIED;
- err = futex_get_value_locked(&uval, uaddr);
+ err = futex_get_value_locked(&uval, uaddr, FLAGS_SIZE_32);
if (err)
goto handle_err;
--- a/kernel/futex/requeue.c
+++ b/kernel/futex/requeue.c
@@ -275,7 +275,7 @@ futex_proxy_trylock_atomic(u32 __user *p
u32 curval;
int ret;
- if (futex_get_value_locked(&curval, pifutex))
+ if (futex_get_value_locked(&curval, pifutex, FLAGS_SIZE_32))
return -EFAULT;
if (unlikely(should_fail_futex(true)))
@@ -453,7 +453,7 @@ int futex_requeue(u32 __user *uaddr1, un
if (likely(cmpval != NULL)) {
u32 curval;
- ret = futex_get_value_locked(&curval, uaddr1);
+ ret = futex_get_value_locked(&curval, uaddr1, FLAGS_SIZE_32);
if (unlikely(ret)) {
double_unlock_hb(hb1, hb2);
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -453,7 +453,7 @@ int futex_wait_multiple_setup(struct fut
u32 val = vs[i].w.val;
hb = futex_q_lock(q);
- ret = futex_get_value_locked(&uval, uaddr);
+ ret = futex_get_value_locked(&uval, uaddr, FLAGS_SIZE_32);
if (!ret && uval == val) {
/*
@@ -621,7 +621,7 @@ int futex_wait_setup(u32 __user *uaddr,
retry_private:
*hb = futex_q_lock(q);
- ret = futex_get_value_locked(&uval, uaddr);
+ ret = futex_get_value_locked(&uval, uaddr, FLAGS_SIZE_32);
if (ret) {
futex_q_unlock(*hb);
^ permalink raw reply
* [PATCH 6/6] futex,selftests: Extend the futex selftests for NUMA
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong
In-Reply-To: <20241025090347.244183920@infradead.org>
XXX
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/testing/selftests/futex/functional/Makefile | 3
tools/testing/selftests/futex/functional/futex_numa.c | 262 ++++++++++++++++++
2 files changed, 264 insertions(+), 1 deletion(-)
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -17,7 +17,8 @@ TEST_GEN_PROGS := \
futex_wait_private_mapped_file \
futex_wait \
futex_requeue \
- futex_waitv
+ futex_waitv \
+ futex_numa
TEST_PROGS := run.sh
--- /dev/null
+++ b/tools/testing/selftests/futex/functional/futex_numa.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <pthread.h>
+#include <sys/shm.h>
+#include <sys/mman.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <time.h>
+#include <assert.h>
+#include "logging.h"
+#include "futextest.h"
+#include "futex2test.h"
+
+typedef u_int32_t u32;
+typedef int32_t s32;
+typedef u_int64_t u64;
+
+static int fflags = (FUTEX2_SIZE_U32 | FUTEX2_PRIVATE);
+static int fnode = FUTEX_NO_NODE;
+
+/* fairly stupid test-and-set lock with a waiter flag */
+
+#define N_LOCK 0x0000001
+#define N_WAITERS 0x0001000
+
+struct futex_numa_32 {
+ union {
+ u64 full;
+ struct {
+ u32 val;
+ u32 node;
+ };
+ };
+};
+
+void futex_numa_32_lock(struct futex_numa_32 *lock)
+{
+ for (;;) {
+ struct futex_numa_32 new, old = {
+ .full = __atomic_load_n(&lock->full, __ATOMIC_RELAXED),
+ };
+
+ for (;;) {
+ new = old;
+ if (old.val == 0) {
+ /* no waiter, no lock -> first lock, set no-node */
+ new.node = fnode;
+ }
+ if (old.val & N_LOCK) {
+ /* contention, set waiter */
+ new.val |= N_WAITERS;
+ }
+ new.val |= N_LOCK;
+
+ /* nothing changed, ready to block */
+ if (old.full == new.full)
+ break;
+
+ /*
+ * Use u64 cmpxchg to set the futex value and node in a
+ * consistent manner.
+ */
+ if (__atomic_compare_exchange_n(&lock->full,
+ &old.full, new.full,
+ /* .weak */ false,
+ __ATOMIC_ACQUIRE,
+ __ATOMIC_RELAXED)) {
+
+ /* if we just set N_LOCK, we own it */
+ if (!(old.val & N_LOCK))
+ return;
+
+ /* go block */
+ break;
+ }
+ }
+
+ futex2_wait(lock, new.val, ~0U, fflags, NULL, 0);
+ }
+}
+
+void futex_numa_32_unlock(struct futex_numa_32 *lock)
+{
+ u32 val = __atomic_sub_fetch(&lock->val, N_LOCK, __ATOMIC_RELEASE);
+ assert((s32)val >= 0);
+ if (val & N_WAITERS) {
+ int woken = futex2_wake(lock, ~0U, 1, fflags);
+ assert(val == N_WAITERS);
+ if (!woken) {
+ __atomic_compare_exchange_n(&lock->val, &val, 0U,
+ false, __ATOMIC_RELAXED,
+ __ATOMIC_RELAXED);
+ }
+ }
+}
+
+static long nanos = 50000;
+
+struct thread_args {
+ pthread_t tid;
+ volatile int * done;
+ struct futex_numa_32 *lock;
+ int val;
+ int *val1, *val2;
+ int node;
+};
+
+static void *threadfn(void *_arg)
+{
+ struct thread_args *args = _arg;
+ struct timespec ts = {
+ .tv_nsec = nanos,
+ };
+ int node;
+
+ while (!*args->done) {
+
+ futex_numa_32_lock(args->lock);
+ args->val++;
+
+ assert(*args->val1 == *args->val2);
+ (*args->val1)++;
+ nanosleep(&ts, NULL);
+ (*args->val2)++;
+
+ node = args->lock->node;
+ futex_numa_32_unlock(args->lock);
+
+ if (node != args->node) {
+ args->node = node;
+ printf("node: %d\n", node);
+ }
+
+ nanosleep(&ts, NULL);
+ }
+
+ return NULL;
+}
+
+static void *contendfn(void *_arg)
+{
+ struct thread_args *args = _arg;
+
+ while (!*args->done) {
+ /*
+ * futex2_wait() will take hb-lock, verify *var == val and
+ * queue/abort. By knowingly setting val 'wrong' this will
+ * abort and thereby generate hb-lock contention.
+ */
+ futex2_wait(&args->lock->val, ~0U, ~0U, fflags, NULL, 0);
+ args->val++;
+ }
+
+ return NULL;
+}
+
+static volatile int done = 0;
+static struct futex_numa_32 lock = { .val = 0, };
+static int val1, val2;
+
+int main(int argc, char *argv[])
+{
+ struct thread_args *tas[512], *cas[512];
+ int c, t, threads = 2, contenders = 0;
+ int sleeps = 10;
+ int total = 0;
+
+ while ((c = getopt(argc, argv, "c:t:s:n:N::")) != -1) {
+ switch (c) {
+ case 'c':
+ contenders = atoi(optarg);
+ break;
+ case 't':
+ threads = atoi(optarg);
+ break;
+ case 's':
+ sleeps = atoi(optarg);
+ break;
+ case 'n':
+ nanos = atoi(optarg);
+ break;
+ case 'N':
+ fflags |= FUTEX2_NUMA;
+ if (optarg)
+ fnode = atoi(optarg);
+ break;
+ default:
+ exit(1);
+ break;
+ }
+ }
+
+ for (t = 0; t < contenders; t++) {
+ struct thread_args *args = calloc(1, sizeof(*args));
+ if (!args) {
+ perror("thread_args");
+ exit(-1);
+ }
+
+ args->done = &done;
+ args->lock = &lock;
+ args->val1 = &val1;
+ args->val2 = &val2;
+ args->node = -1;
+
+ if (pthread_create(&args->tid, NULL, contendfn, args)) {
+ perror("pthread_create");
+ exit(-1);
+ }
+
+ cas[t] = args;
+ }
+
+ for (t = 0; t < threads; t++) {
+ struct thread_args *args = calloc(1, sizeof(*args));
+ if (!args) {
+ perror("thread_args");
+ exit(-1);
+ }
+
+ args->done = &done;
+ args->lock = &lock;
+ args->val1 = &val1;
+ args->val2 = &val2;
+ args->node = -1;
+
+ if (pthread_create(&args->tid, NULL, threadfn, args)) {
+ perror("pthread_create");
+ exit(-1);
+ }
+
+ tas[t] = args;
+ }
+
+ sleep(sleeps);
+
+ done = true;
+
+ for (t = 0; t < threads; t++) {
+ struct thread_args *args = tas[t];
+
+ pthread_join(args->tid, NULL);
+ total += args->val;
+// printf("tval: %d\n", args->val);
+ }
+ printf("total: %d\n", total);
+
+ if (contenders) {
+ total = 0;
+ for (t = 0; t < contenders; t++) {
+ struct thread_args *args = cas[t];
+
+ pthread_join(args->tid, NULL);
+ total += args->val;
+ // printf("tval: %d\n", args->val);
+ }
+ printf("contenders: %d\n", total);
+ }
+
+ return 0;
+}
+
^ permalink raw reply
* [PATCH 4/6] futex: Enable FUTEX2_{8,16}
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong
In-Reply-To: <20241025090347.244183920@infradead.org>
When futexes are no longer u32 aligned, the lower offset bits are no
longer available to put type info in. However, since offset is the
offset within a page, there are plenty bits available on the top end.
After that, pass flags into futex_get_value_locked() for WAIT and
disallow FUTEX2_SIZE_U64 instead of mandating FUTEX2_SIZE_U32.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/futex.h | 11 ++++++-----
kernel/futex/core.c | 9 +++++++++
kernel/futex/futex.h | 4 ++--
kernel/futex/waitwake.c | 5 +++--
4 files changed, 20 insertions(+), 9 deletions(-)
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -16,18 +16,19 @@ struct task_struct;
* The key type depends on whether it's a shared or private mapping.
* Don't rearrange members without looking at hash_futex().
*
- * offset is aligned to a multiple of sizeof(u32) (== 4) by definition.
- * We use the two low order bits of offset to tell what is the kind of key :
+ * offset is the position within a page and is in the range [0, PAGE_SIZE).
+ * The high bits of the offset indicate what kind of key this is:
* 00 : Private process futex (PTHREAD_PROCESS_PRIVATE)
* (no reference on an inode or mm)
* 01 : Shared futex (PTHREAD_PROCESS_SHARED)
* mapped on a file (reference on the underlying inode)
* 10 : Shared futex (PTHREAD_PROCESS_SHARED)
* (but private mapping on an mm, and reference taken on it)
-*/
+ */
-#define FUT_OFF_INODE 1 /* We set bit 0 if key has a reference on inode */
-#define FUT_OFF_MMSHARED 2 /* We set bit 1 if key has a reference on mm */
+#define FUT_OFF_INODE (PAGE_SIZE << 0)
+#define FUT_OFF_MMSHARED (PAGE_SIZE << 1)
+#define FUT_OFF_SIZE (PAGE_SIZE << 2)
union futex_key {
struct {
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -313,6 +313,15 @@ int get_futex_key(void __user *uaddr, un
}
/*
+ * Encode the futex size in the offset. This makes cross-size
+ * wake-wait fail -- see futex_match().
+ *
+ * NOTE that cross-size wake-wait is fundamentally broken wrt
+ * FLAGS_NUMA.
+ */
+ key->both.offset |= FUT_OFF_SIZE * (flags & FLAGS_SIZE_MASK);
+
+ /*
* PROCESS_PRIVATE futexes are fast.
* As the mm cannot disappear under us and the 'key' only needs
* virtual address, we dont even have to find the underlying vma.
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -81,8 +81,8 @@ static inline bool futex_flags_valid(uns
return false;
}
- /* Only 32bit futexes are implemented -- for now */
- if ((flags & FLAGS_SIZE_MASK) != FLAGS_SIZE_32)
+ /* 64bit futexes aren't implemented -- yet */
+ if ((flags & FLAGS_SIZE_MASK) == FLAGS_SIZE_64)
return false;
/*
--- a/kernel/futex/waitwake.c
+++ b/kernel/futex/waitwake.c
@@ -449,11 +449,12 @@ int futex_wait_multiple_setup(struct fut
for (i = 0; i < count; i++) {
u32 __user *uaddr = (u32 __user *)(unsigned long)vs[i].w.uaddr;
+ unsigned int flags = vs[i].w.flags;
struct futex_q *q = &vs[i].q;
u32 val = vs[i].w.val;
hb = futex_q_lock(q);
- ret = futex_get_value_locked(&uval, uaddr, FLAGS_SIZE_32);
+ ret = futex_get_value_locked(&uval, uaddr, flags);
if (!ret && uval == val) {
/*
@@ -621,7 +622,7 @@ int futex_wait_setup(u32 __user *uaddr,
retry_private:
*hb = futex_q_lock(q);
- ret = futex_get_value_locked(&uval, uaddr, FLAGS_SIZE_32);
+ ret = futex_get_value_locked(&uval, uaddr, flags);
if (ret) {
futex_q_unlock(*hb);
^ permalink raw reply
* [PATCH 0/6] futex: The remaining futex2 bits
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong
Hi!
By popular demand a repost of the remaining futex2 bits.
Notably this is the FUTEX2_NUMA and FUTEX2_{8,16} 'small' futex support.
I'm not sure how much demand there actually is, since back-channels and
whispers don't really count. So for those of you out there on the big wide
interweb, if you want this, respond with Tested-by tags.
I know Christoph wants the NUMA bits, so I suppose that is at least one user
for this new interface, and that might see it through, but more interested
parties would be better and certainly move things along faster.
Same goes for the small futex bits, if nobody replies, they're going to get
left behind -- again!
^ permalink raw reply
* [PATCH 5/6] futex,selftests: Extend the futex selftests
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong
In-Reply-To: <20241025090347.244183920@infradead.org>
Extend the wait/requeue selftests to also cover the futex2 syscalls.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
tools/testing/selftests/futex/functional/futex_requeue.c | 100 +++++++++-
tools/testing/selftests/futex/functional/futex_wait.c | 56 ++++-
tools/testing/selftests/futex/functional/futex_wait_timeout.c | 16 +
tools/testing/selftests/futex/functional/futex_wait_wouldblock.c | 28 ++
tools/testing/selftests/futex/functional/futex_waitv.c | 15 -
tools/testing/selftests/futex/functional/run.sh | 6
tools/testing/selftests/futex/include/futex2test.h | 52 +++++
7 files changed, 243 insertions(+), 30 deletions(-)
--- a/tools/testing/selftests/futex/functional/futex_requeue.c
+++ b/tools/testing/selftests/futex/functional/futex_requeue.c
@@ -7,8 +7,10 @@
#include <pthread.h>
#include <limits.h>
+#include <stdbool.h>
#include "logging.h"
#include "futextest.h"
+#include "futex2test.h"
#define TEST_NAME "futex-requeue"
#define timeout_ns 30000000
@@ -16,24 +18,58 @@
volatile futex_t *f1;
+bool futex2 = 0;
+bool mixed = 0;
+
void usage(char *prog)
{
printf("Usage: %s\n", prog);
printf(" -c Use color\n");
+ printf(" -n Use futex2 interface\n");
+ printf(" -x Use mixed size futex\n");
printf(" -h Display this help message\n");
printf(" -v L Verbosity level: %d=QUIET %d=CRITICAL %d=INFO\n",
VQUIET, VCRITICAL, VINFO);
}
-void *waiterfn(void *arg)
+static void *waiterfn(void *arg)
{
+ unsigned int flags = 0;
struct timespec to;
- to.tv_sec = 0;
- to.tv_nsec = timeout_ns;
+ if (futex2) {
+ unsigned long mask;
+
+ if (clock_gettime(CLOCK_MONOTONIC, &to)) {
+ printf("clock_gettime() failed errno %d", errno);
+ return NULL;
+ }
+
+ to.tv_nsec += timeout_ns;
+ if (to.tv_nsec >= 1000000000) {
+ to.tv_sec++;
+ to.tv_nsec -= 1000000000;
+ }
+
+ if (mixed) {
+ flags |= FUTEX2_SIZE_U16;
+ mask = (unsigned short)(~0U);
+ } else {
+ flags |= FUTEX2_SIZE_U32;
+ mask = (unsigned int)(~0U);
+ }
+
+ if (futex2_wait(f1, *f1, mask, flags,
+ &to, CLOCK_MONOTONIC))
+ printf("waiter failed errno %d\n", errno);
+ } else {
+
+ to.tv_sec = 0;
+ to.tv_nsec = timeout_ns;
- if (futex_wait(f1, *f1, &to, 0))
- printf("waiter failed errno %d\n", errno);
+ if (futex_wait(f1, *f1, &to, flags))
+ printf("waiter failed errno %d\n", errno);
+ }
return NULL;
}
@@ -48,7 +84,7 @@ int main(int argc, char *argv[])
f1 = &_f1;
- while ((c = getopt(argc, argv, "cht:v:")) != -1) {
+ while ((c = getopt(argc, argv, "xncht:v:")) != -1) {
switch (c) {
case 'c':
log_color(1);
@@ -59,6 +95,12 @@ int main(int argc, char *argv[])
case 'v':
log_verbosity(atoi(optarg));
break;
+ case 'x':
+ mixed=1;
+ /* fallthrough */
+ case 'n':
+ futex2=1;
+ break;
default:
usage(basename(argv[0]));
exit(1);
@@ -79,7 +121,22 @@ int main(int argc, char *argv[])
usleep(WAKE_WAIT_US);
info("Requeuing 1 futex from f1 to f2\n");
- res = futex_cmp_requeue(f1, 0, &f2, 0, 1, 0);
+ if (futex2) {
+ struct futex_waitv futexes[2] = {
+ {
+ .val = 0,
+ .uaddr = (unsigned long)f1,
+ .flags = mixed ? FUTEX2_SIZE_U16 : FUTEX2_SIZE_U32,
+ },
+ {
+ .uaddr = (unsigned long)&f2,
+ .flags = FUTEX2_SIZE_U32,
+ },
+ };
+ res = futex2_requeue(futexes, 0, 0, 1);
+ } else {
+ res = futex_cmp_requeue(f1, 0, &f2, 0, 1, 0);
+ }
if (res != 1) {
ksft_test_result_fail("futex_requeue simple returned: %d %s\n",
res ? errno : res,
@@ -89,7 +146,11 @@ int main(int argc, char *argv[])
info("Waking 1 futex at f2\n");
- res = futex_wake(&f2, 1, 0);
+ if (futex2) {
+ res = futex2_wake(&f2, ~0U, 1, FUTEX2_SIZE_U32);
+ } else {
+ res = futex_wake(&f2, 1, 0);
+ }
if (res != 1) {
ksft_test_result_fail("futex_requeue simple returned: %d %s\n",
res ? errno : res,
@@ -112,7 +173,22 @@ int main(int argc, char *argv[])
usleep(WAKE_WAIT_US);
info("Waking 3 futexes at f1 and requeuing 7 futexes from f1 to f2\n");
- res = futex_cmp_requeue(f1, 0, &f2, 3, 7, 0);
+ if (futex2) {
+ struct futex_waitv futexes[2] = {
+ {
+ .val = 0,
+ .uaddr = (unsigned long)f1,
+ .flags = mixed ? FUTEX2_SIZE_U16 : FUTEX2_SIZE_U32,
+ },
+ {
+ .uaddr = (unsigned long)&f2,
+ .flags = FUTEX2_SIZE_U32,
+ },
+ };
+ res = futex2_requeue(futexes, 0, 3, 7);
+ } else {
+ res = futex_cmp_requeue(f1, 0, &f2, 3, 7, 0);
+ }
if (res != 10) {
ksft_test_result_fail("futex_requeue many returned: %d %s\n",
res ? errno : res,
@@ -121,7 +197,11 @@ int main(int argc, char *argv[])
}
info("Waking INT_MAX futexes at f2\n");
- res = futex_wake(&f2, INT_MAX, 0);
+ if (futex2) {
+ res = futex2_wake(&f2, ~0U, INT_MAX, FUTEX2_SIZE_U32);
+ } else {
+ res = futex_wake(&f2, INT_MAX, 0);
+ }
if (res != 7) {
ksft_test_result_fail("futex_requeue many returned: %d %s\n",
res ? errno : res,
--- a/tools/testing/selftests/futex/functional/futex_wait.c
+++ b/tools/testing/selftests/futex/functional/futex_wait.c
@@ -9,8 +9,10 @@
#include <sys/shm.h>
#include <sys/mman.h>
#include <fcntl.h>
+#include <stdbool.h>
#include "logging.h"
#include "futextest.h"
+#include "futex2test.h"
#define TEST_NAME "futex-wait"
#define timeout_ns 30000000
@@ -19,10 +21,13 @@
void *futex;
+bool futex2 = 0;
+
void usage(char *prog)
{
printf("Usage: %s\n", prog);
printf(" -c Use color\n");
+ printf(" -n Use futex2 interface\n");
printf(" -h Display this help message\n");
printf(" -v L Verbosity level: %d=QUIET %d=CRITICAL %d=INFO\n",
VQUIET, VCRITICAL, VINFO);
@@ -30,17 +35,35 @@ void usage(char *prog)
static void *waiterfn(void *arg)
{
- struct timespec to;
unsigned int flags = 0;
+ struct timespec to;
if (arg)
flags = *((unsigned int *) arg);
- to.tv_sec = 0;
- to.tv_nsec = timeout_ns;
+ if (futex2) {
+ if (clock_gettime(CLOCK_MONOTONIC, &to)) {
+ printf("clock_gettime() failed errno %d", errno);
+ return NULL;
+ }
- if (futex_wait(futex, 0, &to, flags))
- printf("waiter failed errno %d\n", errno);
+ to.tv_nsec += timeout_ns;
+ if (to.tv_nsec >= 1000000000) {
+ to.tv_sec++;
+ to.tv_nsec -= 1000000000;
+ }
+
+ if (futex2_wait(futex, 0, ~0U, flags | FUTEX2_SIZE_U32,
+ &to, CLOCK_MONOTONIC))
+ printf("waiter failed errno %d\n", errno);
+ } else {
+
+ to.tv_sec = 0;
+ to.tv_nsec = timeout_ns;
+
+ if (futex_wait(futex, 0, &to, flags))
+ printf("waiter failed errno %d\n", errno);
+ }
return NULL;
}
@@ -55,7 +78,7 @@ int main(int argc, char *argv[])
futex = &f_private;
- while ((c = getopt(argc, argv, "cht:v:")) != -1) {
+ while ((c = getopt(argc, argv, "ncht:v:")) != -1) {
switch (c) {
case 'c':
log_color(1);
@@ -66,6 +89,9 @@ int main(int argc, char *argv[])
case 'v':
log_verbosity(atoi(optarg));
break;
+ case 'n':
+ futex2=1;
+ break;
default:
usage(basename(argv[0]));
exit(1);
@@ -84,7 +110,11 @@ int main(int argc, char *argv[])
usleep(WAKE_WAIT_US);
info("Calling private futex_wake on futex: %p\n", futex);
- res = futex_wake(futex, 1, FUTEX_PRIVATE_FLAG);
+ if (futex2) {
+ res = futex2_wake(futex, ~0U, 1, FUTEX2_SIZE_U32 | FUTEX2_PRIVATE);
+ } else {
+ res = futex_wake(futex, 1, FUTEX_PRIVATE_FLAG);
+ }
if (res != 1) {
ksft_test_result_fail("futex_wake private returned: %d %s\n",
errno, strerror(errno));
@@ -112,7 +142,11 @@ int main(int argc, char *argv[])
usleep(WAKE_WAIT_US);
info("Calling shared (page anon) futex_wake on futex: %p\n", futex);
- res = futex_wake(futex, 1, 0);
+ if (futex2) {
+ res = futex2_wake(futex, ~0U, 1, FUTEX2_SIZE_U32);
+ } else {
+ res = futex_wake(futex, 1, 0);
+ }
if (res != 1) {
ksft_test_result_fail("futex_wake shared (page anon) returned: %d %s\n",
errno, strerror(errno));
@@ -151,7 +185,11 @@ int main(int argc, char *argv[])
usleep(WAKE_WAIT_US);
info("Calling shared (file backed) futex_wake on futex: %p\n", futex);
- res = futex_wake(shm, 1, 0);
+ if (futex2) {
+ res = futex2_wake(shm, ~0U, 1, FUTEX2_SIZE_U32);
+ } else {
+ res = futex_wake(shm, 1, 0);
+ }
if (res != 1) {
ksft_test_result_fail("futex_wake shared (file backed) returned: %d %s\n",
errno, strerror(errno));
--- a/tools/testing/selftests/futex/functional/futex_wait_timeout.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_timeout.c
@@ -103,7 +103,7 @@ int main(int argc, char *argv[])
struct futex_waitv waitv = {
.uaddr = (uintptr_t)&f1,
.val = f1,
- .flags = FUTEX_32,
+ .flags = FUTEX2_SIZE_U32,
.__reserved = 0
};
@@ -128,7 +128,7 @@ int main(int argc, char *argv[])
}
ksft_print_header();
- ksft_set_plan(9);
+ ksft_set_plan(11);
ksft_print_msg("%s: Block on a futex and wait for timeout\n",
basename(argv[0]));
ksft_print_msg("\tArguments: timeout=%ldns\n", timeout_ns);
@@ -201,6 +201,18 @@ int main(int argc, char *argv[])
res = futex_waitv(&waitv, 1, 0, &to, CLOCK_REALTIME);
test_timeout(res, &ret, "futex_waitv realtime", ETIMEDOUT);
+ /* futex2_wait with CLOCK_MONOTONIC */
+ if (futex_get_abs_timeout(CLOCK_MONOTONIC, &to, timeout_ns))
+ return RET_FAIL;
+ res = futex2_wait(&f1, f1, 1, FUTEX2_SIZE_U32, &to, CLOCK_MONOTONIC);
+ test_timeout(res, &ret, "futex2_wait monotonic", ETIMEDOUT);
+
+ /* futex2_wait with CLOCK_REALTIME */
+ if (futex_get_abs_timeout(CLOCK_REALTIME, &to, timeout_ns))
+ return RET_FAIL;
+ res = futex2_wait(&f1, f1, 1, FUTEX2_SIZE_U32, &to, CLOCK_REALTIME);
+ test_timeout(res, &ret, "futex2_wait realtime", ETIMEDOUT);
+
ksft_print_cnts();
return ret;
}
--- a/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
+++ b/tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
@@ -46,7 +46,7 @@ int main(int argc, char *argv[])
struct futex_waitv waitv = {
.uaddr = (uintptr_t)&f1,
.val = f1+1,
- .flags = FUTEX_32,
+ .flags = FUTEX2_SIZE_U32 | FUTEX2_PRIVATE,
.__reserved = 0
};
@@ -68,7 +68,7 @@ int main(int argc, char *argv[])
}
ksft_print_header();
- ksft_set_plan(2);
+ ksft_set_plan(3);
ksft_print_msg("%s: Test the unexpected futex value in FUTEX_WAIT\n",
basename(argv[0]));
@@ -106,6 +106,30 @@ int main(int argc, char *argv[])
ksft_test_result_pass("futex_waitv\n");
}
+ if (clock_gettime(CLOCK_MONOTONIC, &to)) {
+ error("clock_gettime failed\n", errno);
+ return errno;
+ }
+
+ to.tv_nsec += timeout_ns;
+
+ if (to.tv_nsec >= 1000000000) {
+ to.tv_sec++;
+ to.tv_nsec -= 1000000000;
+ }
+
+ info("Calling futex2_wait on f1: %u @ %p with val=%u\n", f1, &f1, f1+1);
+ res = futex2_wait(&f1, f1+1, ~0U, FUTEX2_SIZE_U32 | FUTEX2_PRIVATE,
+ &to, CLOCK_MONOTONIC);
+ if (!res || errno != EWOULDBLOCK) {
+ ksft_test_result_pass("futex2_wait returned: %d %s\n",
+ res ? errno : res,
+ res ? strerror(errno) : "");
+ ret = RET_FAIL;
+ } else {
+ ksft_test_result_pass("futex2_wait\n");
+ }
+
ksft_print_cnts();
return ret;
}
--- a/tools/testing/selftests/futex/functional/futex_waitv.c
+++ b/tools/testing/selftests/futex/functional/futex_waitv.c
@@ -88,7 +88,7 @@ int main(int argc, char *argv[])
for (i = 0; i < NR_FUTEXES; i++) {
waitv[i].uaddr = (uintptr_t)&futexes[i];
- waitv[i].flags = FUTEX_32 | FUTEX_PRIVATE_FLAG;
+ waitv[i].flags = FUTEX2_SIZE_U32 | FUTEX2_PRIVATE;
waitv[i].val = 0;
waitv[i].__reserved = 0;
}
@@ -99,7 +99,8 @@ int main(int argc, char *argv[])
usleep(WAKE_WAIT_US);
- res = futex_wake(u64_to_ptr(waitv[NR_FUTEXES - 1].uaddr), 1, FUTEX_PRIVATE_FLAG);
+ res = futex2_wake(u64_to_ptr(waitv[NR_FUTEXES - 1].uaddr), ~0U, 1,
+ FUTEX2_PRIVATE | FUTEX2_SIZE_U32);
if (res != 1) {
ksft_test_result_fail("futex_wake private returned: %d %s\n",
res ? errno : res,
@@ -122,7 +123,7 @@ int main(int argc, char *argv[])
*shared_data = 0;
waitv[i].uaddr = (uintptr_t)shared_data;
- waitv[i].flags = FUTEX_32;
+ waitv[i].flags = FUTEX2_SIZE_U32;
waitv[i].val = 0;
waitv[i].__reserved = 0;
}
@@ -145,8 +146,8 @@ int main(int argc, char *argv[])
for (i = 0; i < NR_FUTEXES; i++)
shmdt(u64_to_ptr(waitv[i].uaddr));
- /* Testing a waiter without FUTEX_32 flag */
- waitv[0].flags = FUTEX_PRIVATE_FLAG;
+ /* Testing a waiter without FUTEX2_SIZE_U32 flag */
+ waitv[0].flags = FUTEX2_PRIVATE;
if (clock_gettime(CLOCK_MONOTONIC, &to))
error("gettime64 failed\n", errno);
@@ -160,11 +161,11 @@ int main(int argc, char *argv[])
res ? strerror(errno) : "");
ret = RET_FAIL;
} else {
- ksft_test_result_pass("futex_waitv without FUTEX_32\n");
+ ksft_test_result_pass("futex_waitv without FUTEX2_SIZE_U32\n");
}
/* Testing a waiter with an unaligned address */
- waitv[0].flags = FUTEX_PRIVATE_FLAG | FUTEX_32;
+ waitv[0].flags = FUTEX2_PRIVATE | FUTEX2_SIZE_U32;
waitv[0].uaddr = 1;
if (clock_gettime(CLOCK_MONOTONIC, &to))
--- a/tools/testing/selftests/futex/functional/run.sh
+++ b/tools/testing/selftests/futex/functional/run.sh
@@ -76,9 +76,15 @@ echo
echo
./futex_wait $COLOR
+echo
+./futex_wait -n $COLOR
echo
./futex_requeue $COLOR
+echo
+./futex_requeue -n $COLOR
+echo
+./futex_requeue -x $COLOR
echo
./futex_waitv $COLOR
--- a/tools/testing/selftests/futex/include/futex2test.h
+++ b/tools/testing/selftests/futex/include/futex2test.h
@@ -8,6 +8,41 @@
#define u64_to_ptr(x) ((void *)(uintptr_t)(x))
+#ifndef __NR_futex_waitv
+#define __NR_futex_waitv 449
+
+struct futex_waitv {
+ __u64 val;
+ __u64 uaddr;
+ __u32 flags;
+ __u32 __reserved;
+};
+#endif
+
+#ifndef __NR_futex_wake
+#define __NR_futex_wake 454
+#define __NR_futex_wait 455
+#define __NR_futex_requeue 456
+#endif
+
+#ifndef FUTEX2_SIZE_U8
+/*
+ * Flags for futex2 syscalls.
+ */
+#define FUTEX2_SIZE_U8 0x00
+#define FUTEX2_SIZE_U16 0x01
+#define FUTEX2_SIZE_U32 0x02
+#define FUTEX2_SIZE_U64 0x03
+#define FUTEX2_NUMA 0x04
+ /* 0x08 */
+ /* 0x10 */
+ /* 0x20 */
+ /* 0x40 */
+#define FUTEX2_PRIVATE FUTEX_PRIVATE_FLAG
+#endif
+
+#define FUTEX_NO_NODE (-1)
+
/**
* futex_waitv - Wait at multiple futexes, wake on any
* @waiters: Array of waiters
@@ -20,3 +55,20 @@ static inline int futex_waitv(volatile s
{
return syscall(__NR_futex_waitv, waiters, nr_waiters, flags, timo, clockid);
}
+
+static inline int futex2_wake(volatile void *uaddr, unsigned long mask, int nr, unsigned int flags)
+{
+ return syscall(__NR_futex_wake, uaddr, mask, nr, flags);
+}
+
+static inline int futex2_wait(volatile void *uaddr, unsigned long val, unsigned long mask,
+ unsigned int flags, struct timespec *timo, clockid_t clockid)
+{
+ return syscall(__NR_futex_wait, uaddr, val, mask, flags, timo, clockid);
+}
+
+static inline int futex2_requeue(struct futex_waitv *futexes, unsigned int flags,
+ int nr_wake, int nr_requeue)
+{
+ return syscall(__NR_futex_requeue, futexes, flags, nr_wake, nr_requeue);
+}
^ permalink raw reply
* [PATCH 2/6] futex: Implement FUTEX2_NUMA
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong
In-Reply-To: <20241025090347.244183920@infradead.org>
Extend the futex2 interface to be numa aware.
When FUTEX2_NUMA is specified for a futex, the user value is extended
to two words (of the same size). The first is the user value we all
know, the second one will be the node to place this futex on.
struct futex_numa_32 {
u32 val;
u32 node;
};
When node is set to ~0, WAIT will set it to the current node_id such
that WAKE knows where to find it. If userspace corrupts the node value
between WAIT and WAKE, the futex will not be found and no wakeup will
happen.
When FUTEX2_NUMA is not set, the node is simply an extention of the
hash, such that traditional futexes are still interleaved over the
nodes.
This is done to avoid having to have a separate !numa hash-table.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/futex.h | 3 +
include/uapi/linux/futex.h | 8 ++
kernel/futex/core.c | 128 ++++++++++++++++++++++++++++++++++++---------
kernel/futex/futex.h | 17 +++++
4 files changed, 131 insertions(+), 25 deletions(-)
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -34,6 +34,7 @@ union futex_key {
u64 i_seq;
unsigned long pgoff;
unsigned int offset;
+ /* unsigned int node; */
} shared;
struct {
union {
@@ -42,11 +43,13 @@ union futex_key {
};
unsigned long address;
unsigned int offset;
+ /* unsigned int node; */
} private;
struct {
u64 ptr;
unsigned long word;
unsigned int offset;
+ unsigned int node; /* NOT hashed! */
} both;
};
--- a/include/uapi/linux/futex.h
+++ b/include/uapi/linux/futex.h
@@ -74,6 +74,14 @@
/* do not use */
#define FUTEX_32 FUTEX2_SIZE_U32 /* historical accident :-( */
+
+/*
+ * When FUTEX2_NUMA doubles the futex word, the second word is a node value.
+ * The special value -1 indicates no-node. This is the same value as
+ * NUMA_NO_NODE, except that value is not ABI, this is.
+ */
+#define FUTEX_NO_NODE (-1)
+
/*
* Max numbers of elements in a futex_waitv array
*/
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -36,7 +36,8 @@
#include <linux/pagemap.h>
#include <linux/debugfs.h>
#include <linux/plist.h>
-#include <linux/memblock.h>
+#include <linux/gfp.h>
+#include <linux/vmalloc.h>
#include <linux/fault-inject.h>
#include <linux/slab.h>
@@ -49,12 +50,14 @@
* reside in the same cacheline.
*/
static struct {
- struct futex_hash_bucket *queues;
unsigned long hashsize;
+ unsigned int hashshift;
+ struct futex_hash_bucket *queues[MAX_NUMNODES];
} __futex_data __read_mostly __aligned(2*sizeof(long));
-#define futex_queues (__futex_data.queues)
-#define futex_hashsize (__futex_data.hashsize)
+#define futex_hashsize (__futex_data.hashsize)
+#define futex_hashshift (__futex_data.hashshift)
+#define futex_queues (__futex_data.queues)
/*
* Fault injections for futexes.
@@ -107,6 +110,26 @@ late_initcall(fail_futex_debugfs);
#endif /* CONFIG_FAIL_FUTEX */
+static int futex_get_value(u32 *val, u32 __user *from, unsigned int flags)
+{
+ switch (futex_size(flags)) {
+ case 1: return __get_user(*val, (u8 __user *)from);
+ case 2: return __get_user(*val, (u16 __user *)from);
+ case 4: return __get_user(*val, (u32 __user *)from);
+ default: BUG();
+ }
+}
+
+static int futex_put_value(u32 val, u32 __user *to, unsigned int flags)
+{
+ switch (futex_size(flags)) {
+ case 1: return __put_user(val, (u8 __user *)to);
+ case 2: return __put_user(val, (u16 __user *)to);
+ case 4: return __put_user(val, (u32 __user *)to);
+ default: BUG();
+ }
+}
+
/**
* futex_hash - Return the hash bucket in the global hash
* @key: Pointer to the futex key for which the hash is calculated
@@ -116,10 +139,29 @@ late_initcall(fail_futex_debugfs);
*/
struct futex_hash_bucket *futex_hash(union futex_key *key)
{
- u32 hash = jhash2((u32 *)key, offsetof(typeof(*key), both.offset) / 4,
+ u32 hash = jhash2((u32 *)key,
+ offsetof(typeof(*key), both.offset) / sizeof(u32),
key->both.offset);
+ int node = key->both.node;
- return &futex_queues[hash & (futex_hashsize - 1)];
+ if (node == FUTEX_NO_NODE) {
+ /*
+ * In case of !FLAGS_NUMA, use some unused hash bits to pick a
+ * node -- this ensures regular futexes are interleaved across
+ * the nodes and avoids having to allocate multiple
+ * hash-tables.
+ *
+ * NOTE: this isn't perfectly uniform, but it is fast and
+ * handles sparse node masks.
+ */
+ node = (hash >> futex_hashshift) % nr_node_ids;
+ if (!node_possible(node)) {
+ node = find_next_bit_wrap(node_possible_map.bits,
+ nr_node_ids, node);
+ }
+ }
+
+ return &futex_queues[node][hash & (futex_hashsize - 1)];
}
@@ -219,7 +261,7 @@ static u64 get_inode_sequence_number(str
*
* lock_page() might sleep, the caller should not hold a spinlock.
*/
-int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
+int get_futex_key(void __user *uaddr, unsigned int flags, union futex_key *key,
enum futex_access rw)
{
unsigned long address = (unsigned long)uaddr;
@@ -227,25 +269,49 @@ int get_futex_key(u32 __user *uaddr, uns
struct page *page;
struct folio *folio;
struct address_space *mapping;
- int err, ro = 0;
+ int node, err, size, ro = 0;
bool fshared;
fshared = flags & FLAGS_SHARED;
+ size = futex_size(flags);
+ if (flags & FLAGS_NUMA)
+ size *= 2;
/*
* The futex address must be "naturally" aligned.
*/
key->both.offset = address % PAGE_SIZE;
- if (unlikely((address % sizeof(u32)) != 0))
+ if (unlikely((address % size) != 0))
return -EINVAL;
address -= key->both.offset;
- if (unlikely(!access_ok(uaddr, sizeof(u32))))
+ if (unlikely(!access_ok(uaddr, size)))
return -EFAULT;
if (unlikely(should_fail_futex(fshared)))
return -EFAULT;
+ if (flags & FLAGS_NUMA) {
+ void __user *naddr = uaddr + size / 2;
+
+ if (futex_get_value(&node, naddr, flags))
+ return -EFAULT;
+
+ if (node == FUTEX_NO_NODE) {
+ node = numa_node_id();
+ if (futex_put_value(node, naddr, flags))
+ return -EFAULT;
+
+ } else if (node >= MAX_NUMNODES || !node_possible(node)) {
+ return -EINVAL;
+ }
+
+ key->both.node = node;
+
+ } else {
+ key->both.node = FUTEX_NO_NODE;
+ }
+
/*
* PROCESS_PRIVATE futexes are fast.
* As the mm cannot disappear under us and the 'key' only needs
@@ -1148,26 +1214,42 @@ void futex_exit_release(struct task_stru
static int __init futex_init(void)
{
- unsigned int futex_shift;
- unsigned long i;
+ unsigned int order, n;
+ unsigned long size, i;
#ifdef CONFIG_BASE_SMALL
futex_hashsize = 16;
#else
- futex_hashsize = roundup_pow_of_two(256 * num_possible_cpus());
+ futex_hashsize = 256 * num_possible_cpus();
+ futex_hashsize /= num_possible_nodes();
+ futex_hashsize = roundup_pow_of_two(futex_hashsize);
#endif
+ futex_hashshift = ilog2(futex_hashsize);
+ size = sizeof(struct futex_hash_bucket) * futex_hashsize;
+ order = get_order(size);
+
+ for_each_node(n) {
+ struct futex_hash_bucket *table;
+
+ if (order > MAX_ORDER)
+ table = vmalloc_huge_node(size, GFP_KERNEL, n);
+ else
+ table = alloc_pages_exact_nid(n, size, GFP_KERNEL);
+
+ BUG_ON(!table);
+
+ for (i = 0; i < futex_hashsize; i++) {
+ atomic_set(&table[i].waiters, 0);
+ spin_lock_init(&table[i].lock);
+ plist_head_init(&table[i].chain);
+ }
- futex_queues = alloc_large_system_hash("futex", sizeof(*futex_queues),
- futex_hashsize, 0, 0,
- &futex_shift, NULL,
- futex_hashsize, futex_hashsize);
- futex_hashsize = 1UL << futex_shift;
-
- for (i = 0; i < futex_hashsize; i++) {
- atomic_set(&futex_queues[i].waiters, 0);
- plist_head_init(&futex_queues[i].chain);
- spin_lock_init(&futex_queues[i].lock);
+ futex_queues[n] = table;
}
+ pr_info("futex hash table, %d nodes, %ld entries (order: %d, %lu bytes)\n",
+ num_possible_nodes(),
+ futex_hashsize, order,
+ sizeof(struct futex_hash_bucket) * futex_hashsize);
return 0;
}
--- a/kernel/futex/futex.h
+++ b/kernel/futex/futex.h
@@ -52,7 +52,7 @@ static inline unsigned int futex_to_flag
return flags;
}
-#define FUTEX2_VALID_MASK (FUTEX2_SIZE_MASK | FUTEX2_PRIVATE)
+#define FUTEX2_VALID_MASK (FUTEX2_SIZE_MASK | FUTEX2_NUMA | FUTEX2_PRIVATE)
/* FUTEX2_ to FLAGS_ */
static inline unsigned int futex2_to_flags(unsigned int flags2)
@@ -85,6 +85,19 @@ static inline bool futex_flags_valid(uns
if ((flags & FLAGS_SIZE_MASK) != FLAGS_SIZE_32)
return false;
+ /*
+ * Must be able to represent both FUTEX_NO_NODE and every valid nodeid
+ * in a futex word.
+ */
+ if (flags & FLAGS_NUMA) {
+ int bits = 8 * futex_size(flags);
+ u64 max = ~0ULL;
+
+ max >>= 64 - bits;
+ if (nr_node_ids >= max)
+ return false;
+ }
+
return true;
}
@@ -193,7 +206,7 @@ enum futex_access {
FUTEX_WRITE
};
-extern int get_futex_key(u32 __user *uaddr, unsigned int flags, union futex_key *key,
+extern int get_futex_key(void __user *uaddr, unsigned int flags, union futex_key *key,
enum futex_access rw);
extern struct hrtimer_sleeper *
^ permalink raw reply
* [PATCH 1/6] mm: Add vmalloc_huge_node()
From: Peter Zijlstra @ 2024-10-25 9:03 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, peterz, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke, cl, llong, Christoph Hellwig
In-Reply-To: <20241025090347.244183920@infradead.org>
To enable node specific hash-tables.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
include/linux/vmalloc.h | 3 +++
mm/vmalloc.c | 7 +++++++
2 files changed, 10 insertions(+)
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -177,6 +177,9 @@ void *__vmalloc_node_noprof(unsigned lon
void *vmalloc_huge_noprof(unsigned long size, gfp_t gfp_mask) __alloc_size(1);
#define vmalloc_huge(...) alloc_hooks(vmalloc_huge_noprof(__VA_ARGS__))
+void *vmalloc_huge_node_noprof(unsigned long size, gfp_t gfp_mask, int node) __alloc_size(1);
+#define vmalloc_huge_node(...) alloc_hooks(vmalloc_huge_node_noprof(__VA_ARGS__))
+
extern void *__vmalloc_array_noprof(size_t n, size_t size, gfp_t flags) __alloc_size(1, 2);
#define __vmalloc_array(...) alloc_hooks(__vmalloc_array_noprof(__VA_ARGS__))
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3948,6 +3948,13 @@ void *vmalloc_huge_noprof(unsigned long
}
EXPORT_SYMBOL_GPL(vmalloc_huge_noprof);
+void *vmalloc_huge_node_noprof(unsigned long size, gfp_t gfp_mask, int node)
+{
+ return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
+ gfp_mask, PAGE_KERNEL, VM_ALLOW_HUGE_VMAP,
+ node, __builtin_return_address(0));
+}
+
/**
* vzalloc - allocate virtually contiguous memory with zero fill
* @size: allocation size
^ permalink raw reply
* Re: [PATCH v1 11/14] futex: Implement FUTEX2_NUMA
From: Peter Zijlstra @ 2024-10-25 8:58 UTC (permalink / raw)
To: Christoph Lameter (Ampere)
Cc: tglx, axboe, linux-kernel, mingo, dvhart, dave, andrealmeid,
Andrew Morton, urezki, hch, lstoakes, Arnd Bergmann, linux-api,
linux-mm, linux-arch, malteskarupke
In-Reply-To: <9dc04e4c-2adc-5084-4ea1-b200d82be29f@linux.com>
On Wed, Jun 12, 2024 at 10:23:00AM -0700, Christoph Lameter (Ampere) wrote:
> > When FUTEX2_NUMA is not set, the node is simply an extention of the
> > hash, such that traditional futexes are still interleaved over the
> > nodes.
>
> Could we follow NUMA policies like with other metadata allocations during
> systen call processing?
I had a quick look at this, and since the mempolicy stuff is per vma,
and we don't have the vma, this is going to be terribly expensive --
mmap_lock and all that.
Once lockless vma lookups land (soonish, perhaps), this could be
reconsidered. But for now there just isn't a sane way to do this.
Using memory policies is probably okay -- but still risky, since you get
the extra failure case where if you change the mempolicy between WAIT
and WAKE things will not match and sadness happens, but that *SHOULD*
hopefully not happen a lot. Mempolicies are typically fairly static.
> If there is no NUMA task policy then the futex
> should be placed on the local NUMA node.
> That way the placement of the futex can be controlled by the tasks memory
> policy. We could skip the FUTEX2_NUMA option.
That doesn't work. If we don't have storage for the node across
WAIT/WAKE, then the node must be deterministic per futex_hash().
Otherwise wake has no chance of finding the entry.
Consider our random unbound task with no policies etc. (default state)
doing FUTEX_WAIT and going to sleep while on node-0, it's sibling
thread, that happens to run on node-1 issues FUTEX_WAKE.
If they disagree on determining 'node', then they will not find match
and the wakeup doesn't happen and userspace gets really sad.
The current scheme where we determine node based on hash bits is fully
deterministic and WAIT/WAKE will agree on which node-hash to use. The
interleave is no worse than the global hash today -- OTOH it also isn't
better.
^ permalink raw reply
* Re: [PATCH v4 4/4] selftests: pidfd: add tests for PIDFD_SELF_*
From: Lorenzo Stoakes @ 2024-10-25 8:48 UTC (permalink / raw)
To: kernel test robot
Cc: oe-lkp, lkp, Shuah Khan, linux-kernel, Christian Brauner,
Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, John Hubbard
In-Reply-To: <202410251504.707d78fc-oliver.sang@intel.com>
On Fri, Oct 25, 2024 at 04:10:37PM +0800, kernel test robot wrote:
>
>
> Hello,
>
> kernel test robot noticed "kernel-selftests.cgroup.make.fail" on:
>
> commit: 930cb1423ee2522760ffde43455b14df5c0d5487 ("[PATCH v4 4/4] selftests: pidfd: add tests for PIDFD_SELF_*")
> url: https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Stoakes/pidfd-extend-pidfd_get_pid-and-de-duplicate-pid-lookup/20241018-050825
Thanks.
This issue is because, incredibly, the cgroup tests import ../pidfd/pidfd.h
solely to use a helper, but in doing so inadvertently pull in linux/pidfd.h
_without_ the tools wrapper.
Adding a tools wrapper to cgroup fails too because of some other dependency.
I will separate out a header with this helper in it to work around this and
respin.
A gentle point on this - in my view, adding/updating tests shouldn't hold up a
series, rather we should do everything we can to encourage kernel developers to
add/expand tests.
So I'd say, in future, it might be best - if the tests already do something
considered 'bad' - to defer fixing that badness to a dedicated series, rather
than forcing an unrelated one to have to include commits to fixup pre-existing
problems like this.
I don't think anyone is really going to understand why a PIDFD_SELF series is
touching a cgroup test at this point.
Thanks!
> base: https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git next
> patch link: https://lore.kernel.org/all/b9851fa9f87d22f352f960b847d99459ef7d74a1.1729198898.git.lorenzo.stoakes@oracle.com/
> patch subject: [PATCH v4 4/4] selftests: pidfd: add tests for PIDFD_SELF_*
>
> in testcase: kernel-selftests
> version: kernel-selftests-x86_64-977d51cf-1_20240508
> with following parameters:
>
> group: cgroup
>
>
>
> config: x86_64-rhel-8.3-kselftests
> compiler: gcc-12
> test machine: 36 threads 1 sockets Intel(R) Core(TM) i9-10980XE CPU @ 3.00GHz (Cascade Lake) with 32G memory
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>
>
>
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202410251504.707d78fc-oliver.sang@intel.com
>
> KERNEL SELFTESTS: linux_headers_dir is /usr/src/linux-headers-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487
> '/usr/sbin/iptables' -> '/usr/sbin/iptables-nft'
> '/usr/sbin/iptables-restore' -> '/usr/sbin/iptables-nft-restore'
> '/usr/sbin/iptables-save' -> '/usr/sbin/iptables-nft-save'
> '/usr/sbin/ip6tables' -> '/usr/sbin/ip6tables-nft'
> '/usr/sbin/ip6tables-restore' -> '/usr/sbin/ip6tables-nft-restore'
> '/usr/sbin/ip6tables-save' -> '/usr/sbin/ip6tables-nft-save'
> 2024-10-23 12:53:55 sed -i s/default_timeout=45/default_timeout=300/ kselftest/runner.sh
> 2024-10-23 12:53:55 make -j36 -C cgroup
> make: Entering directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
> CC test_core
> CC test_cpu
> CC test_cpuset
> CC test_freezer
> CC test_hugetlb_memcg
> CC test_kill
> CC test_kmem
> CC test_memcontrol
> CC test_pids
> CC test_zswap
> CC wait_inotify
> In file included from /usr/x86_64-linux-gnu/include/asm/fcntl.h:1,
> from /usr/x86_64-linux-gnu/include/linux/fcntl.h:5,
> from /usr/x86_64-linux-gnu/include/linux/pidfd.h:7,
> from ../pidfd/pidfd.h:19,
> from test_kill.c:13:
> /usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:156:8: error: redefinition of ‘struct f_owner_ex’
> 156 | struct f_owner_ex {
> | ^~~~~~~~~~
> In file included from /usr/include/x86_64-linux-gnu/bits/fcntl.h:61,
> from /usr/include/fcntl.h:35,
> from ../pidfd/pidfd.h:8:
> /usr/include/x86_64-linux-gnu/bits/fcntl-linux.h:274:8: note: originally defined here
> 274 | struct f_owner_ex
> | ^~~~~~~~~~
> /usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:196:8: error: redefinition of ‘struct flock’
> 196 | struct flock {
> | ^~~~~
> /usr/include/x86_64-linux-gnu/bits/fcntl.h:35:8: note: originally defined here
> 35 | struct flock
> | ^~~~~
> /usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:210:8: error: redefinition of ‘struct flock64’
> 210 | struct flock64 {
> | ^~~~~~~
> /usr/include/x86_64-linux-gnu/bits/fcntl.h:50:8: note: originally defined here
> 50 | struct flock64
> | ^~~~~~~
> make: *** [../lib.mk:221: /usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup/test_kill] Error 1
> make: *** Waiting for unfinished jobs....
> make: Leaving directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
> 2024-10-23 12:53:56 make quicktest=1 run_tests -C cgroup
> make: Entering directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
> CC test_kill
> In file included from /usr/x86_64-linux-gnu/include/asm/fcntl.h:1,
> from /usr/x86_64-linux-gnu/include/linux/fcntl.h:5,
> from /usr/x86_64-linux-gnu/include/linux/pidfd.h:7,
> from ../pidfd/pidfd.h:19,
> from test_kill.c:13:
> /usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:156:8: error: redefinition of ‘struct f_owner_ex’
> 156 | struct f_owner_ex {
> | ^~~~~~~~~~
> In file included from /usr/include/x86_64-linux-gnu/bits/fcntl.h:61,
> from /usr/include/fcntl.h:35,
> from ../pidfd/pidfd.h:8:
> /usr/include/x86_64-linux-gnu/bits/fcntl-linux.h:274:8: note: originally defined here
> 274 | struct f_owner_ex
> | ^~~~~~~~~~
> /usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:196:8: error: redefinition of ‘struct flock’
> 196 | struct flock {
> | ^~~~~
> /usr/include/x86_64-linux-gnu/bits/fcntl.h:35:8: note: originally defined here
> 35 | struct flock
> | ^~~~~
> /usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:210:8: error: redefinition of ‘struct flock64’
> 210 | struct flock64 {
> | ^~~~~~~
> /usr/include/x86_64-linux-gnu/bits/fcntl.h:50:8: note: originally defined here
> 50 | struct flock64
> | ^~~~~~~
> make: *** [../lib.mk:222: /usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup/test_kill] Error 1
> make: Leaving directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
>
>
>
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20241025/202410251504.707d78fc-oliver.sang@intel.com
>
>
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
>
^ permalink raw reply
* Re: [PATCH v4 4/4] selftests: pidfd: add tests for PIDFD_SELF_*
From: kernel test robot @ 2024-10-25 8:10 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: oe-lkp, lkp, Shuah Khan, linux-kernel, Christian Brauner,
Shuah Khan, Liam R . Howlett, Suren Baghdasaryan, Vlastimil Babka,
pedro.falcato, linux-kselftest, linux-mm, linux-fsdevel,
linux-api, Oliver Sang, John Hubbard
In-Reply-To: <b9851fa9f87d22f352f960b847d99459ef7d74a1.1729198898.git.lorenzo.stoakes@oracle.com>
Hello,
kernel test robot noticed "kernel-selftests.cgroup.make.fail" on:
commit: 930cb1423ee2522760ffde43455b14df5c0d5487 ("[PATCH v4 4/4] selftests: pidfd: add tests for PIDFD_SELF_*")
url: https://github.com/intel-lab-lkp/linux/commits/Lorenzo-Stoakes/pidfd-extend-pidfd_get_pid-and-de-duplicate-pid-lookup/20241018-050825
base: https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git next
patch link: https://lore.kernel.org/all/b9851fa9f87d22f352f960b847d99459ef7d74a1.1729198898.git.lorenzo.stoakes@oracle.com/
patch subject: [PATCH v4 4/4] selftests: pidfd: add tests for PIDFD_SELF_*
in testcase: kernel-selftests
version: kernel-selftests-x86_64-977d51cf-1_20240508
with following parameters:
group: cgroup
config: x86_64-rhel-8.3-kselftests
compiler: gcc-12
test machine: 36 threads 1 sockets Intel(R) Core(TM) i9-10980XE CPU @ 3.00GHz (Cascade Lake) with 32G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202410251504.707d78fc-oliver.sang@intel.com
KERNEL SELFTESTS: linux_headers_dir is /usr/src/linux-headers-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487
'/usr/sbin/iptables' -> '/usr/sbin/iptables-nft'
'/usr/sbin/iptables-restore' -> '/usr/sbin/iptables-nft-restore'
'/usr/sbin/iptables-save' -> '/usr/sbin/iptables-nft-save'
'/usr/sbin/ip6tables' -> '/usr/sbin/ip6tables-nft'
'/usr/sbin/ip6tables-restore' -> '/usr/sbin/ip6tables-nft-restore'
'/usr/sbin/ip6tables-save' -> '/usr/sbin/ip6tables-nft-save'
2024-10-23 12:53:55 sed -i s/default_timeout=45/default_timeout=300/ kselftest/runner.sh
2024-10-23 12:53:55 make -j36 -C cgroup
make: Entering directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
CC test_core
CC test_cpu
CC test_cpuset
CC test_freezer
CC test_hugetlb_memcg
CC test_kill
CC test_kmem
CC test_memcontrol
CC test_pids
CC test_zswap
CC wait_inotify
In file included from /usr/x86_64-linux-gnu/include/asm/fcntl.h:1,
from /usr/x86_64-linux-gnu/include/linux/fcntl.h:5,
from /usr/x86_64-linux-gnu/include/linux/pidfd.h:7,
from ../pidfd/pidfd.h:19,
from test_kill.c:13:
/usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:156:8: error: redefinition of ‘struct f_owner_ex’
156 | struct f_owner_ex {
| ^~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/bits/fcntl.h:61,
from /usr/include/fcntl.h:35,
from ../pidfd/pidfd.h:8:
/usr/include/x86_64-linux-gnu/bits/fcntl-linux.h:274:8: note: originally defined here
274 | struct f_owner_ex
| ^~~~~~~~~~
/usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:196:8: error: redefinition of ‘struct flock’
196 | struct flock {
| ^~~~~
/usr/include/x86_64-linux-gnu/bits/fcntl.h:35:8: note: originally defined here
35 | struct flock
| ^~~~~
/usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:210:8: error: redefinition of ‘struct flock64’
210 | struct flock64 {
| ^~~~~~~
/usr/include/x86_64-linux-gnu/bits/fcntl.h:50:8: note: originally defined here
50 | struct flock64
| ^~~~~~~
make: *** [../lib.mk:221: /usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup/test_kill] Error 1
make: *** Waiting for unfinished jobs....
make: Leaving directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
2024-10-23 12:53:56 make quicktest=1 run_tests -C cgroup
make: Entering directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
CC test_kill
In file included from /usr/x86_64-linux-gnu/include/asm/fcntl.h:1,
from /usr/x86_64-linux-gnu/include/linux/fcntl.h:5,
from /usr/x86_64-linux-gnu/include/linux/pidfd.h:7,
from ../pidfd/pidfd.h:19,
from test_kill.c:13:
/usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:156:8: error: redefinition of ‘struct f_owner_ex’
156 | struct f_owner_ex {
| ^~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/bits/fcntl.h:61,
from /usr/include/fcntl.h:35,
from ../pidfd/pidfd.h:8:
/usr/include/x86_64-linux-gnu/bits/fcntl-linux.h:274:8: note: originally defined here
274 | struct f_owner_ex
| ^~~~~~~~~~
/usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:196:8: error: redefinition of ‘struct flock’
196 | struct flock {
| ^~~~~
/usr/include/x86_64-linux-gnu/bits/fcntl.h:35:8: note: originally defined here
35 | struct flock
| ^~~~~
/usr/x86_64-linux-gnu/include/asm-generic/fcntl.h:210:8: error: redefinition of ‘struct flock64’
210 | struct flock64 {
| ^~~~~~~
/usr/include/x86_64-linux-gnu/bits/fcntl.h:50:8: note: originally defined here
50 | struct flock64
| ^~~~~~~
make: *** [../lib.mk:222: /usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup/test_kill] Error 1
make: Leaving directory '/usr/src/perf_selftests-x86_64-rhel-8.3-kselftests-930cb1423ee2522760ffde43455b14df5c0d5487/tools/testing/selftests/cgroup'
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20241025/202410251504.707d78fc-oliver.sang@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH RFC 1/1] futex: Create set_robust_list2
From: Arnd Bergmann @ 2024-10-25 6:33 UTC (permalink / raw)
To: André Almeida
Cc: Thomas Gleixner, Ingo Molnar, Davidlohr Bueso, Darren Hart,
Peter Zijlstra, sonicadvance1, linux-kernel, kernel-dev,
linux-api
In-Reply-To: <59509fc9-1a19-4162-ac89-559e08b75c06@igalia.com>
On Thu, Oct 24, 2024, at 19:55, André Almeida wrote:
> Em 24/10/2024 15:03, André Almeida escreveu:
>> Em 24/10/2024 12:57, Arnd Bergmann escreveu:
>>> On Thu, Oct 24, 2024, at 14:57, André Almeida wrote:
>>>> This new syscall allows to set multiple list to the same process. There
>>>> are two list types: 32 and 64 bit lists.
>>> Assuming we want them to behave the same way, did you intend
>>> ROBUST_LIST_64BIT to refer to 64-bit pointers on 32-bit
>>> tasks, or should they use normal word-size pointers?
>>
>> Oh right, I haven't covered that indeed. I think I would need to have
>> something like:
>>
>> static void exit_robust_list_64()
>> static void exit_robust_list_32()
>>
>> And then each function would use explicit sizes for pointers. Also, I
>> would rewrite the conditions to make that every combination of 64/32bit
>> kernel/app calls the appropriated function.
>
> Something like this:
>
> #ifdef CONFIG_64BIT
> if (unlikely(tsk->robust_list)) {
> exit_robust_list_64bit(tsk, tsk->robust_list);
> tsk->robust_list = NULL;
> }
> #else
> if (unlikely(tsk->robust_list)) {
> exit_robust_list_32bit(tsk, tsk->robust_list);
> tsk->robust_list = NULL;
> }
> #endif
>
> #ifdef CONFIG_COMPAT
> if (unlikely(tsk->compat_robust_list)) {
> exit_robust_32bit(tsk, tsk->compat_robust_list);
> tsk->compat_robust_list = NULL;
> }
> #endif
This should work, but I wonder if it ends up simpler to
instead have tsk->robust_list64 and tsk->robust_list32
entries and just put the #ifdef CONFIG_64BIT around the code
accessing robust_list64.
Arnd
^ permalink raw reply
* Re: [PATCH RFC 1/1] futex: Create set_robust_list2
From: André Almeida @ 2024-10-24 19:55 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Thomas Gleixner, Ingo Molnar, Davidlohr Bueso, Darren Hart,
Peter Zijlstra, sonicadvance1, linux-kernel, kernel-dev,
linux-api
In-Reply-To: <1e62e083-f97c-4157-8d50-c3655edda97b@igalia.com>
Em 24/10/2024 15:03, André Almeida escreveu:
> Hi Arnd,
>
> Em 24/10/2024 12:57, Arnd Bergmann escreveu:
>> On Thu, Oct 24, 2024, at 14:57, André Almeida wrote:
>>> This new syscall allows to set multiple list to the same process. There
>>> are two list types: 32 and 64 bit lists.
>>>
[...]
>>> + if (unlikely(!list_empty(list2))) {
>>> + list_for_each_entry_safe(curr, n, list2, list) {
>>> + if (curr->head != NULL) {
>>> + if (curr->list_type == ROBUST_LIST_64BIT)
>>> + exit_robust_list(tsk, curr->head);
>>> + else if (curr->list_type == ROBUST_LIST_32BIT)
>>> + compat_exit_robust_list(tsk, curr->head);
>>> + curr->head = NULL;
>>> + }
>>
>> This looks like the behavior of a 32-bit task using
>> ROBUST_LIST_64BIT is different on native 32-bit kernels
>> compared to running on compat mode.
>>
>> Assuming we want them to behave the same way, did you intend
>> ROBUST_LIST_64BIT to refer to 64-bit pointers on 32-bit
>> tasks, or should they use normal word-size pointers?
>
> Oh right, I haven't covered that indeed. I think I would need to have
> something like:
>
> static void exit_robust_list_64()
> static void exit_robust_list_32()
>
> And then each function would use explicit sizes for pointers. Also, I
> would rewrite the conditions to make that every combination of 64/32bit
> kernel/app calls the appropriated function.
Something like this:
#ifdef CONFIG_64BIT
if (unlikely(tsk->robust_list)) {
exit_robust_list_64bit(tsk, tsk->robust_list);
tsk->robust_list = NULL;
}
#else
if (unlikely(tsk->robust_list)) {
exit_robust_list_32bit(tsk, tsk->robust_list);
tsk->robust_list = NULL;
}
#endif
#ifdef CONFIG_COMPAT
if (unlikely(tsk->compat_robust_list)) {
exit_robust_32bit(tsk, tsk->compat_robust_list);
tsk->compat_robust_list = NULL;
}
#endif
/* Simplified */
list_for_each_entry_safe(curr, n, list2, list) {
if (curr->list_type == ROBUST_LIST_64BIT)
exit_robust_list_64bit(tsk, curr->head);
else if (curr->list_type == ROBUST_LIST_32BIT)
exit_robust_list_32bit(tsk, curr->head);
}
^ permalink raw reply
* Re: [PATCH RFC 1/1] futex: Create set_robust_list2
From: Randy Dunlap @ 2024-10-24 18:08 UTC (permalink / raw)
To: André Almeida, Arnd Bergmann
Cc: Thomas Gleixner, Ingo Molnar, Davidlohr Bueso, Darren Hart,
Peter Zijlstra, sonicadvance1, linux-kernel, kernel-dev,
linux-api
In-Reply-To: <1e62e083-f97c-4157-8d50-c3655edda97b@igalia.com>
Hi,
On 10/24/24 11:03 AM, André Almeida wrote:
> Hi Arnd,
>
Please consider putting more of the cover letter explanation into the commit message.
Thanks.
--
~Randy
^ permalink raw reply
* Re: [PATCH RFC 1/1] futex: Create set_robust_list2
From: André Almeida @ 2024-10-24 18:03 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Thomas Gleixner, Ingo Molnar, Davidlohr Bueso, Darren Hart,
Peter Zijlstra, sonicadvance1, linux-kernel, kernel-dev,
linux-api
In-Reply-To: <bde852ec-8e2f-4957-9368-00d8e5a422c4@app.fastmail.com>
Hi Arnd,
Em 24/10/2024 12:57, Arnd Bergmann escreveu:
> On Thu, Oct 24, 2024, at 14:57, André Almeida wrote:
>> This new syscall allows to set multiple list to the same process. There
>> are two list types: 32 and 64 bit lists.
>>
>> It supports up to 10 lists per process (see ROBUST_LISTS_PER_TASK). The
>> lists are dynamically allocated on demand, as part of a linked list.
>> This is the proposed interface:
>>
>> long set_robust_list2(void *head, int index, unsigned int flags)
>>
>> Userspace can ask to set the head of a new list using (index = -1).
>> Kernel will allocate a new list, place in the linked list and return the
>> new index to userspace.
>>
>> Userspace can modify an existing head by using an index >= 0. If the
>> requested list doesn't exist, an error is returned.
>>
>> Userspace cannot remove a robust list.
>>
>> For now, flag is for the list type:
>>
>> enum robust_list_type {
>> ROBUST_LIST_32BIT,
>> ROBUST_LIST_64BIT,
>> };
>>
>> Signed-off-by: André Almeida <andrealmeid@igalia.com>
>
> Hi André,
>
> I have no opinion on the syscall itself, but I'll comment on
> the way you hook it up:
>
>> arch/arm/tools/syscall.tbl | 1 +
>> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
>
> If we agree on the number, this should be added to all
> architectures at the same time. In particular, when
> you add it to 32-bit arm, it also needs to be in the
> corresponding arch/arm64/tools/syscall_32.tbl for
> compat mode.
Ok
>
>> include/uapi/asm-generic/unistd.h | 5 +-
>
> This reminds me that I need to send the patch to remove this
> file, nothing should use it any more, though we still have
> the copy in tools/include/uapi/asm-generic/unistd.h that
> still gets referenced until the scripts are changed to
> use the syscall.tbl format.
>
>> + if (unlikely(!list_empty(list2))) {
>> + list_for_each_entry_safe(curr, n, list2, list) {
>> + if (curr->head != NULL) {
>> + if (curr->list_type == ROBUST_LIST_64BIT)
>> + exit_robust_list(tsk, curr->head);
>> + else if (curr->list_type == ROBUST_LIST_32BIT)
>> + compat_exit_robust_list(tsk, curr->head);
>> + curr->head = NULL;
>> + }
>
> This looks like the behavior of a 32-bit task using
> ROBUST_LIST_64BIT is different on native 32-bit kernels
> compared to running on compat mode.
>
> Assuming we want them to behave the same way, did you intend
> ROBUST_LIST_64BIT to refer to 64-bit pointers on 32-bit
> tasks, or should they use normal word-size pointers?
Oh right, I haven't covered that indeed. I think I would need to have
something like:
static void exit_robust_list_64()
static void exit_robust_list_32()
And then each function would use explicit sizes for pointers. Also, I
would rewrite the conditions to make that every combination of 64/32bit
kernel/app calls the appropriated function.
Alternatively, we could just disable 32bit kernel/app to use the
ROBUST_LIST_64BIT option.
Thank you for your feedback!
André
^ permalink raw reply
* Re: [PATCH RFC 1/1] futex: Create set_robust_list2
From: Arnd Bergmann @ 2024-10-24 15:57 UTC (permalink / raw)
To: André Almeida, Thomas Gleixner, Ingo Molnar, Peter Zijlstra,
Darren Hart, Davidlohr Bueso, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api
In-Reply-To: <20241024145735.162090-2-andrealmeid@igalia.com>
On Thu, Oct 24, 2024, at 14:57, André Almeida wrote:
> This new syscall allows to set multiple list to the same process. There
> are two list types: 32 and 64 bit lists.
>
> It supports up to 10 lists per process (see ROBUST_LISTS_PER_TASK). The
> lists are dynamically allocated on demand, as part of a linked list.
> This is the proposed interface:
>
> long set_robust_list2(void *head, int index, unsigned int flags)
>
> Userspace can ask to set the head of a new list using (index = -1).
> Kernel will allocate a new list, place in the linked list and return the
> new index to userspace.
>
> Userspace can modify an existing head by using an index >= 0. If the
> requested list doesn't exist, an error is returned.
>
> Userspace cannot remove a robust list.
>
> For now, flag is for the list type:
>
> enum robust_list_type {
> ROBUST_LIST_32BIT,
> ROBUST_LIST_64BIT,
> };
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
Hi André,
I have no opinion on the syscall itself, but I'll comment on
the way you hook it up:
> arch/arm/tools/syscall.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
If we agree on the number, this should be added to all
architectures at the same time. In particular, when
you add it to 32-bit arm, it also needs to be in the
corresponding arch/arm64/tools/syscall_32.tbl for
compat mode.
> include/uapi/asm-generic/unistd.h | 5 +-
This reminds me that I need to send the patch to remove this
file, nothing should use it any more, though we still have
the copy in tools/include/uapi/asm-generic/unistd.h that
still gets referenced until the scripts are changed to
use the syscall.tbl format.
> + if (unlikely(!list_empty(list2))) {
> + list_for_each_entry_safe(curr, n, list2, list) {
> + if (curr->head != NULL) {
> + if (curr->list_type == ROBUST_LIST_64BIT)
> + exit_robust_list(tsk, curr->head);
> + else if (curr->list_type == ROBUST_LIST_32BIT)
> + compat_exit_robust_list(tsk, curr->head);
> + curr->head = NULL;
> + }
This looks like the behavior of a 32-bit task using
ROBUST_LIST_64BIT is different on native 32-bit kernels
compared to running on compat mode.
Assuming we want them to behave the same way, did you intend
ROBUST_LIST_64BIT to refer to 64-bit pointers on 32-bit
tasks, or should they use normal word-size pointers?
Arnd
^ permalink raw reply
* [PATCH RFC 1/1] futex: Create set_robust_list2
From: André Almeida @ 2024-10-24 14:57 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Arnd Bergmann, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, André Almeida
In-Reply-To: <20241024145735.162090-1-andrealmeid@igalia.com>
This new syscall allows to set multiple list to the same process. There
are two list types: 32 and 64 bit lists.
It supports up to 10 lists per process (see ROBUST_LISTS_PER_TASK). The
lists are dynamically allocated on demand, as part of a linked list.
This is the proposed interface:
long set_robust_list2(void *head, int index, unsigned int flags)
Userspace can ask to set the head of a new list using (index = -1).
Kernel will allocate a new list, place in the linked list and return the
new index to userspace.
Userspace can modify an existing head by using an index >= 0. If the
requested list doesn't exist, an error is returned.
Userspace cannot remove a robust list.
For now, flag is for the list type:
enum robust_list_type {
ROBUST_LIST_32BIT,
ROBUST_LIST_64BIT,
};
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
arch/arm/tools/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/futex.h | 1 +
include/linux/sched.h | 1 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/futex.h | 24 +++++++++
init/init_task.c | 3 ++
kernel/futex/core.c | 34 +++++++++---
kernel/futex/syscalls.c | 72 ++++++++++++++++++++++++++
scripts/syscall.tbl | 1 +
10 files changed, 134 insertions(+), 9 deletions(-)
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 23c98203c40f..31070d427ea2 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -477,3 +477,4 @@
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
462 common mseal sys_mseal
+463 common set_robust_list2 sys_set_robust_list2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7093ee21c0d1..fbc0cef1a97c 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -386,6 +386,7 @@
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
462 common mseal sys_mseal
+463 common set_robust_list2 sys_set_robust_list2
#
# Due to a historical design error, certain syscalls are numbered differently
diff --git a/include/linux/futex.h b/include/linux/futex.h
index b70df27d7e85..9c5ab84f86a9 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -65,6 +65,7 @@ static inline void futex_init_task(struct task_struct *tsk)
#ifdef CONFIG_COMPAT
tsk->compat_robust_list = NULL;
#endif
+ INIT_LIST_HEAD(&tsk->robust_list2);
INIT_LIST_HEAD(&tsk->pi_state_list);
tsk->pi_state_cache = NULL;
tsk->futex_state = FUTEX_STATE_OK;
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 449dd64ed9ac..5f72fe66add6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1284,6 +1284,7 @@ struct task_struct {
#ifdef CONFIG_COMPAT
struct compat_robust_list_head __user *compat_robust_list;
#endif
+ struct list_head robust_list2;
struct list_head pi_state_list;
struct futex_pi_state *pi_state_cache;
struct mutex futex_exit_mutex;
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 5bf6148cac2b..c1f5c9635c07 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -841,8 +841,11 @@ __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
#define __NR_mseal 462
__SYSCALL(__NR_mseal, sys_mseal)
+#define __NR_set_robust_list2 463
+__SYSCALL(__NR_set_robust_list2, sys_set_robust_list2)
+
#undef __NR_syscalls
-#define __NR_syscalls 463
+#define __NR_syscalls 464
/*
* 32 bit systems traditionally used different
diff --git a/include/uapi/linux/futex.h b/include/uapi/linux/futex.h
index d2ee625ea189..13903a278b71 100644
--- a/include/uapi/linux/futex.h
+++ b/include/uapi/linux/futex.h
@@ -146,6 +146,30 @@ struct robust_list_head {
struct robust_list __user *list_op_pending;
};
+#define ROBUST_LISTS_PER_TASK 10
+
+enum robust_list2_type {
+ ROBUST_LIST_32BIT,
+ ROBUST_LIST_64BIT,
+};
+
+#define ROBUST_LIST_TYPE_MASK (ROBUST_LIST_32BIT | ROBUST_LIST_64BIT)
+
+/*
+ * This is an entry of a linked list of robust lists.
+ *
+ * @head: can point to a 64bit list or a 32bit list
+ * @list_type: determine the size of the futex pointers in the list
+ * @index: the index of this entry in the list
+ * @list: linked list element
+ */
+struct robust_list2_entry {
+ void __user *head;
+ enum robust_list2_type list_type;
+ unsigned int index;
+ struct list_head list;
+};
+
/*
* Are there any waiters for this robust futex:
*/
diff --git a/init/init_task.c b/init/init_task.c
index 136a8231355a..1b08e745c47d 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -219,6 +219,9 @@ struct task_struct init_task __aligned(L1_CACHE_BYTES) = {
#ifdef CONFIG_SECCOMP_FILTER
.seccomp = { .filter_count = ATOMIC_INIT(0) },
#endif
+#ifdef CONFIG_FUTEX
+ .robust_list2 = LIST_HEAD_INIT(init_task.robust_list2),
+#endif
};
EXPORT_SYMBOL(init_task);
diff --git a/kernel/futex/core.c b/kernel/futex/core.c
index 136768ae2637..d71b4b9630f7 100644
--- a/kernel/futex/core.c
+++ b/kernel/futex/core.c
@@ -796,9 +796,8 @@ static inline int fetch_robust_entry(struct robust_list __user **entry,
*
* We silently return on any sign of list-walking problem.
*/
-static void exit_robust_list(struct task_struct *curr)
+static void exit_robust_list(struct task_struct *curr, struct robust_list_head __user *head)
{
- struct robust_list_head __user *head = curr->robust_list;
struct robust_list __user *entry, *next_entry, *pending;
unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
unsigned int next_pi;
@@ -858,7 +857,6 @@ static void exit_robust_list(struct task_struct *curr)
}
}
-#ifdef CONFIG_COMPAT
static void __user *futex_uaddr(struct robust_list __user *entry,
compat_long_t futex_offset)
{
@@ -890,9 +888,9 @@ compat_fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **ent
*
* We silently return on any sign of list-walking problem.
*/
-static void compat_exit_robust_list(struct task_struct *curr)
+static void compat_exit_robust_list(struct task_struct *curr,
+ struct compat_robust_list_head __user *head)
{
- struct compat_robust_list_head __user *head = curr->compat_robust_list;
struct robust_list __user *entry, *next_entry, *pending;
unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
unsigned int next_pi;
@@ -957,7 +955,6 @@ static void compat_exit_robust_list(struct task_struct *curr)
handle_futex_death(uaddr, curr, pip, HANDLE_DEATH_PENDING);
}
}
-#endif
#ifdef CONFIG_FUTEX_PI
@@ -1040,14 +1037,35 @@ static inline void exit_pi_state_list(struct task_struct *curr) { }
static void futex_cleanup(struct task_struct *tsk)
{
+ struct robust_list2_entry *curr, *n;
+ struct list_head *list2 = &tsk->robust_list2;
+
if (unlikely(tsk->robust_list)) {
- exit_robust_list(tsk);
+ exit_robust_list(tsk, tsk->robust_list);
tsk->robust_list = NULL;
}
+ /*
+ * Walk through the linked list, parsing robust lists and freeing the
+ * allocated lists
+ */
+ if (unlikely(!list_empty(list2))) {
+ list_for_each_entry_safe(curr, n, list2, list) {
+ if (curr->head != NULL) {
+ if (curr->list_type == ROBUST_LIST_64BIT)
+ exit_robust_list(tsk, curr->head);
+ else if (curr->list_type == ROBUST_LIST_32BIT)
+ compat_exit_robust_list(tsk, curr->head);
+ curr->head = NULL;
+ }
+ list_del_init(&curr->list);
+ kfree(curr);
+ }
+ }
+
#ifdef CONFIG_COMPAT
if (unlikely(tsk->compat_robust_list)) {
- compat_exit_robust_list(tsk);
+ compat_exit_robust_list(tsk, tsk->compat_robust_list);
tsk->compat_robust_list = NULL;
}
#endif
diff --git a/kernel/futex/syscalls.c b/kernel/futex/syscalls.c
index 4b6da9116aa6..2853de204f9b 100644
--- a/kernel/futex/syscalls.c
+++ b/kernel/futex/syscalls.c
@@ -39,6 +39,78 @@ SYSCALL_DEFINE2(set_robust_list, struct robust_list_head __user *, head,
return 0;
}
+#define ROBUST_LIST_FLAGS ROBUST_LIST_TYPE_MASK
+
+/*
+ * sys_set_robust_list2()
+ *
+ * When index == -1, create a new list for user. When index >= 0, try to find
+ * the corresponding list and re-set the head there.
+ *
+ * Return values:
+ * >= 0: success, index of the robust list
+ * -EINVAL: invalid flags, invalid index
+ * -ENOENT: requested index no where to be found
+ * -ENOMEM: error allocating new list
+ * -ESRCH: too many allocated lists
+ */
+SYSCALL_DEFINE3(set_robust_list2, struct robust_list_head __user *, head,
+ int, index, unsigned int, flags)
+{
+ struct list_head *list2 = ¤t->robust_list2;
+ struct robust_list2_entry *prev, *new = NULL;
+ unsigned int type;
+
+ type = flags & ROBUST_LIST_TYPE_MASK;
+
+ if (index < -1 || index >= ROBUST_LISTS_PER_TASK)
+ return -EINVAL;
+
+ if ((flags & ~ROBUST_LIST_FLAGS) != 0)
+ return -EINVAL;
+
+ if (index == -1) {
+ if (list_empty(list2)) {
+ index = 0;
+ } else {
+ prev = list_last_entry(list2, struct robust_list2_entry, list);
+ index = prev->index + 1;
+ }
+
+ if (index >= ROBUST_LISTS_PER_TASK)
+ return -EINVAL;
+
+ new = kmalloc(sizeof(struct robust_list2_entry), GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ list_add_tail(&new->list, list2);
+ new->index = index;
+
+ } else if (index >= 0) {
+ struct robust_list2_entry *curr;
+
+ if (list_empty(list2))
+ return -ENOENT;
+
+ list_for_each_entry(curr, list2, list) {
+ if (index == curr->index) {
+ new = curr;
+ break;
+ }
+ }
+
+ if (!new)
+ return -ENOENT;
+ }
+
+ BUG_ON(!new);
+ new->head = head;
+ new->list_type = type;
+
+ return index;
+}
+
/**
* sys_get_robust_list() - Get the robust-futex list head of a task
* @pid: pid of the process [zero for current task]
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index 845e24eb372e..e174f6e2d521 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -403,3 +403,4 @@
460 common lsm_set_self_attr sys_lsm_set_self_attr
461 common lsm_list_modules sys_lsm_list_modules
462 common mseal sys_mseal
+463 common set_robust_list2 sys_set_robust_list2
--
2.47.0
^ permalink raw reply related
* [PATCH RFC 0/1] futex: Add a new robust list syscall
From: André Almeida @ 2024-10-24 14:57 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Peter Zijlstra, Darren Hart,
Davidlohr Bueso, Arnd Bergmann, sonicadvance1
Cc: linux-kernel, kernel-dev, linux-api, André Almeida
This patch adds a new robust_list() syscall. The current syscall
can't be expanded to cover the following use case, so a new one is
needed. This new syscall allows users to set multiple robust lists per
process and to have either 32bit or 64bit pointers in the list.
* Use case
FEX-Emu[1] is an application that runs x86 and x86-64 binaries on an
AArch64 Linux host. One of the tasks of FEX-Emu is to translate syscalls
from one platform to another. Existing set_robust_list() can't be easily
translated because of two limitations:
1) x86 apps can have 32bit pointers robust lists. For a x86-64 kernel
this is not a problem, because of the compat entry point. But there's
no such compat entry point for AArch64, so the kernel would do the
pointer arithmetic wrongly. Is also unviable to userspace to keep
track every addition/removal to the robust list and keep a 64bit
version of it somewhere else to feed the kernel. Thus, the new
interface has an option of telling the kernel if the list is filled
with 32bit or 64bit pointers.
2) Apps can set just one robust list (in theory, x86-64 can set two if
they also use the compat entry point). That means that when a x86 app
asks FEX-Emu to call set_robust_list(), FEX have two options: to
overwrite their own robust list pointer and make the app robust, or
to ignore the app robust list and keep the emulator robust. The new
interface allows for multiple robust lists per application, solving
this.
* Interface
This is the proposed interface:
long set_robust_list2(void *head, int index, unsigned int flags)
`head` is the head of the userspace struct robust_list_head, just as old
set_robust_list(). It needs to be a void pointer since it can point to a normal
robust_list_head or a compat_robust_list_head.
`flags` can be used for defining the list type:
enum robust_list_type {
ROBUST_LIST_32BIT,
ROBUST_LIST_64BIT,
};
`index` is the index in the internal robust_list's linked list (the naming
starts to get confusing, I reckon). If `index == -1`, that means that user wants
to set a new robust_list, and the kernel will append it in the end of the list,
assign a new index and return this index to the user. If `index >= 0`, that
means that user wants to re-set `*head` of an already existing list (similarly
to what happens when you call set_robust_list() twice with different `*head`).
If `index` is out of range, or it points to a non-existing robust_list, or if
the internal list is full, an error is returned.
* Implementation
The implementation re-uses most of the existing robust list interface as
possible. The new task_struct member `struct list_head robust_list2` is just a
linked list where new lists are appended as the user requests more lists, and by
futex_cleanup(), the kernel walks through the internal list feeding
exit_robust_list() with the robust_list's.
This implementation supports up to 10 lists (defined at ROBUST_LISTS_PER_TASK),
but it was an arbitrary number for this RFC. For the described use case above, 4
should be enough, I'm not sure which should be the limit.
It doesn't support list removal (should it support?). It doesn't have a proper
get_robust_list2() yet as well, but I can add it in a next revision. We could
also have a generic robust_list() syscall that can be used to set/get and be
controlled by flags.
The new interface has a `unsigned int flags` argument, making it
extensible for future use cases as well.
* Testing
I will provide a selftest similar to the one I proposed for the current
interface here:
https://lore.kernel.org/lkml/20241010011142.905297-1-andrealmeid@igalia.com/
Also, FEX-Emu added support for this interface to validate it:
https://github.com/FEX-Emu/FEX/pull/3966
Feedback is very welcomed!
Thanks,
André
[1] https://github.com/FEX-Emu/FEX
André Almeida (1):
futex: Create set_robust_list2
arch/arm/tools/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/futex.h | 1 +
include/linux/sched.h | 1 +
include/uapi/asm-generic/unistd.h | 5 +-
include/uapi/linux/futex.h | 24 +++++++++
init/init_task.c | 3 ++
kernel/futex/core.c | 34 +++++++++---
kernel/futex/syscalls.c | 71 ++++++++++++++++++++++++++
scripts/syscall.tbl | 1 +
10 files changed, 133 insertions(+), 9 deletions(-)
--
2.47.0
^ permalink raw reply
* Re: [PATCH v3 1/5] mm: pagewalk: add the ability to install PTEs
From: Lorenzo Stoakes @ 2024-10-24 8:07 UTC (permalink / raw)
To: David Hildenbrand
Cc: Andrew Morton, Suren Baghdasaryan, Liam R . Howlett,
Matthew Wilcox, Vlastimil Babka, Paul E . McKenney, Jann Horn,
linux-mm, linux-kernel, Muchun Song, Richard Henderson,
Matt Turner, Thomas Bogendoerfer, James E . J . Bottomley,
Helge Deller, Chris Zankel, Max Filippov, Arnd Bergmann,
linux-alpha, linux-mips, linux-parisc, linux-arch, Shuah Khan,
Christian Brauner, linux-kselftest, Sidhartha Kumar, Jeff Xu,
Christoph Hellwig, linux-api, John Hubbard
In-Reply-To: <a089ff3b-119e-44a0-a780-3abca01886f7@redhat.com>
On Thu, Oct 24, 2024 at 09:45:37AM +0200, David Hildenbrand wrote:
> On 24.10.24 09:34, Lorenzo Stoakes wrote:
> > On Wed, Oct 23, 2024 at 04:04:05PM -0700, Andrew Morton wrote:
> > > On Wed, 23 Oct 2024 17:24:38 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> > >
> > > >
> > > > ...
> > > >
> > > > Existing mechanism for performing a walk which also installs page table
> > > > entries if necessary are heavily duplicated throughout the kernel,
> > >
> > > How complicated is it to migrate those to use this?
> >
> > I would say probably somewhat difficult as very often people are doing quite
> > custom things, but I will take a look at seeing if we can't make things a little
> > more generic.
> >
> > I am also mildly motivated to look at trying to find a generic way to do
> > replaces...
> >
> > Both on the TODO!
>
> I'm not super happy about extending the rusty old pagewalk API, because it's
> inefficient (indirect calls) and not future proof (batching, large folios).
Yeah it could be improved, but I think the ideal way would be to genericise as
much as we can and 'upgrade' this logic.
>
> But I see how we ended up with this patch, and it will be easy to convert to
> something better once we have it.
Yes, I was quite happy with what an ultimately small delta this all ended up
being vs. the various other alternatives (change zap logic, introduce custom
page fault mechanism, duplicating page walk code _yet again_, porting uffd
logic, etc. etc.)
But in an ideal world we'd have _one_ place that does this.
>
> We already discussed in the past that we need a better and more efficient
> way to walk page tables. I have part of that on my TODO list, but I'm
> getting distracted.
Yes I remember an LSF session on this, it's a really obvious area of improvement
that stands out at the moment for sure.
Having worked several 12+ hour days in a row though recently I can relate to
workload making this difficult though :)
>
> *Inserting* (not walking/modifying existing things as most users to) as done
> in this patch is slightly different though, likely "on thing that fits all"
> will not apply to all page table walker user cases.
Yeah, there's also replace scenarios which then have to do egregious amounts of
work to make sure we do everything right, in fact there's duplicates of this in
mm/madvise.c *grumble grumble*.
>
> --
> Cheers,
>
> David / dhildenb
>
OK so I guess I'll hold off my TODOs on this as you are looking in this area and
I trust you :)
Cheers!
^ permalink raw reply
* Re: [PATCH v3 1/5] mm: pagewalk: add the ability to install PTEs
From: David Hildenbrand @ 2024-10-24 7:45 UTC (permalink / raw)
To: Lorenzo Stoakes, Andrew Morton
Cc: Suren Baghdasaryan, Liam R . Howlett, Matthew Wilcox,
Vlastimil Babka, Paul E . McKenney, Jann Horn, linux-mm,
linux-kernel, Muchun Song, Richard Henderson, Matt Turner,
Thomas Bogendoerfer, James E . J . Bottomley, Helge Deller,
Chris Zankel, Max Filippov, Arnd Bergmann, linux-alpha,
linux-mips, linux-parisc, linux-arch, Shuah Khan,
Christian Brauner, linux-kselftest, Sidhartha Kumar, Jeff Xu,
Christoph Hellwig, linux-api, John Hubbard
In-Reply-To: <635de052-35a6-40e0-82ea-4349f3aecc06@lucifer.local>
On 24.10.24 09:34, Lorenzo Stoakes wrote:
> On Wed, Oct 23, 2024 at 04:04:05PM -0700, Andrew Morton wrote:
>> On Wed, 23 Oct 2024 17:24:38 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>>
>>>
>>> ...
>>>
>>> Existing mechanism for performing a walk which also installs page table
>>> entries if necessary are heavily duplicated throughout the kernel,
>>
>> How complicated is it to migrate those to use this?
>
> I would say probably somewhat difficult as very often people are doing quite
> custom things, but I will take a look at seeing if we can't make things a little
> more generic.
>
> I am also mildly motivated to look at trying to find a generic way to do
> replaces...
>
> Both on the TODO!
I'm not super happy about extending the rusty old pagewalk API, because
it's inefficient (indirect calls) and not future proof (batching, large
folios).
But I see how we ended up with this patch, and it will be easy to
convert to something better once we have it.
We already discussed in the past that we need a better and more
efficient way to walk page tables. I have part of that on my TODO list,
but I'm getting distracted.
*Inserting* (not walking/modifying existing things as most users to) as
done in this patch is slightly different though, likely "on thing that
fits all" will not apply to all page table walker user cases.
--
Cheers,
David / dhildenb
^ permalink raw reply
* Re: [PATCH v3 1/5] mm: pagewalk: add the ability to install PTEs
From: Lorenzo Stoakes @ 2024-10-24 7:34 UTC (permalink / raw)
To: Andrew Morton
Cc: Suren Baghdasaryan, Liam R . Howlett, Matthew Wilcox,
Vlastimil Babka, Paul E . McKenney, Jann Horn, David Hildenbrand,
linux-mm, linux-kernel, Muchun Song, Richard Henderson,
Matt Turner, Thomas Bogendoerfer, James E . J . Bottomley,
Helge Deller, Chris Zankel, Max Filippov, Arnd Bergmann,
linux-alpha, linux-mips, linux-parisc, linux-arch, Shuah Khan,
Christian Brauner, linux-kselftest, Sidhartha Kumar, Jeff Xu,
Christoph Hellwig, linux-api, John Hubbard
In-Reply-To: <20241023160405.33995c68f20c141325a8fadb@linux-foundation.org>
On Wed, Oct 23, 2024 at 04:04:05PM -0700, Andrew Morton wrote:
> On Wed, 23 Oct 2024 17:24:38 +0100 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> >
> > ...
> >
> > Existing mechanism for performing a walk which also installs page table
> > entries if necessary are heavily duplicated throughout the kernel,
>
> How complicated is it to migrate those to use this?
I would say probably somewhat difficult as very often people are doing quite
custom things, but I will take a look at seeing if we can't make things a little
more generic.
I am also mildly motivated to look at trying to find a generic way to do
replaces...
Both on the TODO!
>
> > ...
> >
> > We explicitly do not allow the existing page walk API to expose this
> > feature as it is dangerous and intended for internal mm use only. Therefore
> > we provide a new walk_page_range_mm() function exposed only to
> > mm/internal.h.
> >
>
> Is this decision compatible with the above migrations?
Yes, nobody but mm should be doing this. You have to be enormously careful about
locks, caches, accounting and the various other machinery around actually
setting a PTE, and this allows you to establish entirely new mappings, so could
very much be abused.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox