* [RFC PATCH v2 24/29] selftests: ntsync: Add some tests for auto-reset event state.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
Test event-specific ioctls NTSYNC_IOC_EVENT_SET, NTSYNC_IOC_EVENT_RESET,
NTSYNC_IOC_EVENT_PULSE, NTSYNC_IOC_EVENT_READ for auto-reset events, and
waiting on auto-reset events.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 59 +++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 98fc70a9a58b..f1fb28949367 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -421,6 +421,65 @@ TEST(manual_event_state)
close(fd);
}
+TEST(auto_event_state)
+{
+ struct ntsync_event_args event_args;
+ __u32 index, signaled;
+ int fd, event, ret;
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ event_args.manual = 0;
+ event_args.signaled = 1;
+ event_args.event = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, event_args.event);
+ event = event_args.event;
+
+ check_event_state(event, 1, 0);
+
+ signaled = 0xdeadbeef;
+ ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, signaled);
+ check_event_state(event, 1, 0);
+
+ ret = wait_any(fd, 1, &event, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_event_state(event, 0, 0);
+
+ signaled = 0xdeadbeef;
+ ret = ioctl(event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+ check_event_state(event, 0, 0);
+
+ ret = wait_any(fd, 1, &event, 123, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ ret = ioctl(event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+
+ ret = ioctl(event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, signaled);
+ check_event_state(event, 0, 0);
+
+ ret = ioctl(event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+ check_event_state(event, 0, 0);
+
+ close(event);
+
+ close(fd);
+}
+
TEST(test_wait_any)
{
struct ntsync_mutex_args mutex_args = {0};
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v2 09/29] ntsync: Introduce NTSYNC_IOC_CREATE_EVENT.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
This correspond to the NT syscall NtCreateEvent().
An NT event holds a single bit of state denoting whether it is signaled or
unsignaled.
There are two types of events: manual-reset and automatic-reset. When an
automatic-reset event is acquired via a wait function, its state is reset to
unsignaled. Manual-reset events are not affected by wait functions.
Whether the event is manual-reset, and its initial state, are specified at
creation time.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
drivers/misc/ntsync.c | 60 +++++++++++++++++++++++++++++++++++++
include/uapi/linux/ntsync.h | 7 +++++
2 files changed, 67 insertions(+)
diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index aadf01c65ca0..c719ddd9f6d7 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -18,6 +18,7 @@
enum ntsync_type {
NTSYNC_TYPE_SEM,
NTSYNC_TYPE_MUTEX,
+ NTSYNC_TYPE_EVENT,
};
struct ntsync_obj {
@@ -39,6 +40,10 @@ struct ntsync_obj {
__u32 owner;
bool ownerdead;
} mutex;
+ struct {
+ bool manual;
+ bool signaled;
+ } event;
} u;
/*
@@ -123,6 +128,8 @@ static bool is_signaled(struct ntsync_obj *obj, __u32 owner)
if (obj->u.mutex.owner && obj->u.mutex.owner != owner)
return false;
return obj->u.mutex.count < UINT_MAX;
+ case NTSYNC_TYPE_EVENT:
+ return obj->u.event.signaled;
}
WARN(1, "bad object type %#x\n", obj->type);
@@ -172,6 +179,10 @@ static void try_wake_all(struct ntsync_device *dev, struct ntsync_q *q,
obj->u.mutex.count++;
obj->u.mutex.owner = q->owner;
break;
+ case NTSYNC_TYPE_EVENT:
+ if (!obj->u.event.manual)
+ obj->u.event.signaled = false;
+ break;
}
}
wake_up_process(q->task);
@@ -238,6 +249,26 @@ static void try_wake_any_mutex(struct ntsync_obj *mutex)
}
}
+static void try_wake_any_event(struct ntsync_obj *event)
+{
+ struct ntsync_q_entry *entry;
+
+ lockdep_assert_held(&event->lock);
+
+ list_for_each_entry(entry, &event->any_waiters, node) {
+ struct ntsync_q *q = entry->q;
+
+ if (!event->u.event.signaled)
+ break;
+
+ if (atomic_cmpxchg(&q->signaled, -1, entry->index) == -1) {
+ if (!event->u.event.manual)
+ event->u.event.signaled = false;
+ wake_up_process(q->task);
+ }
+ }
+}
+
/*
* Actually change the semaphore state, returning -EOVERFLOW if it is made
* invalid.
@@ -544,6 +575,30 @@ static int ntsync_create_mutex(struct ntsync_device *dev, void __user *argp)
return put_user(fd, &user_args->mutex);
}
+static int ntsync_create_event(struct ntsync_device *dev, void __user *argp)
+{
+ struct ntsync_event_args __user *user_args = argp;
+ struct ntsync_event_args args;
+ struct ntsync_obj *event;
+ int fd;
+
+ if (copy_from_user(&args, argp, sizeof(args)))
+ return -EFAULT;
+
+ event = ntsync_alloc_obj(dev, NTSYNC_TYPE_EVENT);
+ if (!event)
+ return -ENOMEM;
+ event->u.event.manual = args.manual;
+ event->u.event.signaled = args.signaled;
+ fd = ntsync_obj_get_fd(event);
+ if (fd < 0) {
+ kfree(event);
+ return fd;
+ }
+
+ return put_user(fd, &user_args->event);
+}
+
static struct ntsync_obj *get_obj(struct ntsync_device *dev, int fd)
{
struct file *file = fget(fd);
@@ -665,6 +720,9 @@ static void try_wake_any_obj(struct ntsync_obj *obj)
case NTSYNC_TYPE_MUTEX:
try_wake_any_mutex(obj);
break;
+ case NTSYNC_TYPE_EVENT:
+ try_wake_any_event(obj);
+ break;
}
}
@@ -857,6 +915,8 @@ static long ntsync_char_ioctl(struct file *file, unsigned int cmd,
void __user *argp = (void __user *)parm;
switch (cmd) {
+ case NTSYNC_IOC_CREATE_EVENT:
+ return ntsync_create_event(dev, argp);
case NTSYNC_IOC_CREATE_MUTEX:
return ntsync_create_mutex(dev, argp);
case NTSYNC_IOC_CREATE_SEM:
diff --git a/include/uapi/linux/ntsync.h b/include/uapi/linux/ntsync.h
index 3861397c6c2f..b8cf503365ef 100644
--- a/include/uapi/linux/ntsync.h
+++ b/include/uapi/linux/ntsync.h
@@ -22,6 +22,12 @@ struct ntsync_mutex_args {
__u32 count;
};
+struct ntsync_event_args {
+ __u32 event;
+ __u32 manual;
+ __u32 signaled;
+};
+
struct ntsync_wait_args {
__u64 timeout;
__u64 objs;
@@ -37,6 +43,7 @@ struct ntsync_wait_args {
#define NTSYNC_IOC_WAIT_ANY _IOWR('N', 0x82, struct ntsync_wait_args)
#define NTSYNC_IOC_WAIT_ALL _IOWR('N', 0x83, struct ntsync_wait_args)
#define NTSYNC_IOC_CREATE_MUTEX _IOWR('N', 0x84, struct ntsync_sem_args)
+#define NTSYNC_IOC_CREATE_EVENT _IOWR('N', 0x87, struct ntsync_event_args)
#define NTSYNC_IOC_SEM_POST _IOWR('N', 0x81, __u32)
#define NTSYNC_IOC_MUTEX_UNLOCK _IOWR('N', 0x85, struct ntsync_mutex_args)
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v2 25/29] selftests: ntsync: Add some tests for wakeup signaling with events.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
Expand the contended wait tests, which previously only covered events and
semaphores, to cover events as well.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 151 +++++++++++++++++-
1 file changed, 147 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index f1fb28949367..598333df3e6d 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -587,6 +587,7 @@ TEST(test_wait_any)
TEST(test_wait_all)
{
+ struct ntsync_event_args event_args = {0};
struct ntsync_mutex_args mutex_args = {0};
struct ntsync_sem_args sem_args = {0};
__u32 owner, index, count;
@@ -609,6 +610,11 @@ TEST(test_wait_all)
EXPECT_EQ(0, ret);
EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ event_args.manual = true;
+ event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
objs[0] = sem_args.sem;
objs[1] = mutex_args.mutex;
@@ -657,6 +663,14 @@ TEST(test_wait_all)
check_sem_state(sem_args.sem, 1, 3);
check_mutex_state(mutex_args.mutex, 1, 123);
+ objs[0] = sem_args.sem;
+ objs[1] = event_args.event;
+ ret = wait_all(fd, 2, objs, 123, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+ check_sem_state(sem_args.sem, 0, 3);
+ check_event_state(event_args.event, 1, 1);
+
/* test waiting on the same object twice */
objs[0] = objs[1] = sem_args.sem;
ret = wait_all(fd, 2, objs, 123, &index);
@@ -665,6 +679,7 @@ TEST(test_wait_all)
close(sem_args.sem);
close(mutex_args.mutex);
+ close(event_args.event);
close(fd);
}
@@ -713,12 +728,13 @@ static int wait_for_thread(pthread_t thread, unsigned int ms)
TEST(wake_any)
{
+ struct ntsync_event_args event_args = {0};
struct ntsync_mutex_args mutex_args = {0};
struct ntsync_wait_args wait_args = {0};
struct ntsync_sem_args sem_args = {0};
struct wait_args thread_args;
+ __u32 count, index, signaled;
int objs[2], fd, ret;
- __u32 count, index;
pthread_t thread;
fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
@@ -800,10 +816,101 @@ TEST(wake_any)
EXPECT_EQ(0, thread_args.ret);
EXPECT_EQ(1, wait_args.index);
+ /* test waking events */
+
+ event_args.manual = false;
+ event_args.signaled = false;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
+ objs[1] = event_args.event;
+ wait_args.timeout = get_abs_timeout(1000);
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+ check_event_state(event_args.event, 0, 0);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(1, wait_args.index);
+
+ wait_args.timeout = get_abs_timeout(1000);
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+ check_event_state(event_args.event, 0, 0);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(1, wait_args.index);
+
+ close(event_args.event);
+
+ event_args.manual = true;
+ event_args.signaled = false;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
+ objs[1] = event_args.event;
+ wait_args.timeout = get_abs_timeout(1000);
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+ check_event_state(event_args.event, 1, 1);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(1, wait_args.index);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, signaled);
+
+ wait_args.timeout = get_abs_timeout(1000);
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_PULSE, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+ check_event_state(event_args.event, 0, 1);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(1, wait_args.index);
+
+ close(event_args.event);
+
/* delete an object while it's being waited on */
wait_args.timeout = get_abs_timeout(200);
wait_args.owner = 123;
+ objs[1] = mutex_args.mutex;
ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
EXPECT_EQ(0, ret);
@@ -823,12 +930,14 @@ TEST(wake_any)
TEST(wake_all)
{
+ struct ntsync_event_args manual_event_args = {0};
+ struct ntsync_event_args auto_event_args = {0};
struct ntsync_mutex_args mutex_args = {0};
struct ntsync_wait_args wait_args = {0};
struct ntsync_sem_args sem_args = {0};
struct wait_args thread_args;
- int objs[2], fd, ret;
- __u32 count, index;
+ __u32 count, index, signaled;
+ int objs[4], fd, ret;
pthread_t thread;
fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
@@ -848,12 +957,24 @@ TEST(wake_all)
EXPECT_EQ(0, ret);
EXPECT_NE(0xdeadbeef, mutex_args.mutex);
+ manual_event_args.manual = true;
+ manual_event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &manual_event_args);
+ EXPECT_EQ(0, ret);
+
+ auto_event_args.manual = false;
+ auto_event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &auto_event_args);
+ EXPECT_EQ(0, ret);
+
objs[0] = sem_args.sem;
objs[1] = mutex_args.mutex;
+ objs[2] = manual_event_args.event;
+ objs[3] = auto_event_args.event;
wait_args.timeout = get_abs_timeout(1000);
wait_args.objs = (uintptr_t)objs;
- wait_args.count = 2;
+ wait_args.count = 4;
wait_args.owner = 456;
thread_args.fd = fd;
thread_args.args = &wait_args;
@@ -887,12 +1008,32 @@ TEST(wake_all)
check_mutex_state(mutex_args.mutex, 0, 0);
+ ret = ioctl(manual_event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, signaled);
+
count = 2;
ret = post_sem(sem_args.sem, &count);
EXPECT_EQ(0, ret);
EXPECT_EQ(0, count);
+ check_sem_state(sem_args.sem, 2, 3);
+
+ ret = ioctl(auto_event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, signaled);
+
+ ret = ioctl(manual_event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+
+ ret = ioctl(auto_event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, signaled);
+
check_sem_state(sem_args.sem, 1, 3);
check_mutex_state(mutex_args.mutex, 1, 456);
+ check_event_state(manual_event_args.event, 1, 1);
+ check_event_state(auto_event_args.event, 0, 0);
ret = wait_for_thread(thread, 100);
EXPECT_EQ(0, ret);
@@ -910,6 +1051,8 @@ TEST(wake_all)
close(sem_args.sem);
close(mutex_args.mutex);
+ close(manual_event_args.event);
+ close(auto_event_args.event);
ret = wait_for_thread(thread, 200);
EXPECT_EQ(0, ret);
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v2 26/29] selftests: ntsync: Add tests for alertable waits.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
Test the "alert" functionality of NTSYNC_IOC_WAIT_ALL and NTSYNC_IOC_WAIT_ANY,
when a wait is woken with an alert and when it is woken by an object.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 179 +++++++++++++++++-
1 file changed, 176 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 598333df3e6d..6c00a55909aa 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -95,7 +95,7 @@ static int read_event_state(int event, __u32 *signaled, __u32 *manual)
})
static int wait_objs(int fd, unsigned long request, __u32 count,
- const int *objs, __u32 owner, __u32 *index)
+ const int *objs, __u32 owner, int alert, __u32 *index)
{
struct ntsync_wait_args args = {0};
struct timespec timeout;
@@ -108,6 +108,7 @@ static int wait_objs(int fd, unsigned long request, __u32 count,
args.objs = (uintptr_t)objs;
args.owner = owner;
args.index = 0xdeadbeef;
+ args.alert = alert;
ret = ioctl(fd, request, &args);
*index = args.index;
return ret;
@@ -115,12 +116,26 @@ static int wait_objs(int fd, unsigned long request, __u32 count,
static int wait_any(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
{
- return wait_objs(fd, NTSYNC_IOC_WAIT_ANY, count, objs, owner, index);
+ return wait_objs(fd, NTSYNC_IOC_WAIT_ANY, count, objs, owner, 0, index);
}
static int wait_all(int fd, __u32 count, const int *objs, __u32 owner, __u32 *index)
{
- return wait_objs(fd, NTSYNC_IOC_WAIT_ALL, count, objs, owner, index);
+ return wait_objs(fd, NTSYNC_IOC_WAIT_ALL, count, objs, owner, 0, index);
+}
+
+static int wait_any_alert(int fd, __u32 count, const int *objs,
+ __u32 owner, int alert, __u32 *index)
+{
+ return wait_objs(fd, NTSYNC_IOC_WAIT_ANY,
+ count, objs, owner, alert, index);
+}
+
+static int wait_all_alert(int fd, __u32 count, const int *objs,
+ __u32 owner, int alert, __u32 *index)
+{
+ return wait_objs(fd, NTSYNC_IOC_WAIT_ALL,
+ count, objs, owner, alert, index);
}
TEST(semaphore_state)
@@ -1062,4 +1077,162 @@ TEST(wake_all)
close(fd);
}
+TEST(alert_any)
+{
+ struct ntsync_event_args event_args = {0};
+ struct ntsync_sem_args sem_args = {0};
+ __u32 index, count, signaled;
+ int objs[2], fd, ret;
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ sem_args.count = 0;
+ sem_args.max = 2;
+ sem_args.sem = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, sem_args.sem);
+ objs[0] = sem_args.sem;
+
+ sem_args.count = 1;
+ sem_args.max = 2;
+ sem_args.sem = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, sem_args.sem);
+ objs[1] = sem_args.sem;
+
+ event_args.manual = true;
+ event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_any_alert(fd, 0, NULL, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_any_alert(fd, 0, NULL, 123, event_args.event, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(1, index);
+
+ ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, index);
+
+ close(event_args.event);
+
+ /* test with an auto-reset event */
+
+ event_args.manual = false;
+ event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
+ count = 1;
+ ret = post_sem(objs[0], &count);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+
+ ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, index);
+
+ ret = wait_any_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ close(event_args.event);
+
+ close(objs[0]);
+ close(objs[1]);
+
+ close(fd);
+}
+
+TEST(alert_all)
+{
+ struct ntsync_event_args event_args = {0};
+ struct ntsync_sem_args sem_args = {0};
+ __u32 index, count, signaled;
+ int objs[2], fd, ret;
+
+ fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
+ ASSERT_LE(0, fd);
+
+ sem_args.count = 2;
+ sem_args.max = 2;
+ sem_args.sem = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, sem_args.sem);
+ objs[0] = sem_args.sem;
+
+ sem_args.count = 1;
+ sem_args.max = 2;
+ sem_args.sem = 0xdeadbeef;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
+ EXPECT_EQ(0, ret);
+ EXPECT_NE(0xdeadbeef, sem_args.sem);
+ objs[1] = sem_args.sem;
+
+ event_args.manual = true;
+ event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+
+ ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, index);
+
+ close(event_args.event);
+
+ /* test with an auto-reset event */
+
+ event_args.manual = false;
+ event_args.signaled = true;
+ ret = ioctl(fd, NTSYNC_IOC_CREATE_EVENT, &event_args);
+ EXPECT_EQ(0, ret);
+
+ count = 2;
+ ret = post_sem(objs[1], &count);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, index);
+
+ ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(2, index);
+
+ ret = wait_all_alert(fd, 2, objs, 123, event_args.event, &index);
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(ETIMEDOUT, errno);
+
+ close(event_args.event);
+
+ close(objs[0]);
+ close(objs[1]);
+
+ close(fd);
+}
+
TEST_HARNESS_MAIN
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v2 27/29] selftests: ntsync: Add some tests for wakeup signaling via alerts.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
Expand the alert tests to cover alerting a thread mid-wait, to test that the
relevant scheduling logic works correctly.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
.../testing/selftests/drivers/ntsync/ntsync.c | 62 +++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/tools/testing/selftests/drivers/ntsync/ntsync.c b/tools/testing/selftests/drivers/ntsync/ntsync.c
index 6c00a55909aa..09153d0686ac 100644
--- a/tools/testing/selftests/drivers/ntsync/ntsync.c
+++ b/tools/testing/selftests/drivers/ntsync/ntsync.c
@@ -1080,9 +1080,12 @@ TEST(wake_all)
TEST(alert_any)
{
struct ntsync_event_args event_args = {0};
+ struct ntsync_wait_args wait_args = {0};
struct ntsync_sem_args sem_args = {0};
__u32 index, count, signaled;
+ struct wait_args thread_args;
int objs[2], fd, ret;
+ pthread_t thread;
fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
ASSERT_LE(0, fd);
@@ -1130,6 +1133,34 @@ TEST(alert_any)
EXPECT_EQ(0, ret);
EXPECT_EQ(2, index);
+ /* test wakeup via alert */
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+
+ wait_args.timeout = get_abs_timeout(1000);
+ wait_args.objs = (uintptr_t)objs;
+ wait_args.count = 2;
+ wait_args.owner = 123;
+ wait_args.index = 0xdeadbeef;
+ wait_args.alert = event_args.event;
+ thread_args.fd = fd;
+ thread_args.args = &wait_args;
+ thread_args.request = NTSYNC_IOC_WAIT_ANY;
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(2, wait_args.index);
+
close(event_args.event);
/* test with an auto-reset event */
@@ -1166,9 +1197,12 @@ TEST(alert_any)
TEST(alert_all)
{
struct ntsync_event_args event_args = {0};
+ struct ntsync_wait_args wait_args = {0};
struct ntsync_sem_args sem_args = {0};
+ struct wait_args thread_args;
__u32 index, count, signaled;
int objs[2], fd, ret;
+ pthread_t thread;
fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
ASSERT_LE(0, fd);
@@ -1202,6 +1236,34 @@ TEST(alert_all)
EXPECT_EQ(0, ret);
EXPECT_EQ(2, index);
+ /* test wakeup via alert */
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_RESET, &signaled);
+ EXPECT_EQ(0, ret);
+
+ wait_args.timeout = get_abs_timeout(1000);
+ wait_args.objs = (uintptr_t)objs;
+ wait_args.count = 2;
+ wait_args.owner = 123;
+ wait_args.index = 0xdeadbeef;
+ wait_args.alert = event_args.event;
+ thread_args.fd = fd;
+ thread_args.args = &wait_args;
+ thread_args.request = NTSYNC_IOC_WAIT_ALL;
+ ret = pthread_create(&thread, NULL, wait_thread, &thread_args);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(ETIMEDOUT, ret);
+
+ ret = ioctl(event_args.event, NTSYNC_IOC_EVENT_SET, &signaled);
+ EXPECT_EQ(0, ret);
+
+ ret = wait_for_thread(thread, 100);
+ EXPECT_EQ(0, ret);
+ EXPECT_EQ(0, thread_args.ret);
+ EXPECT_EQ(2, wait_args.index);
+
close(event_args.event);
/* test with an auto-reset event */
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v2 28/29] maintainers: Add an entry for ntsync.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
Add myself as maintainer, supported by CodeWeavers.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8d1052fa6a69..7924127d351b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15585,6 +15585,15 @@ T: git https://github.com/Paragon-Software-Group/linux-ntfs3.git
F: Documentation/filesystems/ntfs3.rst
F: fs/ntfs3/
+NTSYNC SYNCHRONIZATION PRIMITIVE DRIVER
+M: Elizabeth Figura <zfigura@codeweavers.com>
+L: wine-devel@winehq.org
+S: Supported
+F: Documentation/userspace-api/ntsync.rst
+F: drivers/misc/ntsync.c
+F: include/uapi/linux/ntsync.h
+F: tools/testing/selftests/drivers/ntsync/
+
NUBUS SUBSYSTEM
M: Finn Thain <fthain@linux-m68k.org>
L: linux-m68k@lists.linux-m68k.org
--
2.43.0
^ permalink raw reply related
* [RFC PATCH v2 29/29] docs: ntsync: Add documentation for the ntsync uAPI.
From: Elizabeth Figura @ 2024-01-31 2:13 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest, Elizabeth Figura
In-Reply-To: <20240131021356.10322-1-zfigura@codeweavers.com>
Add an overall explanation of the driver architecture, and complete and precise
specification for its intended behaviour.
Signed-off-by: Elizabeth Figura <zfigura@codeweavers.com>
---
Documentation/userspace-api/index.rst | 1 +
Documentation/userspace-api/ntsync.rst | 390 +++++++++++++++++++++++++
2 files changed, 391 insertions(+)
create mode 100644 Documentation/userspace-api/ntsync.rst
diff --git a/Documentation/userspace-api/index.rst b/Documentation/userspace-api/index.rst
index 09f61bd2ac2e..f5a72ed27def 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -34,6 +34,7 @@ place where this information is gathered.
tee
isapnp
dcdbas
+ ntsync
.. only:: subproject and html
diff --git a/Documentation/userspace-api/ntsync.rst b/Documentation/userspace-api/ntsync.rst
new file mode 100644
index 000000000000..bda9401f6747
--- /dev/null
+++ b/Documentation/userspace-api/ntsync.rst
@@ -0,0 +1,390 @@
+===================================
+NT synchronization primitive driver
+===================================
+
+This page documents the user-space API for the ntsync driver.
+
+ntsync is a support driver for emulation of NT synchronization
+primitives by user-space NT emulators. It exists because implementation
+in user-space, using existing tools, cannot match Windows performance
+while offering accurate semantics. It is implemented entirely in
+software, and does not drive any hardware device.
+
+This interface is meant as a compatibility tool only, and should not
+be used for general synchronization. Instead use generic, versatile
+interfaces such as futex(2) and poll(2).
+
+Synchronization primitives
+==========================
+
+The ntsync driver exposes three types of synchronization primitives:
+semaphores, mutexes, and events.
+
+A semaphore holds a single volatile 32-bit counter, and a static 32-bit
+integer denoting the maximum value. It is considered signaled when the
+counter is nonzero. The counter is decremented by one when a wait is
+satisfied. Both the initial and maximum count are established when the
+semaphore is created.
+
+A mutex holds a volatile 32-bit recursion count, and a volatile 32-bit
+identifier denoting its owner. A mutex is considered signaled when its
+owner is zero (indicating that it is not owned). The recursion count is
+incremented when a wait is satisfied, and ownership is set to the given
+identifier.
+
+A mutex also holds an internal flag denoting whether its previous owner
+has died; such a mutex is said to be abandoned. Owner death is not
+tracked automatically based on thread death, but rather must be
+communicated using ``NTSYNC_IOC_MUTEX_KILL``. An abandoned mutex is
+inherently considered unowned.
+
+Except for the "unowned" semantics of zero, the actual value of the
+owner identifier is not interpreted by the ntsync driver at all. The
+intended use is to store a thread identifier; however, the ntsync
+driver does not actually validate that a calling thread provides
+consistent or unique identifiers.
+
+An event holds a volatile boolean state denoting whether it is signaled
+or not. There are two types of events, auto-reset and manual-reset. An
+auto-reset event is designaled when a wait is satisfied; a manual-reset
+event is not. The event type is specified when the event is created.
+
+Unless specified otherwise, all operations on an object are atomic and
+totally ordered with respect to other operations on the same object.
+
+Objects are represented by files. When all file descriptors to an
+object are closed, that object is deleted.
+
+Char device
+===========
+
+The ntsync driver creates a single char device /dev/ntsync. Each file
+description opened on the device represents a unique instance intended
+to back an individual NT virtual machine. Objects created by one ntsync
+instance may only be used with other objects created by the same
+instance.
+
+ioctl reference
+===============
+
+All operations on the device are done through ioctls. There are four
+structures used in ioctl calls::
+
+ struct ntsync_sem_args {
+ __u32 sem;
+ __u32 count;
+ __u32 max;
+ };
+
+ struct ntsync_mutex_args {
+ __u32 mutex;
+ __u32 owner;
+ __u32 count;
+ };
+
+ struct ntsync_event_args {
+ __u32 event;
+ __u32 signaled;
+ __u32 manual;
+ };
+
+ struct ntsync_wait_args {
+ __u64 timeout;
+ __u64 objs;
+ __u32 count;
+ __u32 owner;
+ __u32 index;
+ __u32 alert;
+ };
+
+Depending on the ioctl, members of the structure may be used as input,
+output, or not at all. All ioctls return 0 on success.
+
+The ioctls on the device file are as follows:
+
+.. c:macro:: NTSYNC_IOC_CREATE_SEM
+
+ Create a semaphore object. Takes a pointer to struct
+ :c:type:`ntsync_sem_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``sem``
+ - On output, contains a file descriptor to the created semaphore.
+ * - ``count``
+ - Initial count of the semaphore.
+ * - ``max``
+ - Maximum count of the semaphore.
+
+ Fails with ``EINVAL`` if ``count`` is greater than ``max``.
+
+.. c:macro:: NTSYNC_IOC_CREATE_MUTEX
+
+ Create a mutex object. Takes a pointer to struct
+ :c:type:`ntsync_mutex_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``mutex``
+ - On output, contains a file descriptor to the created mutex.
+ * - ``count``
+ - Initial recursion count of the mutex.
+ * - ``owner``
+ - Initial owner of the mutex.
+
+ If ``owner`` is nonzero and ``count`` is zero, or if ``owner`` is
+ zero and ``count`` is nonzero, the function fails with ``EINVAL``.
+
+.. c:macro:: NTSYNC_IOC_CREATE_EVENT
+
+ Create an event object. Takes a pointer to struct
+ :c:type:`ntsync_event_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``event``
+ - On output, contains a file descriptor to the created event.
+ * - ``signaled``
+ - If nonzero, the event is initially signaled, otherwise
+ nonsignaled.
+ * - ``manual``
+ - If nonzero, the event is a manual-reset event, otherwise
+ auto-reset.
+
+The ioctls on the individual objects are as follows:
+
+.. c:macro:: NTSYNC_IOC_SEM_POST
+
+ Post to a semaphore object. Takes a pointer to a 32-bit integer,
+ which on input holds the count to be added to the semaphore, and on
+ output contains its previous count.
+
+ If adding to the semaphore's current count would raise the latter
+ past the semaphore's maximum count, the ioctl fails with
+ ``EOVERFLOW`` and the semaphore is not affected. If raising the
+ semaphore's count causes it to become signaled, eligible threads
+ waiting on this semaphore will be woken and the semaphore's count
+ decremented appropriately.
+
+.. c:macro:: NTSYNC_IOC_MUTEX_UNLOCK
+
+ Release a mutex object. Takes a pointer to struct
+ :c:type:`ntsync_mutex_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``mutex``
+ - Ignored.
+ * - ``owner``
+ - Specifies the owner trying to release this mutex.
+ * - ``count``
+ - On output, contains the previous recursion count.
+
+ If ``owner`` is zero, the ioctl fails with ``EINVAL``. If ``owner``
+ is not the current owner of the mutex, the ioctl fails with
+ ``EPERM``.
+
+ The mutex's count will be decremented by one. If decrementing the
+ mutex's count causes it to become zero, the mutex is marked as
+ unowned and signaled, and eligible threads waiting on it will be
+ woken as appropriate.
+
+.. c:macro:: NTSYNC_IOC_SET_EVENT
+
+ Signal an event object. Takes a pointer to a 32-bit integer, which on
+ output contains the previous state of the event.
+
+ Eligible threads will be woken, and auto-reset events will be
+ designaled appropriately.
+
+.. c:macro:: NTSYNC_IOC_RESET_EVENT
+
+ Designal an event object. Takes a pointer to a 32-bit integer, which
+ on output contains the previous state of the event.
+
+.. c:macro:: NTSYNC_IOC_PULSE_EVENT
+
+ Wake threads waiting on an event object while leaving it in an
+ unsignaled state. Takes a pointer to a 32-bit integer, which on
+ output contains the previous state of the event.
+
+ A pulse operation can be thought of as a set followed by a reset,
+ performed as a single atomic operation. If two threads are waiting on
+ an auto-reset event which is pulsed, only one will be woken. If two
+ threads are waiting a manual-reset event which is pulsed, both will
+ be woken. However, in both cases, the event will be unsignaled
+ afterwards, and a simultaneous read operation will always report the
+ event as unsignaled.
+
+.. c:macro:: NTSYNC_IOC_READ_SEM
+
+ Read the current state of a semaphore object. Takes a pointer to
+ struct :c:type:`ntsync_sem_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``sem``
+ - Ignored.
+ * - ``count``
+ - On output, contains the current count of the semaphore.
+ * - ``max``
+ - On output, contains the maximum count of the semaphore.
+
+.. c:macro:: NTSYNC_IOC_READ_MUTEX
+
+ Read the current state of a mutex object. Takes a pointer to struct
+ :c:type:`ntsync_mutex_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``mutex``
+ - Ignored.
+ * - ``owner``
+ - On output, contains the current owner of the mutex, or zero
+ if the mutex is not currently owned.
+ * - ``count``
+ - On output, contains the current recursion count of the mutex.
+
+ If the mutex is marked as abandoned, the function fails with
+ ``EOWNERDEAD``. In this case, ``count`` and ``owner`` are set to
+ zero.
+
+.. c:macro:: NTSYNC_IOC_READ_EVENT
+
+ Read the current state of an event object. Takes a pointer to struct
+ :c:type:`ntsync_event_args`, which is used as follows:
+
+ .. list-table::
+
+ * - ``event``
+ - Ignored.
+ * - ``signaled``
+ - On output, contains the current state of the event.
+ * - ``manual``
+ - On output, contains 1 if the event is a manual-reset event,
+ and 0 otherwise.
+
+.. c:macro:: NTSYNC_IOC_KILL_OWNER
+
+ Mark a mutex as unowned and abandoned if it is owned by the given
+ owner. Takes an input-only pointer to a 32-bit integer denoting the
+ owner. If the owner is zero, the ioctl fails with ``EINVAL``. If the
+ owner does not own the mutex, the function fails with ``EPERM``.
+
+ Eligible threads waiting on the mutex will be woken as appropriate
+ (and such waits will fail with ``EOWNERDEAD``, as described below).
+
+.. c:macro:: NTSYNC_IOC_WAIT_ANY
+
+ Poll on any of a list of objects, atomically acquiring at most one.
+ Takes a pointer to struct :c:type:`ntsync_wait_args`, which is
+ used as follows:
+
+ .. list-table::
+
+ * - ``timeout``
+ - Absolute timeout in nanoseconds, measured against the
+ MONOTONIC clock. If the timeout is equal to or earlier than
+ the current time, the function returns immediately without
+ sleeping. If ``timeout`` is U64_MAX, the function will sleep
+ until an object is signaled, and will not fail with
+ ``ETIMEDOUT``.
+ * - ``objs``
+ - Pointer to an array of ``count`` file descriptors
+ (specified as an integer so that the structure has the same
+ size regardless of architecture). If any object is
+ invalid, the function fails with ``EINVAL``.
+ * - ``count``
+ - Number of objects specified in the ``objs`` array.
+ If greater than ``NTSYNC_MAX_WAIT_COUNT``, the function fails
+ with ``EINVAL``.
+ * - ``owner``
+ - Mutex owner identifier. If any object in ``objs`` is a mutex,
+ the ioctl will attempt to acquire that mutex on behalf of
+ ``owner``. If ``owner`` is zero, the ioctl fails with
+ ``EINVAL``.
+ * - ``index``
+ - On success, contains the index (into ``objs``) of the object
+ which was signaled. If ``alert`` was signaled instead,
+ this contains ``count``.
+ * - ``alert``
+ - Optional event object file descriptor. If nonzero, this
+ specifies an "alert" event object which, if signaled, will
+ terminate the wait. If nonzero, the identifier must point to a
+ valid event.
+
+ This function attempts to acquire one of the given objects. If unable
+ to do so, it sleeps until an object becomes signaled, subsequently
+ acquiring it, or the timeout expires. In the latter case the ioctl
+ fails with ``ETIMEDOUT``. The function only acquires one object, even
+ if multiple objects are signaled.
+
+ A semaphore is considered to be signaled if its count is nonzero, and
+ is acquired by decrementing its count by one. A mutex is considered
+ to be signaled if it is unowned or if its owner matches the ``owner``
+ argument, and is acquired by incrementing its recursion count by one
+ and setting its owner to the ``owner`` argument. An auto-reset event
+ is acquired by designaling it; a manual-reset event is not affected
+ by acquisition.
+
+ Acquisition is atomic and totally ordered with respect to other
+ operations on the same object. If two wait operations (with different
+ ``owner`` identifiers) are queued on the same mutex, only one is
+ signaled. If two wait operations are queued on the same semaphore,
+ and a value of one is posted to it, only one is signaled. The order
+ in which threads are signaled is not specified.
+
+ If an abandoned mutex is acquired, the ioctl fails with
+ ``EOWNERDEAD``. Although this is a failure return, the function may
+ otherwise be considered successful. The mutex is marked as owned by
+ the given owner (with a recursion count of 1) and as no longer
+ abandoned, and ``index`` is still set to the index of the mutex.
+
+ The ``alert`` argument is an "extra" event which can terminate the
+ wait, independently of all other objects. If members of ``objs`` and
+ ``alert`` are both simultaneously signaled, a member of ``objs`` will
+ always be given priority and acquired first.
+
+ It is valid to pass the same object more than once, including by
+ passing the same event in the ``objs`` array and in ``alert``. If a
+ wakeup occurs due to that object being signaled, ``index`` is set to
+ the lowest index corresponding to that object.
+
+ The function may fail with ``EINTR`` if a signal is received.
+
+.. c:macro:: NTSYNC_IOC_WAIT_ALL
+
+ Poll on a list of objects, atomically acquiring all of them. Takes a
+ pointer to struct :c:type:`ntsync_wait_args`, which is used
+ identically to ``NTSYNC_IOC_WAIT_ANY``, except that ``index`` is
+ always filled with zero on success if not woken via alert.
+
+ This function attempts to simultaneously acquire all of the given
+ objects. If unable to do so, it sleeps until all objects become
+ simultaneously signaled, subsequently acquiring them, or the timeout
+ expires. In the latter case the ioctl fails with ``ETIMEDOUT`` and no
+ objects are modified.
+
+ Objects may become signaled and subsequently designaled (through
+ acquisition by other threads) while this thread is sleeping. Only
+ once all objects are simultaneously signaled does the ioctl acquire
+ them and return. The entire acquisition is atomic and totally ordered
+ with respect to other operations on any of the given objects.
+
+ If an abandoned mutex is acquired, the ioctl fails with
+ ``EOWNERDEAD``. Similarly to ``NTSYNC_IOC_WAIT_ANY``, all objects are
+ nevertheless marked as acquired. Note that if multiple mutex objects
+ are specified, there is no way to know which were marked as
+ abandoned.
+
+ As with "any" waits, the ``alert`` argument is an "extra" event which
+ can terminate the wait. Critically, however, an "all" wait will
+ succeed if all members in ``objs`` are signaled, *or* if ``alert`` is
+ signaled. In the latter case ``index`` will be set to ``count``. As
+ with "any" waits, if both conditions are filled, the former takes
+ priority, and objects in ``objs`` will be acquired.
+
+ Unlike ``NTSYNC_IOC_WAIT_ANY``, it is not valid to pass the same
+ object more than once, nor is it valid to pass the same object in
+ ``objs`` and in ``alert``. If this is attempted, the function fails
+ with ``EINVAL``.
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v4 3/3] eventpoll: Add epoll ioctl for epoll_params
From: Joe Damato @ 2024-01-31 2:27 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel, netdev, chuck.lever, jlayton, linux-api, brauner,
edumazet, davem, alexander.duyck, sridhar.samudrala, kuba,
willemdebruijn.kernel, weiwan, David.Laight, arnd,
Jonathan Corbet, Alexander Viro, Jan Kara, Michael Ellerman,
Nathan Lynch, Thomas Zimmermann, Maik Broemme, Steve French,
Julien Panis, Jiri Slaby, Thomas Huth, Andrew Waterman, Albert Ou,
Palmer Dabbelt, open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <2024013001-prison-strum-899d@gregkh>
On Tue, Jan 30, 2024 at 06:08:36PM -0800, Greg Kroah-Hartman wrote:
> On Wed, Jan 31, 2024 at 01:47:33AM +0000, Joe Damato wrote:
> > +struct epoll_params {
> > + __aligned_u64 busy_poll_usecs;
> > + __u16 busy_poll_budget;
> > +
> > + /* pad the struct to a multiple of 64bits for alignment on all arches */
> > + __u8 __pad[6];
>
> You HAVE to check this padding to be sure it is all 0, otherwise it can
> never be used in the future for anything.
Is there some preferred mechanism for this in the kernel that I should be
using or is this as simple as adding a for loop to check each u8 == 0 ?
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] eventpoll: Add epoll ioctl for epoll_params
From: Greg Kroah-Hartman @ 2024-01-31 2:31 UTC (permalink / raw)
To: Joe Damato
Cc: linux-kernel, netdev, chuck.lever, jlayton, linux-api, brauner,
edumazet, davem, alexander.duyck, sridhar.samudrala, kuba,
willemdebruijn.kernel, weiwan, David.Laight, arnd,
Jonathan Corbet, Alexander Viro, Jan Kara, Michael Ellerman,
Nathan Lynch, Thomas Zimmermann, Maik Broemme, Steve French,
Julien Panis, Jiri Slaby, Thomas Huth, Andrew Waterman, Albert Ou,
Palmer Dabbelt, open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20240131022756.GA4837@fastly.com>
On Tue, Jan 30, 2024 at 06:27:57PM -0800, Joe Damato wrote:
> On Tue, Jan 30, 2024 at 06:08:36PM -0800, Greg Kroah-Hartman wrote:
> > On Wed, Jan 31, 2024 at 01:47:33AM +0000, Joe Damato wrote:
> > > +struct epoll_params {
> > > + __aligned_u64 busy_poll_usecs;
> > > + __u16 busy_poll_budget;
> > > +
> > > + /* pad the struct to a multiple of 64bits for alignment on all arches */
> > > + __u8 __pad[6];
> >
> > You HAVE to check this padding to be sure it is all 0, otherwise it can
> > never be used in the future for anything.
>
> Is there some preferred mechanism for this in the kernel that I should be
> using or is this as simple as adding a for loop to check each u8 == 0 ?
It's as simple as a loop :)
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] eventpoll: Add epoll ioctl for epoll_params
From: Dave Chinner @ 2024-01-31 2:57 UTC (permalink / raw)
To: Joe Damato
Cc: Greg Kroah-Hartman, linux-kernel, netdev, chuck.lever, jlayton,
linux-api, brauner, edumazet, davem, alexander.duyck,
sridhar.samudrala, kuba, willemdebruijn.kernel, weiwan,
David.Laight, arnd, Jonathan Corbet, Alexander Viro, Jan Kara,
Michael Ellerman, Nathan Lynch, Thomas Zimmermann, Maik Broemme,
Steve French, Julien Panis, Jiri Slaby, Thomas Huth,
Andrew Waterman, Albert Ou, Palmer Dabbelt,
open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20240131022756.GA4837@fastly.com>
On Tue, Jan 30, 2024 at 06:27:57PM -0800, Joe Damato wrote:
> On Tue, Jan 30, 2024 at 06:08:36PM -0800, Greg Kroah-Hartman wrote:
> > On Wed, Jan 31, 2024 at 01:47:33AM +0000, Joe Damato wrote:
> > > +struct epoll_params {
> > > + __aligned_u64 busy_poll_usecs;
> > > + __u16 busy_poll_budget;
> > > +
> > > + /* pad the struct to a multiple of 64bits for alignment on all arches */
> > > + __u8 __pad[6];
> >
> > You HAVE to check this padding to be sure it is all 0, otherwise it can
> > never be used in the future for anything.
>
> Is there some preferred mechanism for this in the kernel that I should be
> using or is this as simple as adding a for loop to check each u8 == 0 ?
memchr_inv()
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-01-31 5:12 UTC (permalink / raw)
To: Gregory Price
Cc: linux-mm, linux-kernel, linux-doc, linux-fsdevel, linux-api,
corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji, mhocko,
ying.huang, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <20240130182046.74278-4-gregory.price@memverge.com>
On Tue, Jan 30, 2024 at 01:20:46PM -0500, Gregory Price wrote:
> + /* Continue allocating from most recent node and adjust the nr_pages */
> + node = me->il_prev;
> + weight = me->il_weight;
> + if (weight && node_isset(node, nodes)) {
> + node_pages = min(rem_pages, weight);
> + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> + NULL, page_array);
> + page_array += nr_allocated;
> + total_allocated += nr_allocated;
> + /* if that's all the pages, no need to interleave */
> + if (rem_pages < weight) {
> + /* stay on current node, adjust il_weight */
> + me->il_weight -= rem_pages;
> + return total_allocated;
> + } else if (rem_pages == weight) {
> + /* move to next node / weight */
> + me->il_prev = next_node_in(node, nodes);
> + me->il_weight = get_il_weight(next_node);
Sigh, I managed to miss a small update that killed next_node in favor of
operating directly on il_prev. Can you squash this fix into the patch?
Otherwise I can submit a separate patch.
~Gregory
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 7cd92f4ec0d7..2c1aef8eab70 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2382,7 +2382,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
unsigned int weight_total = 0;
unsigned long rem_pages = nr_pages;
nodemask_t nodes;
- int nnodes, node, next_node;
+ int nnodes, node;
int resume_node = MAX_NUMNODES - 1;
u8 resume_weight = 0;
int prev_node;
@@ -2412,7 +2412,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
} else if (rem_pages == weight) {
/* move to next node / weight */
me->il_prev = next_node_in(node, nodes);
- me->il_weight = get_il_weight(next_node);
+ me->il_weight = get_il_weight(me->il_prev);
return total_allocated;
}
/* Otherwise we adjust remaining pages, continue from there */
^ permalink raw reply related
* Re: [PATCH 4/5] Documentation/ABI/README: convert to ReST
From: Mauro Carvalho Chehab @ 2024-01-31 5:42 UTC (permalink / raw)
To: Vegard Nossum
Cc: Jonathan Corbet, Randy Dunlap, Greg Kroah-Hartman, Jani Nikula,
linux-doc, linux-api
In-Reply-To: <c77c1875-5e32-491a-8abf-502ebe371eb5@oracle.com>
Em Mon, 8 Jan 2024 14:18:55 +0100
Vegard Nossum <vegard.nossum@oracle.com> escreveu:
> On 05/01/2024 21:07, Mauro Carvalho Chehab wrote:
> > Em Thu, 4 Jan 2024 17:09:45 +0100
> > Vegard Nossum <vegard.nossum@oracle.com> escreveu:
> >> +Every file in these directories will contain the following information::
> >> +
> >> + What: Short description of the interface
> >> + Date: Date created
> >> + KernelVersion: Kernel version this feature first showed up in.
> >> + Contact: Primary contact for this interface (may be a mailing list)
> >> + Description: Long description of the interface and how to use it.
> >> + Users: All users of this interface who wish to be notified when
> >> + it changes. This is very important for interfaces in
> >> + the "testing" stage, so that kernel developers can work
> >> + with userspace developers to ensure that things do not
> >> + break in ways that are unacceptable. It is also
> >> + important to get feedback for these interfaces to make
> >> + sure they are working in a proper way and do not need to
> >> + be changed further.
> >
> > My personal preference would be to use:
> >
> > :What:
> >
> > as this produces a better markup.
>
> I would prefer to alter this as little as possible, since it describes
> the literal format of those ABI files, keeping it readable and
> understandable in plain text as well as HTML -- with a single leading
> space this whole block shows up as a code block in the HTML, which I
> think is appropriate when giving an example of a literal file.
Well, you're still not being strict by adding a single space after
the field. That's OK for ReST, but if one uses it as a template, the
extra space will cause problems.
Btw, in the specific case of this code block, there is one alternative
approach: keep it untouched and create a new ReST file on a similar
approach to what it was done at Documentation/core-api/wrappers/, e. g.:
.. SPDX-License-Identifier: GPL-2.0
This is a simple wrapper to bring ABI/README into the RST world.
<snip>
===============================
Linux userspace ABI description
===============================
.. raw:: latex
\footnotesize
.. include:: ../../ABI/README
:literal:
.. raw:: latex
\normalsize
</snip>
While I don't like very much this approach, in this very specific
case, it is justified, at least for the field description.
(Note: the latex part to change the font size may not be needed - it will
depend on how this file will appear at the pdf version)
> >> diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
> >> index b1bc2d37bd0a..7e6198ab368d 100644
> >> --- a/Documentation/process/submit-checklist.rst
> >> +++ b/Documentation/process/submit-checklist.rst
> >> @@ -85,7 +85,7 @@ and elsewhere regarding submitting Linux kernel patches.
> >> 17) All new module parameters are documented with ``MODULE_PARM_DESC()``
> >>
> >> 18) All new userspace interfaces are documented in ``Documentation/ABI/``.
> >> - See ``Documentation/ABI/README`` for more information.
> >> + See ``Documentation/ABI/README.rst`` for more information.
> >
> > If you're willing to convert to ReST, please remove ``, as this will
> > let automarkup.py to create cross-reference links. Same note for the
> > translations too.
>
> Good point -- Jon, do you want me to resubmit this or can you fix it up?
>
> We could also just run a "treewide" fix for this as a separate patch:
>
> git grep -l '``Documentation/.*\.rst``' 'Documentation/**.rst' \
> | xargs sed -i 's|``\(Documentation[^`*]*\.rst\)``|\1|g'
Doing it globally won't work, as there are a few cases where `` is needed:
- when there are wildcards at the file name, like:
Documentation/driver-api/usb/power-management.rst:covered to some extent (see ``Documentation/power/*.rst`` for more
- when they don't point to the actual docs, like:
Documentation/doc-guide/sphinx.rst:documentation is under ``Documentation/gpu``, split to several ``.rst`` files,
- on some cases, it may require a different approach, or may not
make sense, like here:
Documentation/doc-guide/sphinx.rst:2. Refer to it from the Sphinx main `TOC tree`_ in ``Documentation/index.rst``.
(the `Toc tree`_ is already an cross-reference link. So, it OK to keep
``Documentation/index.rst`` to help people reading at the sources)
- when it points, for instance, to ./tools/*/Documentation, as those
are currently outside the scope of the ReST docs. Not sure if we
still have this at the docs
Thanks,
Mauro
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] eventpoll: Add epoll ioctl for epoll_params
From: Jiri Slaby @ 2024-01-31 6:03 UTC (permalink / raw)
To: Joe Damato, Greg Kroah-Hartman
Cc: linux-kernel, netdev, chuck.lever, jlayton, linux-api, brauner,
edumazet, davem, alexander.duyck, sridhar.samudrala, kuba,
willemdebruijn.kernel, weiwan, David.Laight, arnd,
Jonathan Corbet, Alexander Viro, Jan Kara, Michael Ellerman,
Nathan Lynch, Thomas Zimmermann, Maik Broemme, Steve French,
Julien Panis, Thomas Huth, Andrew Waterman, Albert Ou,
Palmer Dabbelt, open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20240131022756.GA4837@fastly.com>
On 31. 01. 24, 3:27, Joe Damato wrote:
> On Tue, Jan 30, 2024 at 06:08:36PM -0800, Greg Kroah-Hartman wrote:
>> On Wed, Jan 31, 2024 at 01:47:33AM +0000, Joe Damato wrote:
>>> +struct epoll_params {
>>> + __aligned_u64 busy_poll_usecs;
>>> + __u16 busy_poll_budget;
>>> +
>>> + /* pad the struct to a multiple of 64bits for alignment on all arches */
>>> + __u8 __pad[6];
>>
>> You HAVE to check this padding to be sure it is all 0, otherwise it can
>> never be used in the future for anything.
>
> Is there some preferred mechanism for this in the kernel that I should be
> using or is this as simple as adding a for loop to check each u8 == 0 ?
You are likely looking for memchr_inv().
--
js
suse labs
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Huang, Ying @ 2024-01-31 6:43 UTC (permalink / raw)
To: Gregory Price
Cc: linux-mm, linux-kernel, linux-doc, linux-fsdevel, linux-api,
corbet, akpm, gregory.price, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <20240130182046.74278-4-gregory.price@memverge.com>
Gregory Price <gourry.memverge@gmail.com> writes:
> When a system has multiple NUMA nodes and it becomes bandwidth hungry,
> using the current MPOL_INTERLEAVE could be an wise option.
>
> However, if those NUMA nodes consist of different types of memory such
> as socket-attached DRAM and CXL/PCIe attached DRAM, the round-robin
> based interleave policy does not optimally distribute data to make use
> of their different bandwidth characteristics.
>
> Instead, interleave is more effective when the allocation policy follows
> each NUMA nodes' bandwidth weight rather than a simple 1:1 distribution.
>
> This patch introduces a new memory policy, MPOL_WEIGHTED_INTERLEAVE,
> enabling weighted interleave between NUMA nodes. Weighted interleave
> allows for proportional distribution of memory across multiple numa
> nodes, preferably apportioned to match the bandwidth of each node.
>
> For example, if a system has 1 CPU node (0), and 2 memory nodes (0,1),
> with bandwidth of (100GB/s, 50GB/s) respectively, the appropriate
> weight distribution is (2:1).
>
> Weights for each node can be assigned via the new sysfs extension:
> /sys/kernel/mm/mempolicy/weighted_interleave/
>
> For now, the default value of all nodes will be `1`, which matches
> the behavior of standard 1:1 round-robin interleave. An extension
> will be added in the future to allow default values to be registered
> at kernel and device bringup time.
>
> The policy allocates a number of pages equal to the set weights. For
> example, if the weights are (2,1), then 2 pages will be allocated on
> node0 for every 1 page allocated on node1.
>
> The new flag MPOL_WEIGHTED_INTERLEAVE can be used in set_mempolicy(2)
> and mbind(2).
>
> Some high level notes about the pieces of weighted interleave:
>
> current->il_prev:
> Default interleave uses this to track the last used node.
> Weighted interleave uses this to track the *current* node, and
> when weight reaches 0 it will be used to acquire the next node.
>
> current->il_weight:
> The active weight of the current node (current->il_prev)
> When this reaches 0, current->il_prev is set to the next node
> and current->il_weight is set to the next weight.
I still think that my description of the 2 fields above is easier to be
understood. For weighted interleave,
current->il_prev is the node from which we allocated page in previous
allocation.
current->il_weight is the remaining weight for current->il_prev after
previous allocation.
But I will not force you to use this. Use it only if you think that
they are better.
> weighted_interleave_nodes:
> Counts the number of allocations as they occur, and applies the
> weight for the current node. When the weight reaches 0, switch
> to the next node. Operates only on task->mempolicy.
>
> weighted_interleave_nid:
> Gets the total weight of the nodemask as well as each individual
> node weight, then calculates the node based on the given index.
> Operates on VMA policies.
>
> bulk_array_weighted_interleave:
> Gets the total weight of the nodemask as well as each individual
> node weight, then calculates the number of "interleave rounds" as
> well as any delta ("partial round"). Calculates the number of
> pages for each node and allocates them.
>
> If a node was scheduled for interleave via interleave_nodes, the
> current weight will be allocated first.
>
> Operates only on the task->mempolicy.
>
> One piece of complexity is the interaction between a recent refactor
> which split the logic to acquire the "ilx" (interleave index) of an
> allocation and the actual application of the interleave. If a call
> to alloc_pages_mpol() were made with a weighted-interleave policy and
> ilx set to NO_INTERLEAVE_INDEX, weighted_interleave_nodes() would
> operate on a VMA policy - violating the description above.
>
> An inspection of all callers of alloc_pages_mpol() shows that all
> external callers set ilx to `0`, an index value, or will call
> get_vma_policy() to acquire the ilx.
>
> For example, mm/shmem.c may call into alloc_pages_mpol. The call stacks
> all set (pgoff_t ilx) or end up in `get_vma_policy()`. This enforces
> the `weighted_interleave_nodes()` and `weighted_interleave_nid()`
> policy requirements (task/vma respectively).
>
> Suggested-by: Hasan Al Maruf <Hasan.Maruf@amd.com>
> Signed-off-by: Gregory Price <gregory.price@memverge.com>
> Co-developed-by: Rakie Kim <rakie.kim@sk.com>
> Signed-off-by: Rakie Kim <rakie.kim@sk.com>
> Co-developed-by: Honggyu Kim <honggyu.kim@sk.com>
> Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
> Co-developed-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
> Signed-off-by: Hyeongtak Ji <hyeongtak.ji@sk.com>
> Co-developed-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> Co-developed-by: Ravi Jonnalagadda <ravis.opensrc@micron.com>
> Signed-off-by: Ravi Jonnalagadda <ravis.opensrc@micron.com>
> ---
> .../admin-guide/mm/numa_memory_policy.rst | 9 +
> include/linux/sched.h | 1 +
> include/uapi/linux/mempolicy.h | 1 +
> mm/mempolicy.c | 231 +++++++++++++++++-
> 4 files changed, 238 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/mm/numa_memory_policy.rst b/Documentation/admin-guide/mm/numa_memory_policy.rst
> index eca38fa81e0f..a70f20ce1ffb 100644
> --- a/Documentation/admin-guide/mm/numa_memory_policy.rst
> +++ b/Documentation/admin-guide/mm/numa_memory_policy.rst
> @@ -250,6 +250,15 @@ MPOL_PREFERRED_MANY
> can fall back to all existing numa nodes. This is effectively
> MPOL_PREFERRED allowed for a mask rather than a single node.
>
> +MPOL_WEIGHTED_INTERLEAVE
> + This mode operates the same as MPOL_INTERLEAVE, except that
> + interleaving behavior is executed based on weights set in
> + /sys/kernel/mm/mempolicy/weighted_interleave/
> +
> + Weighted interleave allocates pages on nodes according to a
> + weight. For example if nodes [0,1] are weighted [5,2], 5 pages
> + will be allocated on node0 for every 2 pages allocated on node1.
> +
> NUMA memory policy supports the following optional mode flags:
>
> MPOL_F_STATIC_NODES
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index ffe8f618ab86..b9ce285d8c9c 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1259,6 +1259,7 @@ struct task_struct {
> /* Protected by alloc_lock: */
> struct mempolicy *mempolicy;
> short il_prev;
> + u8 il_weight;
> short pref_node_fork;
> #endif
> #ifdef CONFIG_NUMA_BALANCING
> diff --git a/include/uapi/linux/mempolicy.h b/include/uapi/linux/mempolicy.h
> index a8963f7ef4c2..1f9bb10d1a47 100644
> --- a/include/uapi/linux/mempolicy.h
> +++ b/include/uapi/linux/mempolicy.h
> @@ -23,6 +23,7 @@ enum {
> MPOL_INTERLEAVE,
> MPOL_LOCAL,
> MPOL_PREFERRED_MANY,
> + MPOL_WEIGHTED_INTERLEAVE,
> MPOL_MAX, /* always last member of enum */
> };
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 3bdfaf03b660..7cd92f4ec0d7 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -19,6 +19,13 @@
> * for anonymous memory. For process policy an process counter
> * is used.
> *
> + * weighted interleave
> + * Allocate memory interleaved over a set of nodes based on
> + * a set of weights (per-node), with normal fallback if it
> + * fails. Otherwise operates the same as interleave.
> + * Example: nodeset(0,1) & weights (2,1) - 2 pages allocated
> + * on node 0 for every 1 page allocated on node 1.
> + *
> * bind Only allocate memory on a specific set of nodes,
> * no fallback.
> * FIXME: memory is allocated starting with the first node
> @@ -441,6 +448,10 @@ static const struct mempolicy_operations mpol_ops[MPOL_MAX] = {
> .create = mpol_new_nodemask,
> .rebind = mpol_rebind_preferred,
> },
> + [MPOL_WEIGHTED_INTERLEAVE] = {
> + .create = mpol_new_nodemask,
> + .rebind = mpol_rebind_nodemask,
> + },
> };
>
> static bool migrate_folio_add(struct folio *folio, struct list_head *foliolist,
> @@ -862,8 +873,11 @@ static long do_set_mempolicy(unsigned short mode, unsigned short flags,
>
> old = current->mempolicy;
> current->mempolicy = new;
> - if (new && new->mode == MPOL_INTERLEAVE)
> + if (new && (new->mode == MPOL_INTERLEAVE ||
> + new->mode == MPOL_WEIGHTED_INTERLEAVE)) {
> current->il_prev = MAX_NUMNODES-1;
> + current->il_weight = 0;
> + }
> task_unlock(current);
> mpol_put(old);
> ret = 0;
> @@ -888,6 +902,7 @@ static void get_policy_nodemask(struct mempolicy *pol, nodemask_t *nodes)
> case MPOL_INTERLEAVE:
> case MPOL_PREFERRED:
> case MPOL_PREFERRED_MANY:
> + case MPOL_WEIGHTED_INTERLEAVE:
> *nodes = pol->nodes;
> break;
> case MPOL_LOCAL:
> @@ -972,6 +987,13 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
> } else if (pol == current->mempolicy &&
> pol->mode == MPOL_INTERLEAVE) {
> *policy = next_node_in(current->il_prev, pol->nodes);
> + } else if (pol == current->mempolicy &&
> + pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> + if (current->il_weight)
> + *policy = current->il_prev;
> + else
> + *policy = next_node_in(current->il_prev,
> + pol->nodes);
> } else {
> err = -EINVAL;
> goto out;
> @@ -1336,7 +1358,8 @@ static long do_mbind(unsigned long start, unsigned long len,
> * VMAs, the nodes will still be interleaved from the targeted
> * nodemask, but one by one may be selected differently.
> */
> - if (new->mode == MPOL_INTERLEAVE) {
> + if (new->mode == MPOL_INTERLEAVE ||
> + new->mode == MPOL_WEIGHTED_INTERLEAVE) {
> struct page *page;
> unsigned int order;
> unsigned long addr = -EFAULT;
> @@ -1784,7 +1807,8 @@ struct mempolicy *__get_vma_policy(struct vm_area_struct *vma,
> * @vma: virtual memory area whose policy is sought
> * @addr: address in @vma for shared policy lookup
> * @order: 0, or appropriate huge_page_order for interleaving
> - * @ilx: interleave index (output), for use only when MPOL_INTERLEAVE
> + * @ilx: interleave index (output), for use only when MPOL_INTERLEAVE or
> + * MPOL_WEIGHTED_INTERLEAVE
> *
> * Returns effective policy for a VMA at specified address.
> * Falls back to current->mempolicy or system default policy, as necessary.
> @@ -1801,7 +1825,8 @@ struct mempolicy *get_vma_policy(struct vm_area_struct *vma,
> pol = __get_vma_policy(vma, addr, ilx);
> if (!pol)
> pol = get_task_policy(current);
> - if (pol->mode == MPOL_INTERLEAVE) {
> + if (pol->mode == MPOL_INTERLEAVE ||
> + pol->mode == MPOL_WEIGHTED_INTERLEAVE) {
> *ilx += vma->vm_pgoff >> order;
> *ilx += (addr - vma->vm_start) >> (PAGE_SHIFT + order);
> }
> @@ -1851,6 +1876,22 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
> return zone >= dynamic_policy_zone;
> }
>
> +static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> +{
> + unsigned int node = current->il_prev;
> +
> + if (!current->il_weight || !node_isset(node, policy->nodes)) {
> + node = next_node_in(node, policy->nodes);
> + /* can only happen if nodemask is being rebound */
> + if (node == MAX_NUMNODES)
> + return node;
I feel a little unsafe to read policy->nodes at same time of writing in
rebound. Is it better to use a seqlock to guarantee its consistency?
It's unnecessary to be a part of this series though.
> + current->il_prev = node;
> + current->il_weight = get_il_weight(node);
> + }
> + current->il_weight--;
> + return node;
> +}
> +
> /* Do dynamic interleaving for a process */
> static unsigned int interleave_nodes(struct mempolicy *policy)
> {
> @@ -1885,6 +1926,9 @@ unsigned int mempolicy_slab_node(void)
> case MPOL_INTERLEAVE:
> return interleave_nodes(policy);
>
> + case MPOL_WEIGHTED_INTERLEAVE:
> + return weighted_interleave_nodes(policy);
> +
> case MPOL_BIND:
> case MPOL_PREFERRED_MANY:
> {
> @@ -1923,6 +1967,45 @@ static unsigned int read_once_policy_nodemask(struct mempolicy *pol,
> return nodes_weight(*mask);
> }
>
> +static unsigned int weighted_interleave_nid(struct mempolicy *pol, pgoff_t ilx)
> +{
> + nodemask_t nodemask;
> + unsigned int target, nr_nodes;
> + u8 __rcu *table;
> + unsigned int weight_total = 0;
> + u8 weight;
> + int nid;
> +
> + nr_nodes = read_once_policy_nodemask(pol, &nodemask);
> + if (!nr_nodes)
> + return numa_node_id();
> +
> + rcu_read_lock();
> + table = rcu_dereference(iw_table);
> + /* calculate the total weight */
> + for_each_node_mask(nid, nodemask) {
> + /* detect system default usage */
> + weight = table ? table[nid] : 1;
> + weight = weight ? weight : 1;
> + weight_total += weight;
> + }
> +
> + /* Calculate the node offset based on totals */
> + target = ilx % weight_total;
> + nid = first_node(nodemask);
> + while (target) {
> + /* detect system default usage */
> + weight = table ? table[nid] : 1;
> + weight = weight ? weight : 1;
I found duplicated pattern as above in this patch. Can we define a
function like below to remove the duplication?
u8 __get_il_weight(u8 *table, int nid)
{
u8 weight;
weight = table ? table[nid] : 1;
return weight ? : 1;
}
This can be used in alloc_pages_bulk_array_weighted_interleave() to copy
from global to local weights array too.
But this isn't a big deal. I will leave it to you to decide.
> + if (target < weight)
> + break;
> + target -= weight;
> + nid = next_node_in(nid, nodemask);
> + }
> + rcu_read_unlock();
> + return nid;
> +}
> +
> /*
> * Do static interleaving for interleave index @ilx. Returns the ilx'th
> * node in pol->nodes (starting from ilx=0), wrapping around if ilx
> @@ -1983,6 +2066,11 @@ static nodemask_t *policy_nodemask(gfp_t gfp, struct mempolicy *pol,
> *nid = (ilx == NO_INTERLEAVE_INDEX) ?
> interleave_nodes(pol) : interleave_nid(pol, ilx);
> break;
> + case MPOL_WEIGHTED_INTERLEAVE:
> + *nid = (ilx == NO_INTERLEAVE_INDEX) ?
> + weighted_interleave_nodes(pol) :
> + weighted_interleave_nid(pol, ilx);
> + break;
> }
>
> return nodemask;
> @@ -2044,6 +2132,7 @@ bool init_nodemask_of_mempolicy(nodemask_t *mask)
> case MPOL_PREFERRED_MANY:
> case MPOL_BIND:
> case MPOL_INTERLEAVE:
> + case MPOL_WEIGHTED_INTERLEAVE:
> *mask = mempolicy->nodes;
> break;
>
> @@ -2144,6 +2233,7 @@ struct page *alloc_pages_mpol(gfp_t gfp, unsigned int order,
> * node in its nodemask, we allocate the standard way.
> */
> if (pol->mode != MPOL_INTERLEAVE &&
> + pol->mode != MPOL_WEIGHTED_INTERLEAVE &&
> (!nodemask || node_isset(nid, *nodemask))) {
> /*
> * First, try to allocate THP only on local node, but
> @@ -2279,6 +2369,127 @@ static unsigned long alloc_pages_bulk_array_interleave(gfp_t gfp,
> return total_allocated;
> }
>
> +static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
> + struct mempolicy *pol, unsigned long nr_pages,
> + struct page **page_array)
> +{
> + struct task_struct *me = current;
> + unsigned long total_allocated = 0;
> + unsigned long nr_allocated = 0;
> + unsigned long rounds;
> + unsigned long node_pages, delta;
> + u8 __rcu *table, *weights, weight;
> + unsigned int weight_total = 0;
> + unsigned long rem_pages = nr_pages;
> + nodemask_t nodes;
> + int nnodes, node, next_node;
> + int resume_node = MAX_NUMNODES - 1;
> + u8 resume_weight = 0;
> + int prev_node;
> + int i;
> +
> + if (!nr_pages)
> + return 0;
> +
> + nnodes = read_once_policy_nodemask(pol, &nodes);
> + if (!nnodes)
> + return 0;
> +
> + /* Continue allocating from most recent node and adjust the nr_pages */
> + node = me->il_prev;
> + weight = me->il_weight;
> + if (weight && node_isset(node, nodes)) {
> + node_pages = min(rem_pages, weight);
> + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> + NULL, page_array);
> + page_array += nr_allocated;
> + total_allocated += nr_allocated;
> + /* if that's all the pages, no need to interleave */
> + if (rem_pages < weight) {
> + /* stay on current node, adjust il_weight */
> + me->il_weight -= rem_pages;
> + return total_allocated;
> + } else if (rem_pages == weight) {
> + /* move to next node / weight */
> + me->il_prev = next_node_in(node, nodes);
> + me->il_weight = get_il_weight(next_node);
> + return total_allocated;
> + }
> + /* Otherwise we adjust remaining pages, continue from there */
> + rem_pages -= weight;
> + }
> + /* clear active weight in case of an allocation failure */
> + me->il_weight = 0;
> + prev_node = node;
> +
> + /* create a local copy of node weights to operate on outside rcu */
> + weights = kzalloc(nr_node_ids, GFP_KERNEL);
> + if (!weights)
> + return total_allocated;
> +
> + rcu_read_lock();
> + table = rcu_dereference(iw_table);
> + if (table)
> + memcpy(weights, table, nr_node_ids);
> + rcu_read_unlock();
> +
> + /* calculate total, detect system default usage */
> + for_each_node_mask(node, nodes) {
> + if (!weights[node])
> + weights[node] = 1;
> + weight_total += weights[node];
> + }
> +
> + /*
> + * Calculate rounds/partial rounds to minimize __alloc_pages_bulk calls.
> + * Track which node weighted interleave should resume from.
> + *
> + * if (rounds > 0) and (delta == 0), resume_node will always be
> + * the node following prev_node and its weight.
> + */
> + rounds = rem_pages / weight_total;
> + delta = rem_pages % weight_total;
> + resume_node = next_node_in(prev_node, nodes);
> + resume_weight = weights[resume_node];
> + for (i = 0; i < nnodes; i++) {
> + node = next_node_in(prev_node, nodes);
> + weight = weights[node];
> + node_pages = weight * rounds;
> + /* If a delta exists, add this node's portion of the delta */
> + if (delta > weight) {
> + node_pages += weight;
> + delta -= weight;
> + } else if (delta) {
> + node_pages += delta;
> + /* delta may deplete on a boundary or w/ a remainder */
> + if (delta == weight) {
> + /* boundary: resume from next node/weight */
> + resume_node = next_node_in(node, nodes);
> + resume_weight = weights[resume_node];
> + } else {
> + /* remainder: resume this node w/ remainder */
> + resume_node = node;
> + resume_weight = weight - delta;
> + }
If we are comfortable to leave resume_weight == 0, then the above
branch can be simplified to.
resume_node = node;
resume_weight = weight - delta;
But, this is a style issue again. I will leave it to you to decide.
So, except the issue you pointed out already. All series looks good to
me! Thanks! Feel free to add
Reviewed-by: "Huang, Ying" <ying.huang@intel.com>
to the whole series.
> + delta = 0;
> + }
> + /* node_pages can be 0 if an allocation fails and rounds == 0 */
> + if (!node_pages)
> + break;
> + nr_allocated = __alloc_pages_bulk(gfp, node, NULL, node_pages,
> + NULL, page_array);
> + page_array += nr_allocated;
> + total_allocated += nr_allocated;
> + if (total_allocated == nr_pages)
> + break;
> + prev_node = node;
> + }
> + me->il_prev = resume_node;
> + me->il_weight = resume_weight;
> + kfree(weights);
> + return total_allocated;
> +}
> +
> static unsigned long alloc_pages_bulk_array_preferred_many(gfp_t gfp, int nid,
> struct mempolicy *pol, unsigned long nr_pages,
> struct page **page_array)
> @@ -2319,6 +2530,10 @@ unsigned long alloc_pages_bulk_array_mempolicy(gfp_t gfp,
> return alloc_pages_bulk_array_interleave(gfp, pol,
> nr_pages, page_array);
>
> + if (pol->mode == MPOL_WEIGHTED_INTERLEAVE)
> + return alloc_pages_bulk_array_weighted_interleave(
> + gfp, pol, nr_pages, page_array);
> +
> if (pol->mode == MPOL_PREFERRED_MANY)
> return alloc_pages_bulk_array_preferred_many(gfp,
> numa_node_id(), pol, nr_pages, page_array);
> @@ -2394,6 +2609,7 @@ bool __mpol_equal(struct mempolicy *a, struct mempolicy *b)
> case MPOL_INTERLEAVE:
> case MPOL_PREFERRED:
> case MPOL_PREFERRED_MANY:
> + case MPOL_WEIGHTED_INTERLEAVE:
> return !!nodes_equal(a->nodes, b->nodes);
> case MPOL_LOCAL:
> return true;
> @@ -2530,6 +2746,10 @@ int mpol_misplaced(struct folio *folio, struct vm_area_struct *vma,
> polnid = interleave_nid(pol, ilx);
> break;
>
> + case MPOL_WEIGHTED_INTERLEAVE:
> + polnid = weighted_interleave_nid(pol, ilx);
> + break;
> +
> case MPOL_PREFERRED:
> if (node_isset(curnid, pol->nodes))
> goto out;
> @@ -2904,6 +3124,7 @@ static const char * const policy_modes[] =
> [MPOL_PREFERRED] = "prefer",
> [MPOL_BIND] = "bind",
> [MPOL_INTERLEAVE] = "interleave",
> + [MPOL_WEIGHTED_INTERLEAVE] = "weighted interleave",
> [MPOL_LOCAL] = "local",
> [MPOL_PREFERRED_MANY] = "prefer (many)",
> };
> @@ -2963,6 +3184,7 @@ int mpol_parse_str(char *str, struct mempolicy **mpol)
> }
> break;
> case MPOL_INTERLEAVE:
> + case MPOL_WEIGHTED_INTERLEAVE:
> /*
> * Default to online nodes with memory if no nodelist
> */
> @@ -3073,6 +3295,7 @@ void mpol_to_str(char *buffer, int maxlen, struct mempolicy *pol)
> case MPOL_PREFERRED_MANY:
> case MPOL_BIND:
> case MPOL_INTERLEAVE:
> + case MPOL_WEIGHTED_INTERLEAVE:
> nodes = pol->nodes;
> break;
> default:
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-01-31 7:43 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <877cjqgfzz.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Wed, Jan 31, 2024 at 02:43:12PM +0800, Huang, Ying wrote:
> Gregory Price <gourry.memverge@gmail.com> writes:
> >
> > +static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> > +{
> > + unsigned int node = current->il_prev;
> > +
> > + if (!current->il_weight || !node_isset(node, policy->nodes)) {
> > + node = next_node_in(node, policy->nodes);
> > + /* can only happen if nodemask is being rebound */
> > + if (node == MAX_NUMNODES)
> > + return node;
>
> I feel a little unsafe to read policy->nodes at same time of writing in
> rebound. Is it better to use a seqlock to guarantee its consistency?
> It's unnecessary to be a part of this series though.
>
I think this is handled already? It is definitely an explicit race
condition that is documented elsewhere:
/*
* mpol_rebind_policy - Migrate a policy to a different set of nodes
*
* Per-vma policies are protected by mmap_lock. Allocations using per-task
* policies are protected by task->mems_allowed_seq to prevent a premature
* OOM/allocation failure due to parallel nodemask modification.
*/
example from slub:
do {
cpuset_mems_cookie = read_mems_allowed_begin();
zonelist = node_zonelist(mempolicy_slab_node(), pc->flags);
...
} while (read_mems_allowed_retry(cpuset_mems_cookie));
quick perusal through other allocators, show similar checks.
page_alloc.c - check_retry_cpusetset()
filemap.c - filemap_alloc_folio()
If we ever want mempolicy to be swappable from outside the current task
context, this will have to change most likely - but that's another
feature for another day.
> > + while (target) {
> > + /* detect system default usage */
> > + weight = table ? table[nid] : 1;
> > + weight = weight ? weight : 1;
>
> I found duplicated pattern as above in this patch. Can we define a
> function like below to remove the duplication?
>
> u8 __get_il_weight(u8 *table, int nid)
> {
> u8 weight;
>
> weight = table ? table[nid] : 1;
> return weight ? : 1;
> }
>
When we implement the system-default array, this will change to:
weight = sysfs_table ? sysfs_table[nid] : default_table[nid];
This cleanup will get picked up in that patch set since this code is
going to change anyway.
> > + if (delta == weight) {
> > + /* boundary: resume from next node/weight */
> > + resume_node = next_node_in(node, nodes);
> > + resume_weight = weights[resume_node];
> > + } else {
> > + /* remainder: resume this node w/ remainder */
> > + resume_node = node;
> > + resume_weight = weight - delta;
> > + }
>
> If we are comfortable to leave resume_weight == 0, then the above
> branch can be simplified to.
>
> resume_node = node;
> resume_weight = weight - delta;
>
> But, this is a style issue again. I will leave it to you to decide.
Good point, and in fact there's a similar branch in the first half of
the function that can be simplified. Will follow up with a style patch.
mm/mempolicy.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
My favorite style of patch :D
Andrew if you happen to be monitoring, this is the patch (not tested
yet, but it's pretty obvious, otherwise i'll submit individually
tomorrow).
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 2c1aef8eab70..b0ca9bcdd64c 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2405,15 +2405,9 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
page_array += nr_allocated;
total_allocated += nr_allocated;
/* if that's all the pages, no need to interleave */
- if (rem_pages < weight) {
- /* stay on current node, adjust il_weight */
+ if (rem_pages <= weight) {
me->il_weight -= rem_pages;
return total_allocated;
- } else if (rem_pages == weight) {
- /* move to next node / weight */
- me->il_prev = next_node_in(node, nodes);
- me->il_weight = get_il_weight(me->il_prev);
- return total_allocated;
}
/* Otherwise we adjust remaining pages, continue from there */
rem_pages -= weight;
@@ -2460,17 +2454,10 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
node_pages += weight;
delta -= weight;
} else if (delta) {
+ /* when delta is deleted, resume from that node */
node_pages += delta;
- /* delta may deplete on a boundary or w/ a remainder */
- if (delta == weight) {
- /* boundary: resume from next node/weight */
- resume_node = next_node_in(node, nodes);
- resume_weight = weights[resume_node];
- } else {
- /* remainder: resume this node w/ remainder */
- resume_node = node;
- resume_weight = weight - delta;
- }
+ resume_node = node;
+ resume_weight = weight - delta;
delta = 0;
}
/* node_pages can be 0 if an allocation fails and rounds == 0 */
>
> So, except the issue you pointed out already. All series looks good to
> me! Thanks! Feel free to add
>
> Reviewed-by: "Huang, Ying" <ying.huang@intel.com>
>
> to the whole series.
>
Thank you so much for your patience with me! I appreciate all the help.
I am looking forward to this feature very much!
~Gregory
^ permalink raw reply related
* Re: [RFC PATCH v2 19/29] selftests: ntsync: Add some tests for NTSYNC_IOC_WAIT_ANY.
From: Andi Kleen @ 2024-01-31 8:52 UTC (permalink / raw)
To: Elizabeth Figura
Cc: Arnd Bergmann, Greg Kroah-Hartman, Jonathan Corbet, Shuah Khan,
linux-kernel, linux-api, wine-devel, André Almeida,
Wolfram Sang, Arkadiusz Hiler, Peter Zijlstra, Andy Lutomirski,
linux-doc, linux-kselftest
In-Reply-To: <20240131021356.10322-20-zfigura@codeweavers.com>
Elizabeth Figura <zfigura@codeweavers.com> writes:
> +TEST(test_wait_any)
> +{
> + struct ntsync_mutex_args mutex_args = {0};
> + struct ntsync_wait_args wait_args = {0};
> + struct ntsync_sem_args sem_args = {0};
> + __u32 owner, index, count;
> + struct timespec timeout;
> + int objs[2], fd, ret;
> +
> + clock_gettime(CLOCK_MONOTONIC, &timeout);
> +
> + fd = open("/dev/ntsync", O_CLOEXEC | O_RDONLY);
> + ASSERT_LE(0, fd);
> +
> + sem_args.count = 2;
> + sem_args.max = 3;
> + sem_args.sem = 0xdeadbeef;
> + ret = ioctl(fd, NTSYNC_IOC_CREATE_SEM, &sem_args);
> + EXPECT_EQ(0, ret);
> + EXPECT_NE(0xdeadbeef, sem_args.sem);
> +
> + mutex_args.owner = 0;
> + mutex_args.count = 0;
> + mutex_args.mutex = 0xdeadbeef;
> + ret = ioctl(fd, NTSYNC_IOC_CREATE_MUTEX, &mutex_args);
> + EXPECT_EQ(0, ret);
> + EXPECT_NE(0xdeadbeef, mutex_args.mutex);
It seems your tests are missing test cases for exceeding any limits,
especially overflow/underflow cases. Since these are the most likely
for any security problems it would be good to have extra coverage here.
The fuzzers will hopefully hit it too.
Also some stress testing with multiple threads would be useful.
-Andi
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Huang, Ying @ 2024-01-31 9:19 UTC (permalink / raw)
To: Gregory Price
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <Zbn6FG3346jhrQga@memverge.com>
Gregory Price <gregory.price@memverge.com> writes:
> On Wed, Jan 31, 2024 at 02:43:12PM +0800, Huang, Ying wrote:
>> Gregory Price <gourry.memverge@gmail.com> writes:
>> >
>> > +static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
>> > +{
>> > + unsigned int node = current->il_prev;
>> > +
>> > + if (!current->il_weight || !node_isset(node, policy->nodes)) {
>> > + node = next_node_in(node, policy->nodes);
>> > + /* can only happen if nodemask is being rebound */
>> > + if (node == MAX_NUMNODES)
>> > + return node;
>>
>> I feel a little unsafe to read policy->nodes at same time of writing in
>> rebound. Is it better to use a seqlock to guarantee its consistency?
>> It's unnecessary to be a part of this series though.
>>
>
> I think this is handled already? It is definitely an explicit race
> condition that is documented elsewhere:
>
> /*
> * mpol_rebind_policy - Migrate a policy to a different set of nodes
> *
> * Per-vma policies are protected by mmap_lock. Allocations using per-task
> * policies are protected by task->mems_allowed_seq to prevent a premature
> * OOM/allocation failure due to parallel nodemask modification.
> */
Thanks for pointing this out!
If we use task->mems_allowed_seq reader side in
weighted_interleave_nodes() we can guarantee the consistency of
policy->nodes. That may be not deserved, because it's not a big deal to
allocate 1 page in a wrong node.
It makes more sense to do that in
alloc_pages_bulk_array_weighted_interleave(), because a lot of pages may
be allocated there.
> example from slub:
>
> do {
> cpuset_mems_cookie = read_mems_allowed_begin();
> zonelist = node_zonelist(mempolicy_slab_node(), pc->flags);
> ...
> } while (read_mems_allowed_retry(cpuset_mems_cookie));
>
> quick perusal through other allocators, show similar checks.
>
> page_alloc.c - check_retry_cpusetset()
> filemap.c - filemap_alloc_folio()
>
> If we ever want mempolicy to be swappable from outside the current task
> context, this will have to change most likely - but that's another
> feature for another day.
>
--
Best Regards,
Huang, Ying
^ permalink raw reply
* Re: [PATCH net-next v4 3/3] eventpoll: Add epoll ioctl for epoll_params
From: Greg Kroah-Hartman @ 2024-01-31 16:16 UTC (permalink / raw)
To: Jiri Slaby
Cc: Joe Damato, linux-kernel, netdev, chuck.lever, jlayton, linux-api,
brauner, edumazet, davem, alexander.duyck, sridhar.samudrala,
kuba, willemdebruijn.kernel, weiwan, David.Laight, arnd,
Jonathan Corbet, Alexander Viro, Jan Kara, Michael Ellerman,
Nathan Lynch, Thomas Zimmermann, Maik Broemme, Steve French,
Julien Panis, Thomas Huth, Andrew Waterman, Albert Ou,
Palmer Dabbelt, open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <efee9789-4f05-4202-9a95-21d88f6307b0@kernel.org>
On Wed, Jan 31, 2024 at 07:03:54AM +0100, Jiri Slaby wrote:
> On 31. 01. 24, 3:27, Joe Damato wrote:
> > On Tue, Jan 30, 2024 at 06:08:36PM -0800, Greg Kroah-Hartman wrote:
> > > On Wed, Jan 31, 2024 at 01:47:33AM +0000, Joe Damato wrote:
> > > > +struct epoll_params {
> > > > + __aligned_u64 busy_poll_usecs;
> > > > + __u16 busy_poll_budget;
> > > > +
> > > > + /* pad the struct to a multiple of 64bits for alignment on all arches */
> > > > + __u8 __pad[6];
> > >
> > > You HAVE to check this padding to be sure it is all 0, otherwise it can
> > > never be used in the future for anything.
> >
> > Is there some preferred mechanism for this in the kernel that I should be
> > using or is this as simple as adding a for loop to check each u8 == 0 ?
>
> You are likely looking for memchr_inv().
Ah, never noticed that, thanks!
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-01-31 16:35 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <87y1c5g8qw.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Wed, Jan 31, 2024 at 05:19:51PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
>
> > On Wed, Jan 31, 2024 at 02:43:12PM +0800, Huang, Ying wrote:
> >> Gregory Price <gourry.memverge@gmail.com> writes:
> >> >
> >> > +static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> >> > +{
> >> > + unsigned int node = current->il_prev;
> >> > +
> >> > + if (!current->il_weight || !node_isset(node, policy->nodes)) {
> >> > + node = next_node_in(node, policy->nodes);
> >> > + /* can only happen if nodemask is being rebound */
> >> > + if (node == MAX_NUMNODES)
> >> > + return node;
> >>
> >> I feel a little unsafe to read policy->nodes at same time of writing in
> >> rebound. Is it better to use a seqlock to guarantee its consistency?
> >> It's unnecessary to be a part of this series though.
> >>
> >
> > I think this is handled already? It is definitely an explicit race
> > condition that is documented elsewhere:
> >
> > /*
> > * mpol_rebind_policy - Migrate a policy to a different set of nodes
> > *
> > * Per-vma policies are protected by mmap_lock. Allocations using per-task
> > * policies are protected by task->mems_allowed_seq to prevent a premature
> > * OOM/allocation failure due to parallel nodemask modification.
> > */
>
> Thanks for pointing this out!
>
> If we use task->mems_allowed_seq reader side in
> weighted_interleave_nodes() we can guarantee the consistency of
> policy->nodes. That may be not deserved, because it's not a big deal to
> allocate 1 page in a wrong node.
>
> It makes more sense to do that in
> alloc_pages_bulk_array_weighted_interleave(), because a lot of pages may
> be allocated there.
>
That's probably worth just adding now, I'll do it and squash the style
updates into the branch. Sorry Andrew, I guess 1 last version is
inbound :]
I'll pick up the reviewed tags along the way.
~Gregory
^ permalink raw reply
* Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving
From: Gregory Price @ 2024-01-31 17:29 UTC (permalink / raw)
To: Huang, Ying
Cc: Gregory Price, linux-mm, linux-kernel, linux-doc, linux-fsdevel,
linux-api, corbet, akpm, honggyu.kim, rakie.kim, hyeongtak.ji,
mhocko, vtavarespetr, jgroves, ravis.opensrc, sthanneeru,
emirakhur, Hasan.Maruf, seungjun.ha, hannes, dan.j.williams,
Srinivasulu Thanneeru
In-Reply-To: <87y1c5g8qw.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Wed, Jan 31, 2024 at 05:19:51PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
>
> >
> > I think this is handled already? It is definitely an explicit race
> > condition that is documented elsewhere:
> >
> > /*
> > * mpol_rebind_policy - Migrate a policy to a different set of nodes
> > *
> > * Per-vma policies are protected by mmap_lock. Allocations using per-task
> > * policies are protected by task->mems_allowed_seq to prevent a premature
> > * OOM/allocation failure due to parallel nodemask modification.
> > */
>
> Thanks for pointing this out!
>
> If we use task->mems_allowed_seq reader side in
> weighted_interleave_nodes() we can guarantee the consistency of
> policy->nodes. That may be not deserved, because it's not a big deal to
> allocate 1 page in a wrong node.
>
> It makes more sense to do that in
> alloc_pages_bulk_array_weighted_interleave(), because a lot of pages may
> be allocated there.
>
To save the versioning if there are issues, here are the 3 diffs that
I have left. If you are good with these changes, I'll squash the first
2 into the third commit, keep the last one as a separate commit (it
changes the interleave_nodes() logic too), and submit v5 w/ your
reviewed tag on all of them.
Fix one (pedantic?) warning from syzbot:
----------------------------------------
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index b1437396c357..dfd097009606 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2391,7 +2391,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
unsigned long nr_allocated = 0;
unsigned long rounds;
unsigned long node_pages, delta;
- u8 __rcu *table, *weights, weight;
+ u8 __rcu *table, __rcu *weights, weight;
unsigned int weight_total = 0;
unsigned long rem_pages = nr_pages;
nodemask_t nodes;
Simplifying resume_node/weight logic:
-------------------------------------
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 2c1aef8eab70..b0ca9bcdd64c 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2405,15 +2405,9 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
page_array += nr_allocated;
total_allocated += nr_allocated;
/* if that's all the pages, no need to interleave */
- if (rem_pages < weight) {
- /* stay on current node, adjust il_weight */
+ if (rem_pages <= weight) {
me->il_weight -= rem_pages;
return total_allocated;
- } else if (rem_pages == weight) {
- /* move to next node / weight */
- me->il_prev = next_node_in(node, nodes);
- me->il_weight = get_il_weight(me->il_prev);
- return total_allocated;
}
/* Otherwise we adjust remaining pages, continue from there */
rem_pages -= weight;
@@ -2460,17 +2454,10 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
node_pages += weight;
delta -= weight;
} else if (delta) {
+ /* when delta is deleted, resume from that node */
node_pages += delta;
- /* delta may deplete on a boundary or w/ a remainder */
- if (delta == weight) {
- /* boundary: resume from next node/weight */
- resume_node = next_node_in(node, nodes);
- resume_weight = weights[resume_node];
- } else {
- /* remainder: resume this node w/ remainder */
- resume_node = node;
- resume_weight = weight - delta;
- }
+ resume_node = node;
+ resume_weight = weight - delta;
delta = 0;
}
/* node_pages can be 0 if an allocation fails and rounds == 0 */
task->mems_allowed_seq protection (added as 4th patch)
------------------------------------------------------
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index b0ca9bcdd64c..b1437396c357 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1879,10 +1879,15 @@ bool apply_policy_zone(struct mempolicy *policy, enum zone_type zone)
static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
{
unsigned int node = current->il_prev;
+ unsigned int cpuset_mems_cookie;
+retry:
+ /* to prevent miscount use tsk->mems_allowed_seq to detect rebind */
+ cpuset_mems_cookie = read_mems_allowed_begin();
if (!current->il_weight || !node_isset(node, policy->nodes)) {
node = next_node_in(node, policy->nodes);
- /* can only happen if nodemask is being rebound */
+ if (read_mems_allowed_retry(cpuset_mems_cookie))
+ goto retry;
if (node == MAX_NUMNODES)
return node;
current->il_prev = node;
@@ -1896,10 +1901,17 @@ static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
static unsigned int interleave_nodes(struct mempolicy *policy)
{
unsigned int nid;
+ unsigned int cpuset_mems_cookie;
+
+ /* to prevent miscount, use tsk->mems_allowed_seq to detect rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nid = next_node_in(current->il_prev, policy->nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
- nid = next_node_in(current->il_prev, policy->nodes);
if (nid < MAX_NUMNODES)
current->il_prev = nid;
+
return nid;
}
@@ -2374,6 +2386,7 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
struct page **page_array)
{
struct task_struct *me = current;
+ unsigned int cpuset_mems_cookie;
unsigned long total_allocated = 0;
unsigned long nr_allocated = 0;
unsigned long rounds;
@@ -2388,10 +2401,17 @@ static unsigned long alloc_pages_bulk_array_weighted_interleave(gfp_t gfp,
int prev_node;
int i;
+
if (!nr_pages)
return 0;
- nnodes = read_once_policy_nodemask(pol, &nodes);
+ /* read the nodes onto the stack, retry if done during rebind */
+ do {
+ cpuset_mems_cookie = read_mems_allowed_begin();
+ nnodes = read_once_policy_nodemask(pol, &nodes);
+ } while (read_mems_allowed_retry(cpuset_mems_cookie));
+
+ /* if the nodemask has become invalid, we cannot do anything */
if (!nnodes)
return 0;
^ permalink raw reply related
* [PATCH net-next v5 0/3] Per epoll context busy poll support
From: Joe Damato @ 2024-01-31 18:08 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: chuck.lever, jlayton, linux-api, brauner, edumazet, davem,
alexander.duyck, sridhar.samudrala, kuba, willemdebruijn.kernel,
weiwan, David.Laight, arnd, Joe Damato, Albert Ou, Alexander Viro,
Andrew Waterman, Dominik Brodowski, Greg Kroah-Hartman, Jan Kara,
Jiri Slaby, Jonathan Corbet, Julien Panis,
open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure), Michael Ellerman,
Nathan Lynch, Palmer Dabbelt, Steve French, Thomas Zimmermann
Greetings:
Welcome to v5.
TL;DR This builds on commit bf3b9f6372c4 ("epoll: Add busy poll support to
epoll with socket fds.") by allowing user applications to enable
epoll-based busy polling and set a busy poll packet budget on a per epoll
context basis.
This makes epoll-based busy polling much more usable for user
applications than the current system-wide sysctl and hardcoded budget.
To allow for this, two ioctls have been added for epoll contexts for
getting and setting a new struct, struct epoll_params.
ioctl was chosen vs a new syscall after reviewing a suggestion by Willem
de Bruijn [1]. I am open to using a new syscall instead of an ioctl, but it
seemed that:
- Busy poll affects all existing epoll_wait and epoll_pwait variants in
the same way, so new verions of many syscalls might be needed. It
seems much simpler for users to use the correct
epoll_wait/epoll_pwait for their app and add a call to ioctl to enable
or disable busy poll as needed. This also probably means less work to
get an existing epoll app using busy poll.
- previously added epoll_pwait2 helped to bring epoll closer to
existing syscalls (like pselect and ppoll) and this busy poll change
reflected as a new syscall would not have the same effect.
Note: patch 1/4 as of v4 uses an or (||) instead of an xor. I thought about
it some more and I realized that if the user enables both the per-epoll
context setting and the system wide sysctl, then busy poll should be
enabled and not disabled. Using xor doesn't seem to make much sense after
thinking through this a bit.
Longer explanation:
Presently epoll has support for a very useful form of busy poll based on
the incoming NAPI ID (see also: SO_INCOMING_NAPI_ID [2]).
This form of busy poll allows epoll_wait to drive NAPI packet processing
which allows for a few interesting user application designs which can
reduce latency and also potentially improve L2/L3 cache hit rates by
deferring NAPI until userland has finished its work.
The documentation available on this is, IMHO, a bit confusing so please
allow me to explain how one might use this:
1. Ensure each application thread has its own epoll instance mapping
1-to-1 with NIC RX queues. An n-tuple filter would likely be used to
direct connections with specific dest ports to these queues.
2. Optionally: Setup IRQ coalescing for the NIC RX queues where busy
polling will occur. This can help avoid the userland app from being
pre-empted by a hard IRQ while userland is running. Note this means that
userland must take care to call epoll_wait and not take too long in
userland since it now drives NAPI via epoll_wait.
3. Optionally: Consider using napi_defer_hard_irqs and gro_flush_timeout to
further restrict IRQ generation from the NIC. These settings are
system-wide so their impact must be carefully weighed against the running
applications.
4. Ensure that all incoming connections added to an epoll instance
have the same NAPI ID. This can be done with a BPF filter when
SO_REUSEPORT is used or getsockopt + SO_INCOMING_NAPI_ID when a single
accept thread is used which dispatches incoming connections to threads.
5. Lastly, busy poll must be enabled via a sysctl
(/proc/sys/net/core/busy_poll).
Please see Eric Dumazet's paper about busy polling [3] and a recent
academic paper about measured performance improvements of busy polling [4]
(albeit with a modification that is not currently present in the kernel)
for additional context.
The unfortunate part about step 5 above is that this enables busy poll
system-wide which affects all user applications on the system,
including epoll-based network applications which were not intended to
be used this way or applications where increased CPU usage for lower
latency network processing is unnecessary or not desirable.
If the user wants to run one low latency epoll-based server application
with epoll-based busy poll, but would like to run the rest of the
applications on the system (which may also use epoll) without busy poll,
this system-wide sysctl presents a significant problem.
This change preserves the system-wide sysctl, but adds a mechanism (via
ioctl) to enable or disable busy poll for epoll contexts as needed by
individual applications, making epoll-based busy poll more usable.
Note that this change includes an or (as of v4) instead of an xor. If the
user has enabled both the system-wide sysctl and also the per epoll-context
busy poll settings, then epoll should probably busy poll (vs being
disabled).
Thanks,
Joe
v4 -> v5:
- patch 3/3 updated to use memchr_inv to ensure that __pad is zero for
the EPIOCSPARAMS ioctl. Recommended by Greg K-H [5], Dave Chinner [6],
and Jiri Slaby [7].
v3 -> v4:
- patch 1/3 was updated to include an important functional change:
ep_busy_loop_on was updated to use or (||) instead of xor (^). After
thinking about it a bit more, I thought xor didn't make much sense.
Enabling both the per-epoll context and the system-wide sysctl should
probably enable busy poll, not disable it. So, or (||) makes more
sense, I think.
- patch 3/3 was updated:
- to change the epoll_params fields to be __u64, __u16, and __u8 and
to pad the struct to a multiple of 64bits. Suggested by Greg K-H [8]
and Arnd Bergmann [9].
- remove an unused pr_fmt, left over from the previous revision.
- ioctl now returns -EINVAL when epoll_params.busy_poll_usecs >
U32_MAX.
v2 -> v3:
- cover letter updated to mention why ioctl seems (to me) like a better
choice vs a new syscall.
- patch 3/4 was modified in 3 ways:
- when an unknown ioctl is received, -ENOIOCTLCMD is returned instead
of -EINVAL as the ioctl documentation requires.
- epoll_params.busy_poll_budget can only be set to a value larger than
NAPI_POLL_WEIGHT if code is run by privileged (CAP_NET_ADMIN) users.
Otherwise, -EPERM is returned.
- busy poll specific ioctl code moved out to its own function. On
kernels without busy poll support, -EOPNOTSUPP is returned. This also
makes the kernel build robot happier without littering the code with
more #ifdefs.
- dropped patch 4/4 after Eric Dumazet's review of it when it was sent
independently to the list [10].
v1 -> v2:
- cover letter updated to make a mention of napi_defer_hard_irqs and
gro_flush_timeout as an added step 3 and to cite both Eric Dumazet's
busy polling paper and a paper from University of Waterloo for
additional context. Specifically calling out the xor in patch 1/4
incase it is missed by reviewers.
- Patch 2/4 has its commit message updated, but no functional changes.
Commit message now describes that allowing for a settable budget helps
to improve throughput and is more consistent with other busy poll
mechanisms that allow a settable budget via SO_BUSY_POLL_BUDGET.
- Patch 3/4 was modified to check if the epoll_params.busy_poll_budget
exceeds NAPI_POLL_WEIGHT. The larger value is allowed, but an error is
printed. This was done for consistency with netif_napi_add_weight,
which does the same.
- Patch 3/4 the struct epoll_params was updated to fix the type of the
data field; it was uint8_t and was changed to u8.
- Patch 4/4 added to check if SO_BUSY_POLL_BUDGET exceeds
NAPI_POLL_WEIGHT. The larger value is allowed, but an error is
printed. This was done for consistency with netif_napi_add_weight,
which does the same.
[1]: https://lore.kernel.org/lkml/65b1cb7f73a6a_250560294bd@willemb.c.googlers.com.notmuch/
[2]: https://lore.kernel.org/lkml/20170324170836.15226.87178.stgit@localhost.localdomain/
[3]: https://netdevconf.info/2.1/papers/BusyPollingNextGen.pdf
[4]: https://dl.acm.org/doi/pdf/10.1145/3626780
[5]: https://lore.kernel.org/lkml/2024013001-prison-strum-899d@gregkh/
[6]: https://lore.kernel.org/lkml/Zbm3AXgcwL9D6TNM@dread.disaster.area/
[7]: https://lore.kernel.org/lkml/efee9789-4f05-4202-9a95-21d88f6307b0@kernel.org/
[8]: https://lore.kernel.org/lkml/2024012551-anyone-demeaning-867b@gregkh/
[9]: https://lore.kernel.org/lkml/57b62135-2159-493d-a6bb-47d5be55154a@app.fastmail.com/
[10]: https://lore.kernel.org/lkml/CANn89i+uXsdSVFiQT9fDfGw+h_5QOcuHwPdWi9J=5U6oLXkQTA@mail.gmail.com/
Joe Damato (3):
eventpoll: support busy poll per epoll instance
eventpoll: Add per-epoll busy poll packet budget
eventpoll: Add epoll ioctl for epoll_params
.../userspace-api/ioctl/ioctl-number.rst | 1 +
fs/eventpoll.c | 126 +++++++++++++++++-
include/uapi/linux/eventpoll.h | 12 ++
3 files changed, 134 insertions(+), 5 deletions(-)
--
2.25.1
^ permalink raw reply
* [PATCH net-next v5 1/3] eventpoll: support busy poll per epoll instance
From: Joe Damato @ 2024-01-31 18:08 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: chuck.lever, jlayton, linux-api, brauner, edumazet, davem,
alexander.duyck, sridhar.samudrala, kuba, willemdebruijn.kernel,
weiwan, David.Laight, arnd, Joe Damato, Alexander Viro, Jan Kara,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20240131180811.23566-1-jdamato@fastly.com>
Allow busy polling on a per-epoll context basis. The per-epoll context
usec timeout value is preferred, but the pre-existing system wide sysctl
value is still supported if it specified.
Note that this change uses an xor: either per epoll instance busy polling
is enabled on the epoll instance or system wide epoll is enabled. Enabling
both is disallowed.
Signed-off-by: Joe Damato <jdamato@fastly.com>
---
fs/eventpoll.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 3534d36a1474..ce75189d46df 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -227,6 +227,8 @@ struct eventpoll {
#ifdef CONFIG_NET_RX_BUSY_POLL
/* used to track busy poll napi_id */
unsigned int napi_id;
+ /* busy poll timeout */
+ u64 busy_poll_usecs;
#endif
#ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -386,12 +388,44 @@ static inline int ep_events_available(struct eventpoll *ep)
READ_ONCE(ep->ovflist) != EP_UNACTIVE_PTR;
}
+/**
+ * busy_loop_ep_timeout - check if busy poll has timed out. The timeout value
+ * from the epoll instance ep is preferred, but if it is not set fallback to
+ * the system-wide global via busy_loop_timeout.
+ *
+ * @start_time: The start time used to compute the remaining time until timeout.
+ * @ep: Pointer to the eventpoll context.
+ *
+ * Return: true if the timeout has expired, false otherwise.
+ */
+static inline bool busy_loop_ep_timeout(unsigned long start_time, struct eventpoll *ep)
+{
+#ifdef CONFIG_NET_RX_BUSY_POLL
+ unsigned long bp_usec = READ_ONCE(ep->busy_poll_usecs);
+
+ if (bp_usec) {
+ unsigned long end_time = start_time + bp_usec;
+ unsigned long now = busy_loop_current_time();
+
+ return time_after(now, end_time);
+ } else {
+ return busy_loop_timeout(start_time);
+ }
+#endif
+ return true;
+}
+
#ifdef CONFIG_NET_RX_BUSY_POLL
+static bool ep_busy_loop_on(struct eventpoll *ep)
+{
+ return !!ep->busy_poll_usecs || net_busy_loop_on();
+}
+
static bool ep_busy_loop_end(void *p, unsigned long start_time)
{
struct eventpoll *ep = p;
- return ep_events_available(ep) || busy_loop_timeout(start_time);
+ return ep_events_available(ep) || busy_loop_ep_timeout(start_time, ep);
}
/*
@@ -404,7 +438,7 @@ static bool ep_busy_loop(struct eventpoll *ep, int nonblock)
{
unsigned int napi_id = READ_ONCE(ep->napi_id);
- if ((napi_id >= MIN_NAPI_ID) && net_busy_loop_on()) {
+ if ((napi_id >= MIN_NAPI_ID) && ep_busy_loop_on(ep)) {
napi_busy_loop(napi_id, nonblock ? NULL : ep_busy_loop_end, ep, false,
BUSY_POLL_BUDGET);
if (ep_events_available(ep))
@@ -430,7 +464,8 @@ static inline void ep_set_busy_poll_napi_id(struct epitem *epi)
struct socket *sock;
struct sock *sk;
- if (!net_busy_loop_on())
+ ep = epi->ep;
+ if (!ep_busy_loop_on(ep))
return;
sock = sock_from_file(epi->ffd.file);
@@ -442,7 +477,6 @@ static inline void ep_set_busy_poll_napi_id(struct epitem *epi)
return;
napi_id = READ_ONCE(sk->sk_napi_id);
- ep = epi->ep;
/* Non-NAPI IDs can be rejected
* or
@@ -466,6 +500,10 @@ static inline void ep_set_busy_poll_napi_id(struct epitem *epi)
{
}
+static inline bool ep_busy_loop_on(struct eventpoll *ep)
+{
+ return false;
+}
#endif /* CONFIG_NET_RX_BUSY_POLL */
/*
@@ -2058,6 +2096,9 @@ static int do_epoll_create(int flags)
error = PTR_ERR(file);
goto out_free_fd;
}
+#ifdef CONFIG_NET_RX_BUSY_POLL
+ ep->busy_poll_usecs = 0;
+#endif
ep->file = file;
fd_install(fd, file);
return fd;
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 2/3] eventpoll: Add per-epoll busy poll packet budget
From: Joe Damato @ 2024-01-31 18:08 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: chuck.lever, jlayton, linux-api, brauner, edumazet, davem,
alexander.duyck, sridhar.samudrala, kuba, willemdebruijn.kernel,
weiwan, David.Laight, arnd, Joe Damato, Alexander Viro, Jan Kara,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20240131180811.23566-1-jdamato@fastly.com>
When using epoll-based busy poll, the packet budget is hardcoded to
BUSY_POLL_BUDGET (8). Users may desire larger busy poll budgets, which
can potentially increase throughput when busy polling under high network
load.
Other busy poll methods allow setting the busy poll budget via
SO_BUSY_POLL_BUDGET, but epoll-based busy polling uses a hardcoded
value.
Fix this edge case by adding support for a per-epoll context busy poll
packet budget. If not specified, the default value (BUSY_POLL_BUDGET) is
used.
Signed-off-by: Joe Damato <jdamato@fastly.com>
---
fs/eventpoll.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index ce75189d46df..3985434df527 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -229,6 +229,8 @@ struct eventpoll {
unsigned int napi_id;
/* busy poll timeout */
u64 busy_poll_usecs;
+ /* busy poll packet budget */
+ u16 busy_poll_budget;
#endif
#ifdef CONFIG_DEBUG_LOCK_ALLOC
@@ -437,10 +439,14 @@ static bool ep_busy_loop_end(void *p, unsigned long start_time)
static bool ep_busy_loop(struct eventpoll *ep, int nonblock)
{
unsigned int napi_id = READ_ONCE(ep->napi_id);
+ u16 budget = READ_ONCE(ep->busy_poll_budget);
+
+ if (!budget)
+ budget = BUSY_POLL_BUDGET;
if ((napi_id >= MIN_NAPI_ID) && ep_busy_loop_on(ep)) {
napi_busy_loop(napi_id, nonblock ? NULL : ep_busy_loop_end, ep, false,
- BUSY_POLL_BUDGET);
+ budget);
if (ep_events_available(ep))
return true;
/*
@@ -2098,6 +2104,7 @@ static int do_epoll_create(int flags)
}
#ifdef CONFIG_NET_RX_BUSY_POLL
ep->busy_poll_usecs = 0;
+ ep->busy_poll_budget = 0;
#endif
ep->file = file;
fd_install(fd, file);
--
2.25.1
^ permalink raw reply related
* [PATCH net-next v5 3/3] eventpoll: Add epoll ioctl for epoll_params
From: Joe Damato @ 2024-01-31 18:08 UTC (permalink / raw)
To: linux-kernel, netdev
Cc: chuck.lever, jlayton, linux-api, brauner, edumazet, davem,
alexander.duyck, sridhar.samudrala, kuba, willemdebruijn.kernel,
weiwan, David.Laight, arnd, Joe Damato, Jonathan Corbet,
Alexander Viro, Jan Kara, Greg Kroah-Hartman, Nathan Lynch,
Michael Ellerman, Dominik Brodowski, Steve French, Julien Panis,
Thomas Zimmermann, Jiri Slaby, Thomas Huth, Albert Ou,
Andrew Waterman, Palmer Dabbelt, open list:DOCUMENTATION,
open list:FILESYSTEMS (VFS and infrastructure)
In-Reply-To: <20240131180811.23566-1-jdamato@fastly.com>
Add an ioctl for getting and setting epoll_params. User programs can use
this ioctl to get and set the busy poll usec time or packet budget
params for a specific epoll context.
Parameters are limited:
- busy_poll_usecs is limited to <= u32_max
- busy_poll_budget is limited to <= NAPI_POLL_WEIGHT by unprivileged
users (!capable(CAP_NET_ADMIN)).
- __pad must be 0
Signed-off-by: Joe Damato <jdamato@fastly.com>
---
.../userspace-api/ioctl/ioctl-number.rst | 1 +
fs/eventpoll.c | 68 +++++++++++++++++++
include/uapi/linux/eventpoll.h | 12 ++++
3 files changed, 81 insertions(+)
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 457e16f06e04..b33918232f78 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -309,6 +309,7 @@ Code Seq# Include File Comments
0x89 0B-DF linux/sockios.h
0x89 E0-EF linux/sockios.h SIOCPROTOPRIVATE range
0x89 F0-FF linux/sockios.h SIOCDEVPRIVATE range
+0x8A 00-1F linux/eventpoll.h
0x8B all linux/wireless.h
0x8C 00-3F WiNRADiO driver
<http://www.winradio.com.au/>
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 3985434df527..96efca6a9238 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -37,6 +37,7 @@
#include <linux/seq_file.h>
#include <linux/compat.h>
#include <linux/rculist.h>
+#include <linux/capability.h>
#include <net/busy_poll.h>
/*
@@ -495,6 +496,45 @@ static inline void ep_set_busy_poll_napi_id(struct epitem *epi)
ep->napi_id = napi_id;
}
+static long ep_eventpoll_bp_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct eventpoll *ep;
+ struct epoll_params epoll_params;
+ void __user *uarg = (void __user *) arg;
+
+ ep = file->private_data;
+
+ switch (cmd) {
+ case EPIOCSPARAMS:
+ if (copy_from_user(&epoll_params, uarg, sizeof(epoll_params)))
+ return -EFAULT;
+
+ if (memchr_inv(epoll_params.__pad, 0, sizeof(epoll_params.__pad)))
+ return -EINVAL;
+
+ if (epoll_params.busy_poll_usecs > U32_MAX)
+ return -EINVAL;
+
+ if (epoll_params.busy_poll_budget > NAPI_POLL_WEIGHT &&
+ !capable(CAP_NET_ADMIN))
+ return -EPERM;
+
+ ep->busy_poll_usecs = epoll_params.busy_poll_usecs;
+ ep->busy_poll_budget = epoll_params.busy_poll_budget;
+ return 0;
+ case EPIOCGPARAMS:
+ memset(&epoll_params, 0, sizeof(epoll_params));
+ epoll_params.busy_poll_usecs = ep->busy_poll_usecs;
+ epoll_params.busy_poll_budget = ep->busy_poll_budget;
+ if (copy_to_user(uarg, &epoll_params, sizeof(epoll_params)))
+ return -EFAULT;
+ return 0;
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
#else
static inline bool ep_busy_loop(struct eventpoll *ep, int nonblock)
@@ -510,6 +550,12 @@ static inline bool ep_busy_loop_on(struct eventpoll *ep)
{
return false;
}
+
+static long ep_eventpoll_bp_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ return -EOPNOTSUPP;
+}
#endif /* CONFIG_NET_RX_BUSY_POLL */
/*
@@ -869,6 +915,26 @@ static void ep_clear_and_put(struct eventpoll *ep)
ep_free(ep);
}
+static long ep_eventpoll_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ int ret;
+
+ if (!is_file_epoll(file))
+ return -EINVAL;
+
+ switch (cmd) {
+ case EPIOCSPARAMS:
+ case EPIOCGPARAMS:
+ ret = ep_eventpoll_bp_ioctl(file, cmd, arg);
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
static int ep_eventpoll_release(struct inode *inode, struct file *file)
{
struct eventpoll *ep = file->private_data;
@@ -975,6 +1041,8 @@ static const struct file_operations eventpoll_fops = {
.release = ep_eventpoll_release,
.poll = ep_eventpoll_poll,
.llseek = noop_llseek,
+ .unlocked_ioctl = ep_eventpoll_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
};
/*
diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h
index cfbcc4cc49ac..98e5ea525dd0 100644
--- a/include/uapi/linux/eventpoll.h
+++ b/include/uapi/linux/eventpoll.h
@@ -85,4 +85,16 @@ struct epoll_event {
__u64 data;
} EPOLL_PACKED;
+struct epoll_params {
+ __aligned_u64 busy_poll_usecs;
+ __u16 busy_poll_budget;
+
+ /* pad the struct to a multiple of 64bits for alignment on all arches */
+ __u8 __pad[6];
+};
+
+#define EPOLL_IOC_TYPE 0x8A
+#define EPIOCSPARAMS _IOW(EPOLL_IOC_TYPE, 0x01, struct epoll_params)
+#define EPIOCGPARAMS _IOR(EPOLL_IOC_TYPE, 0x02, struct epoll_params)
+
#endif /* _UAPI_LINUX_EVENTPOLL_H */
--
2.25.1
^ permalink raw reply related
* Re: [RFC PATCH] pidfd: implement PIDFD_THREAD flag for pidfd_open()
From: Andy Lutomirski @ 2024-01-31 18:11 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Tycho Andersen, Christian Brauner, linux-kernel, linux-api,
Tycho Andersen, Eric W. Biederman
In-Reply-To: <20240129112313.GA11635@redhat.com>
On Mon, Jan 29, 2024 at 3:24 AM Oleg Nesterov <oleg@redhat.com> wrote:
>
> On 01/27, Oleg Nesterov wrote:
> >
> > I'll (hopefully) send v2 on top of
> >
> > pidfd: cleanup the usage of __pidfd_prepare's flags
> > pidfd: don't do_notify_pidfd() if !thread_group_empty()
> >
> > on Monday
>
> Sorry, I don't have time to finish v2 today, I need to update the comments
> and write the changelog.
>
> But the patch itself is ready, I am sending it for review.
>
> Tycho, Christian, any comments?
Right now, pidfd_send_signal() sends signals to processes, like so:
* The syscall currently only signals via PIDTYPE_PID which covers
* kill(<positive-pid>, <signal>. It does not signal threads or process
* groups.
This patch adds PIDFD_THREAD which, potentially confusingly, doesn't
change this (AFAICS). So at least that should be documented loudly
and clearly, IMO. But I actually just bumped in to this limitation in
pidfd_send_signal(), like so:
https://github.com/systemd/systemd/issues/31093
Specifically, systemd can't properly emulate Ctrl-C using pidfd_send_signal().
I don't know whether implementing the other signal types belongs as
part of this patch, but they're at least thematically related.
--Andy
^ 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