* [PATCH v3 0/2] add epoll_pwait2 syscall
@ 2020-11-18 14:46 Willem de Bruijn
2020-11-18 14:46 ` [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 Willem de Bruijn
` (2 more replies)
0 siblings, 3 replies; 25+ messages in thread
From: Willem de Bruijn @ 2020-11-18 14:46 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-kernel, viro, akpm, soheil.kdev, arnd, shuochen, linux-man,
Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Enable nanosecond timeouts for epoll.
Analogous to pselect and ppoll, introduce an epoll_wait syscall
variant that takes a struct timespec instead of int timeout.
See patch 1 for more details.
patch 1: new syscall
patch 2: selftest
Willem de Bruijn (2):
epoll: add nsec timeout support with epoll_pwait2
selftests/filesystems: expand epoll with epoll_pwait2
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
fs/eventpoll.c | 106 +++++++++++++++---
include/linux/compat.h | 6 +
include/linux/syscalls.h | 5 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/sys_ni.c | 2 +
.../filesystems/epoll/epoll_wakeup_test.c | 70 ++++++++++++
24 files changed, 193 insertions(+), 20 deletions(-)
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply [flat|nested] 25+ messages in thread* [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 14:46 [PATCH v3 0/2] add epoll_pwait2 syscall Willem de Bruijn @ 2020-11-18 14:46 ` Willem de Bruijn 2020-11-18 15:00 ` Matthew Wilcox 2020-11-18 16:21 ` Willem de Bruijn 2020-11-18 14:46 ` [PATCH manpages RFC] epoll_wait.2: add epoll_pwait2 Willem de Bruijn 2020-11-18 14:46 ` [PATCH v3 2/2] selftests/filesystems: expand epoll with epoll_pwait2 Willem de Bruijn 2 siblings, 2 replies; 25+ messages in thread From: Willem de Bruijn @ 2020-11-18 14:46 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, viro, akpm, soheil.kdev, arnd, shuochen, linux-man, Willem de Bruijn From: Willem de Bruijn <willemb@google.com> Add syscall epoll_pwait2, an epoll_wait variant with nsec resolution that replaces int timeout with struct timespec. It is equivalent otherwise. int epoll_pwait2(int fd, struct epoll_event *events, int maxevents, const struct timespec *timeout, const sigset_t *sigset); The underlying hrtimer is already programmed with nsec resolution. pselect and ppoll also set nsec resolution timeout with timespec. The sigset_t in epoll_pwait has a compat variant. epoll_pwait2 needs the same. For timespec, only support this new interface on 2038 aware platforms that define __kernel_timespec_t. So no CONFIG_COMPAT_32BIT_TIME. Changes v3: - rewrite: add epoll_pwait2 syscall instead of epoll_create1 flag v2: - cast to s64: avoid overflow on 32-bit platforms (Shuo Chen) - minor commit message rewording Signed-off-by: Willem de Bruijn <willemb@google.com> --- This version applies cleanly to linux-next-20201117. --- arch/alpha/kernel/syscalls/syscall.tbl | 1 + arch/arm/tools/syscall.tbl | 1 + arch/arm64/include/asm/unistd.h | 2 +- arch/arm64/include/asm/unistd32.h | 2 + arch/ia64/kernel/syscalls/syscall.tbl | 1 + arch/m68k/kernel/syscalls/syscall.tbl | 1 + arch/microblaze/kernel/syscalls/syscall.tbl | 1 + arch/mips/kernel/syscalls/syscall_n32.tbl | 1 + arch/mips/kernel/syscalls/syscall_n64.tbl | 1 + arch/mips/kernel/syscalls/syscall_o32.tbl | 1 + arch/parisc/kernel/syscalls/syscall.tbl | 1 + arch/powerpc/kernel/syscalls/syscall.tbl | 1 + arch/s390/kernel/syscalls/syscall.tbl | 1 + arch/sh/kernel/syscalls/syscall.tbl | 1 + arch/sparc/kernel/syscalls/syscall.tbl | 1 + arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/x86/entry/syscalls/syscall_64.tbl | 1 + arch/xtensa/kernel/syscalls/syscall.tbl | 1 + fs/eventpoll.c | 106 ++++++++++++++++---- include/linux/compat.h | 6 ++ include/linux/syscalls.h | 5 + include/uapi/asm-generic/unistd.h | 4 +- kernel/sys_ni.c | 2 + 23 files changed, 123 insertions(+), 20 deletions(-) diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl index c5cc5bfa2062..e0a599bfb0e1 100644 --- a/arch/alpha/kernel/syscalls/syscall.tbl +++ b/arch/alpha/kernel/syscalls/syscall.tbl @@ -481,3 +481,4 @@ 549 common faccessat2 sys_faccessat2 550 common process_madvise sys_process_madvise 551 common watch_mount sys_watch_mount +552 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl index 47325b3b661a..dbde88a855b6 100644 --- a/arch/arm/tools/syscall.tbl +++ b/arch/arm/tools/syscall.tbl @@ -455,3 +455,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/arm64/include/asm/unistd.h b/arch/arm64/include/asm/unistd.h index 949788f5ba40..d1f7d35f986e 100644 --- a/arch/arm64/include/asm/unistd.h +++ b/arch/arm64/include/asm/unistd.h @@ -38,7 +38,7 @@ #define __ARM_NR_compat_set_tls (__ARM_NR_COMPAT_BASE + 5) #define __ARM_NR_COMPAT_END (__ARM_NR_COMPAT_BASE + 0x800) -#define __NR_compat_syscalls 443 +#define __NR_compat_syscalls 444 #endif #define __ARCH_WANT_SYS_CLONE diff --git a/arch/arm64/include/asm/unistd32.h b/arch/arm64/include/asm/unistd32.h index c71c3fe0b6cd..b84e24a7e2c0 100644 --- a/arch/arm64/include/asm/unistd32.h +++ b/arch/arm64/include/asm/unistd32.h @@ -893,6 +893,8 @@ __SYSCALL(__NR_process_madvise, sys_process_madvise) __SYSCALL(__NR_watch_mount, sys_watch_mount) #define __NR_memfd_secret 442 __SYSCALL(__NR_memfd_secret, sys_memfd_secret) +#define __NR_epoll_pwait2 443 +__SYSCALL(__NR_epoll_pwait2, sys_epoll_pwait2) /* * Please add new compat syscalls above this comment and update diff --git a/arch/ia64/kernel/syscalls/syscall.tbl b/arch/ia64/kernel/syscalls/syscall.tbl index 033244462350..c8809959636f 100644 --- a/arch/ia64/kernel/syscalls/syscall.tbl +++ b/arch/ia64/kernel/syscalls/syscall.tbl @@ -362,3 +362,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl index efd3ecb3cdfc..dde585616bf8 100644 --- a/arch/m68k/kernel/syscalls/syscall.tbl +++ b/arch/m68k/kernel/syscalls/syscall.tbl @@ -441,3 +441,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl index 67ae5a5e4d21..4c09f27fedd0 100644 --- a/arch/microblaze/kernel/syscalls/syscall.tbl +++ b/arch/microblaze/kernel/syscalls/syscall.tbl @@ -447,3 +447,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl index c59bc6acc47a..00921244242d 100644 --- a/arch/mips/kernel/syscalls/syscall_n32.tbl +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl @@ -380,3 +380,4 @@ 439 n32 faccessat2 sys_faccessat2 440 n32 process_madvise sys_process_madvise 441 n32 watch_mount sys_watch_mount +443 n32 epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl index 50bbb9517052..b7920f76358d 100644 --- a/arch/mips/kernel/syscalls/syscall_n64.tbl +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl @@ -356,3 +356,4 @@ 439 n64 faccessat2 sys_faccessat2 440 n64 process_madvise sys_process_madvise 441 n64 watch_mount sys_watch_mount +443 n64 epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl index 1d5644e221b4..2ab5ed500113 100644 --- a/arch/mips/kernel/syscalls/syscall_o32.tbl +++ b/arch/mips/kernel/syscalls/syscall_o32.tbl @@ -429,3 +429,4 @@ 439 o32 faccessat2 sys_faccessat2 440 o32 process_madvise sys_process_madvise 441 o32 watch_mount sys_watch_mount +443 o32 epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl index 4492cc2fce23..9caf7d969846 100644 --- a/arch/parisc/kernel/syscalls/syscall.tbl +++ b/arch/parisc/kernel/syscalls/syscall.tbl @@ -439,3 +439,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl index ee122bfc2ddc..dce635420226 100644 --- a/arch/powerpc/kernel/syscalls/syscall.tbl +++ b/arch/powerpc/kernel/syscalls/syscall.tbl @@ -531,3 +531,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl index b467b57d66d1..dedfa6db9f8d 100644 --- a/arch/s390/kernel/syscalls/syscall.tbl +++ b/arch/s390/kernel/syscalls/syscall.tbl @@ -444,3 +444,4 @@ 439 common faccessat2 sys_faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl index f9c7ca5cb969..f7a383ae2c48 100644 --- a/arch/sh/kernel/syscalls/syscall.tbl +++ b/arch/sh/kernel/syscalls/syscall.tbl @@ -444,3 +444,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl index 341fd4ba053b..a6cdf450d0f4 100644 --- a/arch/sparc/kernel/syscalls/syscall.tbl +++ b/arch/sparc/kernel/syscalls/syscall.tbl @@ -487,3 +487,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl index 109e6681b8fa..9a4e8ec207fc 100644 --- a/arch/x86/entry/syscalls/syscall_32.tbl +++ b/arch/x86/entry/syscalls/syscall_32.tbl @@ -447,3 +447,4 @@ 440 i386 process_madvise sys_process_madvise 441 i386 watch_mount sys_watch_mount 442 i386 memfd_secret sys_memfd_secret +443 i386 epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl index 742cf17d7725..e2cb5f87e760 100644 --- a/arch/x86/entry/syscalls/syscall_64.tbl +++ b/arch/x86/entry/syscalls/syscall_64.tbl @@ -364,6 +364,7 @@ 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount 442 common memfd_secret sys_memfd_secret +443 common epoll_pwait2 sys_epoll_pwait2 # # Due to a historical design error, certain syscalls are numbered differently diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl index 6225d87be81f..1c8ae5fd026e 100644 --- a/arch/xtensa/kernel/syscalls/syscall.tbl +++ b/arch/xtensa/kernel/syscalls/syscall.tbl @@ -412,3 +412,4 @@ 439 common faccessat2 sys_faccessat2 440 common process_madvise sys_process_madvise 441 common watch_mount sys_watch_mount +443 common epoll_pwait2 sys_epoll_pwait2 diff --git a/fs/eventpoll.c b/fs/eventpoll.c index 297aeb0ee9d1..0f0543e1cc6f 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -1714,13 +1714,11 @@ static int ep_send_events(struct eventpoll *ep, return res; } -static inline struct timespec64 ep_set_mstimeout(long ms) +static inline struct timespec64 ep_set_nstimeout(s64 timeout) { - struct timespec64 now, ts = { - .tv_sec = ms / MSEC_PER_SEC, - .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), - }; + struct timespec64 now, ts; + ts = ns_to_timespec64(timeout); ktime_get_ts64(&now); return timespec64_add_safe(now, ts); } @@ -1734,7 +1732,7 @@ static inline struct timespec64 ep_set_mstimeout(long ms) * stored. * @maxevents: Size (in terms of number of events) of the caller event buffer. * @timeout: Maximum timeout for the ready events fetch operation, in - * milliseconds. If the @timeout is zero, the function will not block, + * nanoseconds. If the @timeout is zero, the function will not block, * while if the @timeout is less than zero, the function will block * until at least one event has been retrieved (or an error * occurred). @@ -1743,7 +1741,7 @@ static inline struct timespec64 ep_set_mstimeout(long ms) * error code, in case of error. */ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, - int maxevents, long timeout) + int maxevents, s64 timeout) { int res, eavail, timed_out = 0; u64 slack = 0; @@ -1753,7 +1751,7 @@ static int ep_poll(struct eventpoll *ep, struct epoll_event __user *events, lockdep_assert_irqs_enabled(); if (timeout > 0) { - struct timespec64 end_time = ep_set_mstimeout(timeout); + struct timespec64 end_time = ep_set_nstimeout(timeout); slack = select_estimate_accuracy(&end_time); to = &expires; @@ -2177,7 +2175,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd, * part of the user space epoll_wait(2). */ static int do_epoll_wait(int epfd, struct epoll_event __user *events, - int maxevents, int timeout) + int maxevents, s64 timeout) { int error; struct fd f; @@ -2218,19 +2216,27 @@ static int do_epoll_wait(int epfd, struct epoll_event __user *events, return error; } +static s64 timeout_to_ns(int timeout) +{ + if (timeout <= 0) + return timeout; + else + return timeout * (s64)NSEC_PER_MSEC; +} + SYSCALL_DEFINE4(epoll_wait, int, epfd, struct epoll_event __user *, events, int, maxevents, int, timeout) { - return do_epoll_wait(epfd, events, maxevents, timeout); + return do_epoll_wait(epfd, events, maxevents, timeout_to_ns(timeout)); } /* * Implement the event wait interface for the eventpoll file. It is the kernel * part of the user space epoll_pwait(2). */ -SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events, - int, maxevents, int, timeout, const sigset_t __user *, sigmask, - size_t, sigsetsize) +static int do_epoll_pwait(int epfd, struct epoll_event __user *events, + int maxevents, s64 timeout, + const sigset_t __user *sigmask, size_t sigsetsize) { int error; @@ -2248,12 +2254,40 @@ SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events, return error; } +SYSCALL_DEFINE6(epoll_pwait, int, epfd, struct epoll_event __user *, events, + int, maxevents, int, timeout, const sigset_t __user *, sigmask, + size_t, sigsetsize) +{ + return do_epoll_pwait(epfd, events, maxevents, timeout_to_ns(timeout), + sigmask, sigsetsize); +} + +SYSCALL_DEFINE6(epoll_pwait2, int, epfd, struct epoll_event __user *, events, + int, maxevents, const struct __kernel_timespec __user *, timeout, + const sigset_t __user *, sigmask, size_t, sigsetsize) +{ + struct timespec64 ts; + s64 timeout_ns; + + if (timeout) { + if (get_timespec64(&ts, timeout)) + return -EFAULT; + if (!timespec64_valid(&ts)) + return -EINVAL; + timeout_ns = timespec64_to_ns(&ts); + } else { + timeout_ns = -1; + } + + return do_epoll_pwait(epfd, events, maxevents, timeout_ns, + sigmask, sigsetsize); +} + #ifdef CONFIG_COMPAT -COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd, - struct epoll_event __user *, events, - int, maxevents, int, timeout, - const compat_sigset_t __user *, sigmask, - compat_size_t, sigsetsize) +static int do_compat_epoll_pwait(int epfd, struct epoll_event __user *events, + int maxevents, s64 timeout, + const compat_sigset_t __user *sigmask, + compat_size_t sigsetsize) { long err; @@ -2270,6 +2304,42 @@ COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd, return err; } + +COMPAT_SYSCALL_DEFINE6(epoll_pwait, int, epfd, + struct epoll_event __user *, events, + int, maxevents, int, timeout, + const compat_sigset_t __user *, sigmask, + compat_size_t, sigsetsize) +{ + return do_compat_epoll_pwait(epfd, events, maxevents, + timeout_to_ns(timeout), + sigmask, sigsetsize); +} + +COMPAT_SYSCALL_DEFINE6(epoll_pwait2, int, epfd, + struct epoll_event __user *, events, + int, maxevents, + const struct __kernel_timespec __user *, timeout, + const compat_sigset_t __user *, sigmask, + compat_size_t, sigsetsize) +{ + struct timespec64 ts; + s64 timeout_ns; + + if (timeout) { + if (get_timespec64(&ts, timeout)) + return -EFAULT; + if (!timespec64_valid(&ts)) + return -EINVAL; + timeout_ns = timespec64_to_ns(&ts); + } else { + timeout_ns = -1; + } + + return do_compat_epoll_pwait(epfd, events, maxevents, timeout_ns, + sigmask, sigsetsize); +} + #endif static int __init eventpoll_init(void) diff --git a/include/linux/compat.h b/include/linux/compat.h index 14d514233e1d..76dc28a2e7f1 100644 --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -505,6 +505,12 @@ asmlinkage long compat_sys_epoll_pwait(int epfd, int maxevents, int timeout, const compat_sigset_t __user *sigmask, compat_size_t sigsetsize); +asmlinkage long compat_sys_epoll_pwait2(int epfd, + struct epoll_event __user *events, + int maxevents, + const struct __kernel_timespec __user *timeout, + const compat_sigset_t __user *sigmask, + compat_size_t sigsetsize); /* fs/fcntl.c */ asmlinkage long compat_sys_fcntl(unsigned int fd, unsigned int cmd, diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index e87a96ace85b..2ba45fb7874c 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -362,6 +362,11 @@ asmlinkage long sys_epoll_pwait(int epfd, struct epoll_event __user *events, int maxevents, int timeout, const sigset_t __user *sigmask, size_t sigsetsize); +asmlinkage long sys_epoll_pwait2(int epfd, struct epoll_event __user *events, + int maxevents, + const struct __kernel_timespec __user *timeout, + const sigset_t __user *sigmask, + size_t sigsetsize); /* fs/fcntl.c */ asmlinkage long sys_dup(unsigned int fildes); diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h index 3bc087d14535..aa883388700e 100644 --- a/include/uapi/asm-generic/unistd.h +++ b/include/uapi/asm-generic/unistd.h @@ -865,9 +865,11 @@ __SYSCALL(__NR_watch_mount, sys_watch_mount) #define __NR_memfd_secret 442 __SYSCALL(__NR_memfd_secret, sys_memfd_secret) #endif +#define __NR_epoll_pwait2 443 +__SC_COMP(__NR_epoll_pwait2, sys_epoll_pwait2, compat_sys_epoll_pwait2) #undef __NR_syscalls -#define __NR_syscalls 443 +#define __NR_syscalls 444 /* * 32 bit systems traditionally used different diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c index 805fd7a668be..869aa6b5bf34 100644 --- a/kernel/sys_ni.c +++ b/kernel/sys_ni.c @@ -68,6 +68,8 @@ COND_SYSCALL(epoll_create1); COND_SYSCALL(epoll_ctl); COND_SYSCALL(epoll_pwait); COND_SYSCALL_COMPAT(epoll_pwait); +COND_SYSCALL(epoll_pwait2); +COND_SYSCALL_COMPAT(epoll_pwait2); /* fs/fcntl.c */ -- 2.29.2.454.gaff20da3a2-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 14:46 ` [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 Willem de Bruijn @ 2020-11-18 15:00 ` Matthew Wilcox 2020-11-18 15:10 ` Willem de Bruijn 2020-11-18 16:21 ` Willem de Bruijn 1 sibling, 1 reply; 25+ messages in thread From: Matthew Wilcox @ 2020-11-18 15:00 UTC (permalink / raw) To: Willem de Bruijn Cc: linux-fsdevel, linux-kernel, viro, akpm, soheil.kdev, arnd, shuochen, linux-man, Willem de Bruijn On Wed, Nov 18, 2020 at 09:46:15AM -0500, Willem de Bruijn wrote: > -static inline struct timespec64 ep_set_mstimeout(long ms) > +static inline struct timespec64 ep_set_nstimeout(s64 timeout) > { > - struct timespec64 now, ts = { > - .tv_sec = ms / MSEC_PER_SEC, > - .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), > - }; > + struct timespec64 now, ts; > > + ts = ns_to_timespec64(timeout); > ktime_get_ts64(&now); > return timespec64_add_safe(now, ts); > } Why do you pass around an s64 for timeout, converting it to and from a timespec64 instead of passing around a timespec64? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 15:00 ` Matthew Wilcox @ 2020-11-18 15:10 ` Willem de Bruijn 2020-11-18 15:37 ` Arnd Bergmann 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-11-18 15:10 UTC (permalink / raw) To: Matthew Wilcox Cc: linux-fsdevel, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Wed, Nov 18, 2020 at 10:00 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Wed, Nov 18, 2020 at 09:46:15AM -0500, Willem de Bruijn wrote: > > -static inline struct timespec64 ep_set_mstimeout(long ms) > > +static inline struct timespec64 ep_set_nstimeout(s64 timeout) > > { > > - struct timespec64 now, ts = { > > - .tv_sec = ms / MSEC_PER_SEC, > > - .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), > > - }; > > + struct timespec64 now, ts; > > > > + ts = ns_to_timespec64(timeout); > > ktime_get_ts64(&now); > > return timespec64_add_safe(now, ts); > > } > > Why do you pass around an s64 for timeout, converting it to and from > a timespec64 instead of passing around a timespec64? I implemented both approaches. The alternative was no simpler. Conversion in existing epoll_wait, epoll_pwait and epoll_pwait (compat) becomes a bit more complex and adds a stack variable there if passing the timespec64 by reference. And in ep_poll the ternary timeout test > 0, 0, < 0 now requires checking both tv_secs and tv_nsecs. Based on that, I found this simpler. But no strong preference. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 15:10 ` Willem de Bruijn @ 2020-11-18 15:37 ` Arnd Bergmann 2020-11-18 15:59 ` David Laight 0 siblings, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-11-18 15:37 UTC (permalink / raw) To: Willem de Bruijn Cc: Matthew Wilcox, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Wed, Nov 18, 2020 at 4:10 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Wed, Nov 18, 2020 at 10:00 AM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Wed, Nov 18, 2020 at 09:46:15AM -0500, Willem de Bruijn wrote: > > > -static inline struct timespec64 ep_set_mstimeout(long ms) > > > +static inline struct timespec64 ep_set_nstimeout(s64 timeout) > > > { > > > - struct timespec64 now, ts = { > > > - .tv_sec = ms / MSEC_PER_SEC, > > > - .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), > > > - }; > > > + struct timespec64 now, ts; > > > > > > + ts = ns_to_timespec64(timeout); > > > ktime_get_ts64(&now); > > > return timespec64_add_safe(now, ts); > > > } > > > > Why do you pass around an s64 for timeout, converting it to and from > > a timespec64 instead of passing around a timespec64? > > I implemented both approaches. The alternative was no simpler. > Conversion in existing epoll_wait, epoll_pwait and epoll_pwait > (compat) becomes a bit more complex and adds a stack variable there if > passing the timespec64 by reference. And in ep_poll the ternary > timeout test > 0, 0, < 0 now requires checking both tv_secs and > tv_nsecs. Based on that, I found this simpler. But no strong > preference. The 64-bit division can be fairly expensive on 32-bit architectures, at least when it doesn't get optimized into a multiply+shift. Arnd ^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 15:37 ` Arnd Bergmann @ 2020-11-18 15:59 ` David Laight 2020-11-19 14:19 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: David Laight @ 2020-11-18 15:59 UTC (permalink / raw) To: 'Arnd Bergmann', Willem de Bruijn Cc: Matthew Wilcox, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man From: Arnd Bergmann > Sent: 18 November 2020 15:38 > > On Wed, Nov 18, 2020 at 4:10 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > On Wed, Nov 18, 2020 at 10:00 AM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > On Wed, Nov 18, 2020 at 09:46:15AM -0500, Willem de Bruijn wrote: > > > > -static inline struct timespec64 ep_set_mstimeout(long ms) > > > > +static inline struct timespec64 ep_set_nstimeout(s64 timeout) > > > > { > > > > - struct timespec64 now, ts = { > > > > - .tv_sec = ms / MSEC_PER_SEC, > > > > - .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), > > > > - }; > > > > + struct timespec64 now, ts; > > > > > > > > + ts = ns_to_timespec64(timeout); > > > > ktime_get_ts64(&now); > > > > return timespec64_add_safe(now, ts); > > > > } > > > > > > Why do you pass around an s64 for timeout, converting it to and from > > > a timespec64 instead of passing around a timespec64? > > > > I implemented both approaches. The alternative was no simpler. > > Conversion in existing epoll_wait, epoll_pwait and epoll_pwait > > (compat) becomes a bit more complex and adds a stack variable there if > > passing the timespec64 by reference. And in ep_poll the ternary > > timeout test > 0, 0, < 0 now requires checking both tv_secs and > > tv_nsecs. Based on that, I found this simpler. But no strong > > preference. > > The 64-bit division can be fairly expensive on 32-bit architectures, > at least when it doesn't get optimized into a multiply+shift. I'd have thought you'd want to do everything in 64bit nanosecs. Conversions to/from any of the 'timespec' structure are expensive. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 15:59 ` David Laight @ 2020-11-19 14:19 ` Willem de Bruijn 2020-11-19 14:31 ` Matthew Wilcox 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-11-19 14:19 UTC (permalink / raw) To: David Laight Cc: Arnd Bergmann, Matthew Wilcox, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Wed, Nov 18, 2020 at 10:59 AM David Laight <David.Laight@aculab.com> wrote: > > From: Arnd Bergmann > > Sent: 18 November 2020 15:38 > > > > On Wed, Nov 18, 2020 at 4:10 PM Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> wrote: > > > On Wed, Nov 18, 2020 at 10:00 AM Matthew Wilcox <willy@infradead.org> wrote: > > > > > > > > On Wed, Nov 18, 2020 at 09:46:15AM -0500, Willem de Bruijn wrote: > > > > > -static inline struct timespec64 ep_set_mstimeout(long ms) > > > > > +static inline struct timespec64 ep_set_nstimeout(s64 timeout) > > > > > { > > > > > - struct timespec64 now, ts = { > > > > > - .tv_sec = ms / MSEC_PER_SEC, > > > > > - .tv_nsec = NSEC_PER_MSEC * (ms % MSEC_PER_SEC), > > > > > - }; > > > > > + struct timespec64 now, ts; > > > > > > > > > > + ts = ns_to_timespec64(timeout); > > > > > ktime_get_ts64(&now); > > > > > return timespec64_add_safe(now, ts); > > > > > } > > > > > > > > Why do you pass around an s64 for timeout, converting it to and from > > > > a timespec64 instead of passing around a timespec64? > > > > > > I implemented both approaches. The alternative was no simpler. > > > Conversion in existing epoll_wait, epoll_pwait and epoll_pwait > > > (compat) becomes a bit more complex and adds a stack variable there if > > > passing the timespec64 by reference. And in ep_poll the ternary > > > timeout test > 0, 0, < 0 now requires checking both tv_secs and > > > tv_nsecs. Based on that, I found this simpler. But no strong > > > preference. > > > > The 64-bit division can be fairly expensive on 32-bit architectures, > > at least when it doesn't get optimized into a multiply+shift. > > I'd have thought you'd want to do everything in 64bit nanosecs. > Conversions to/from any of the 'timespec' structure are expensive. I took another look at this. The only real reason for the timespec64 is that select_estimate_accuracy takes that type. Which makes sense, because do_select does. But for epoll, this is inefficient: in ep_set_mstimeout it calls ktime_get_ts64 to convert timeout to an offset from current time, only to pass it to select_estimate_accuracy to then perform another ktime_get_ts64 and subtract this to get back to (approx.) the original timeout. How about a separate patch that adds epoll_estimate_accuracy with the same rules (wrt rt_task, current->timer_slack, nice and upper bound) but taking an s64 timeout. One variation, since it is approximate, I suppose we could even replace division by a right shift? After that, using s64 everywhere is indeed much simpler. And with that I will revise the new epoll_pwait2 interface to take a long long instead of struct timespec. Apologies for the delay. I forgot that I'm only subscribed to netdev@ in my main email account. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-19 14:19 ` Willem de Bruijn @ 2020-11-19 14:31 ` Matthew Wilcox 2020-11-19 15:37 ` Willem de Bruijn 2020-11-19 15:45 ` Arnd Bergmann 0 siblings, 2 replies; 25+ messages in thread From: Matthew Wilcox @ 2020-11-19 14:31 UTC (permalink / raw) To: Willem de Bruijn Cc: David Laight, Arnd Bergmann, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Thu, Nov 19, 2020 at 09:19:35AM -0500, Willem de Bruijn wrote: > But for epoll, this is inefficient: in ep_set_mstimeout it calls > ktime_get_ts64 to convert timeout to an offset from current time, only > to pass it to select_estimate_accuracy to then perform another > ktime_get_ts64 and subtract this to get back to (approx.) the original > timeout. > > How about a separate patch that adds epoll_estimate_accuracy with > the same rules (wrt rt_task, current->timer_slack, nice and upper bound) > but taking an s64 timeout. > > One variation, since it is approximate, I suppose we could even replace > division by a right shift? > > After that, using s64 everywhere is indeed much simpler. And with that > I will revise the new epoll_pwait2 interface to take a long long > instead of struct timespec. I think the userspace interface should take a struct timespec for consistency with ppoll and pselect. And epoll should use poll_select_set_timeout() to convert the relative timeout to an absolute endtime. Make epoll more consistent with select/poll, not less ... ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-19 14:31 ` Matthew Wilcox @ 2020-11-19 15:37 ` Willem de Bruijn 2020-11-19 15:45 ` Arnd Bergmann 1 sibling, 0 replies; 25+ messages in thread From: Willem de Bruijn @ 2020-11-19 15:37 UTC (permalink / raw) To: Matthew Wilcox Cc: David Laight, Arnd Bergmann, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Thu, Nov 19, 2020 at 9:31 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Thu, Nov 19, 2020 at 09:19:35AM -0500, Willem de Bruijn wrote: > > But for epoll, this is inefficient: in ep_set_mstimeout it calls > > ktime_get_ts64 to convert timeout to an offset from current time, only > > to pass it to select_estimate_accuracy to then perform another > > ktime_get_ts64 and subtract this to get back to (approx.) the original > > timeout. > > > > How about a separate patch that adds epoll_estimate_accuracy with > > the same rules (wrt rt_task, current->timer_slack, nice and upper bound) > > but taking an s64 timeout. > > > > One variation, since it is approximate, I suppose we could even replace > > division by a right shift? > > > > After that, using s64 everywhere is indeed much simpler. And with that > > I will revise the new epoll_pwait2 interface to take a long long > > instead of struct timespec. > > I think the userspace interface should take a struct timespec > for consistency with ppoll and pselect. And epoll should use > poll_select_set_timeout() to convert the relative timeout to an absolute > endtime. Make epoll more consistent with select/poll, not less ... Okay. The absolute time is also needed for schedule_hrtimeout_range, so it could not be entirely avoided, anyway. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-19 14:31 ` Matthew Wilcox 2020-11-19 15:37 ` Willem de Bruijn @ 2020-11-19 15:45 ` Arnd Bergmann 2020-11-19 20:13 ` Willem de Bruijn 1 sibling, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-11-19 15:45 UTC (permalink / raw) To: Matthew Wilcox Cc: Willem de Bruijn, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Thu, Nov 19, 2020 at 3:31 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Thu, Nov 19, 2020 at 09:19:35AM -0500, Willem de Bruijn wrote: > > But for epoll, this is inefficient: in ep_set_mstimeout it calls > > ktime_get_ts64 to convert timeout to an offset from current time, only > > to pass it to select_estimate_accuracy to then perform another > > ktime_get_ts64 and subtract this to get back to (approx.) the original > > timeout. Right, it would be good to avoid the second ktime_get_ts64(), as reading the clocksource itself can be expensive. > > How about a separate patch that adds epoll_estimate_accuracy with > > the same rules (wrt rt_task, current->timer_slack, nice and upper bound) > > but taking an s64 timeout. > > > > One variation, since it is approximate, I suppose we could even replace > > division by a right shift? The right shift would work indeed, but it's also a bit ugly unless __estimate_accuracy() is changed to always use the same shift. I see that on 32-bit ARM, select_estimate_accuracy() calls the external __aeabi_idiv() function to do the 32-bit division, so changing it to a shift would speed up select as well. Changing select_estimate_accuracy() to take the relative timeout as an argument to avoid the extra ktime_get_ts64() should have a larger impact. > > After that, using s64 everywhere is indeed much simpler. And with that > > I will revise the new epoll_pwait2 interface to take a long long > > instead of struct timespec. > > I think the userspace interface should take a struct timespec > for consistency with ppoll and pselect. And epoll should use > poll_select_set_timeout() to convert the relative timeout to an absolute > endtime. Make epoll more consistent with select/poll, not less ... I don't see a problem with an s64 timeout if that makes the interface simpler by avoiding differences between the 32-bit and 64-bit ABIs. More importantly, I think it should differ from poll/select by calculating and writing back the remaining timeout. I don't know what the latest view on absolute timeouts at the syscall ABI is, it would probably simplify the implementation, but make it less consistent with the others. Futex uses absolute timeouts, but is itself inconsistent about that. Arnd ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-19 15:45 ` Arnd Bergmann @ 2020-11-19 20:13 ` Willem de Bruijn 2020-11-20 8:13 ` Arnd Bergmann 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-11-19 20:13 UTC (permalink / raw) To: Arnd Bergmann Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Thu, Nov 19, 2020 at 10:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Thu, Nov 19, 2020 at 3:31 PM Matthew Wilcox <willy@infradead.org> wrote: > > > > On Thu, Nov 19, 2020 at 09:19:35AM -0500, Willem de Bruijn wrote: > > > But for epoll, this is inefficient: in ep_set_mstimeout it calls > > > ktime_get_ts64 to convert timeout to an offset from current time, only > > > to pass it to select_estimate_accuracy to then perform another > > > ktime_get_ts64 and subtract this to get back to (approx.) the original > > > timeout. > > Right, it would be good to avoid the second ktime_get_ts64(), as reading > the clocksource itself can be expensive. > > > > How about a separate patch that adds epoll_estimate_accuracy with > > > the same rules (wrt rt_task, current->timer_slack, nice and upper bound) > > > but taking an s64 timeout. > > > > > > One variation, since it is approximate, I suppose we could even replace > > > division by a right shift? > > The right shift would work indeed, but it's also a bit ugly unless > __estimate_accuracy() is changed to always use the same shift. > > I see that on 32-bit ARM, select_estimate_accuracy() calls > the external __aeabi_idiv() function to do the 32-bit division, so > changing it to a shift would speed up select as well. > > Changing select_estimate_accuracy() to take the relative timeout > as an argument to avoid the extra ktime_get_ts64() should > have a larger impact. It could be done by having poll_select_set_timeout take an extra u64* slack, call select_estimate_accuracy before adding in the current time and then pass the slack down to do_select and do_sys_poll, also through core_sys_select and compat_core_sys_select. It could be a patch independent from this new syscall. Since it changes poll_select_set_timeout it clearly has a conflict with the planned next revision of this. I can include it in the next patchset to decide whether it's worth it. > > > After that, using s64 everywhere is indeed much simpler. And with that > > > I will revise the new epoll_pwait2 interface to take a long long > > > instead of struct timespec. > > > > I think the userspace interface should take a struct timespec > > for consistency with ppoll and pselect. And epoll should use > > poll_select_set_timeout() to convert the relative timeout to an absolute > > endtime. Make epoll more consistent with select/poll, not less ... > > I don't see a problem with an s64 timeout if that makes the interface > simpler by avoiding differences between the 32-bit and 64-bit ABIs. > > More importantly, I think it should differ from poll/select by calculating > and writing back the remaining timeout. > > I don't know what the latest view on absolute timeouts at the syscall > ABI is, it would probably simplify the implementation, but make it > less consistent with the others. Futex uses absolute timeouts, but > is itself inconsistent about that. If the implementation internally uses poll_select_set_timeout and passes around timespec64 *, it won't matter much in terms of performance or implementation. Then there seems to be no downside to following the consistency argument. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-19 20:13 ` Willem de Bruijn @ 2020-11-20 8:13 ` Arnd Bergmann 2020-11-20 16:01 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-11-20 8:13 UTC (permalink / raw) To: Willem de Bruijn Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Thu, Nov 19, 2020 at 9:13 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Thu, Nov 19, 2020 at 10:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Thu, Nov 19, 2020 at 3:31 PM Matthew Wilcox <willy@infradead.org> wrote: > > The right shift would work indeed, but it's also a bit ugly unless > > __estimate_accuracy() is changed to always use the same shift. > > > > I see that on 32-bit ARM, select_estimate_accuracy() calls > > the external __aeabi_idiv() function to do the 32-bit division, so > > changing it to a shift would speed up select as well. > > > > Changing select_estimate_accuracy() to take the relative timeout > > as an argument to avoid the extra ktime_get_ts64() should > > have a larger impact. > > It could be done by having poll_select_set_timeout take an extra u64* > slack, call select_estimate_accuracy before adding in the current time > and then pass the slack down to do_select and do_sys_poll, also > through core_sys_select and compat_core_sys_select. > > It could be a patch independent from this new syscall. Since it changes > poll_select_set_timeout it clearly has a conflict with the planned next > revision of this. I can include it in the next patchset to decide whether > it's worth it. Yes, that sounds good, not sure how much rework this would require. It would be easier to do if we first merged the native and compat native versions of select/pselect/ppoll by moving the in_compat_syscall() check into combined get_sigset() and get_fd_set() helpers. I would assume you have enough on your plate already and don't want to add that to it. > > I don't see a problem with an s64 timeout if that makes the interface > > simpler by avoiding differences between the 32-bit and 64-bit ABIs. > > > > More importantly, I think it should differ from poll/select by calculating > > and writing back the remaining timeout. > > > > I don't know what the latest view on absolute timeouts at the syscall > > ABI is, it would probably simplify the implementation, but make it > > less consistent with the others. Futex uses absolute timeouts, but > > is itself inconsistent about that. > > If the implementation internally uses poll_select_set_timeout and > passes around timespec64 *, it won't matter much in terms of > performance or implementation. Then there seems to be no downside to > following the consistency argument. Ok. So to clarify, you would stay with relative __kernel_timespec pointers and not copy back the remaining time, correct? ARnd ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-20 8:13 ` Arnd Bergmann @ 2020-11-20 16:01 ` Willem de Bruijn 2020-11-20 19:23 ` Arnd Bergmann 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-11-20 16:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Fri, Nov 20, 2020 at 3:13 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Thu, Nov 19, 2020 at 9:13 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > On Thu, Nov 19, 2020 at 10:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > On Thu, Nov 19, 2020 at 3:31 PM Matthew Wilcox <willy@infradead.org> wrote: > > > The right shift would work indeed, but it's also a bit ugly unless > > > __estimate_accuracy() is changed to always use the same shift. > > > > > > I see that on 32-bit ARM, select_estimate_accuracy() calls > > > the external __aeabi_idiv() function to do the 32-bit division, so > > > changing it to a shift would speed up select as well. > > > > > > Changing select_estimate_accuracy() to take the relative timeout > > > as an argument to avoid the extra ktime_get_ts64() should > > > have a larger impact. > > > > It could be done by having poll_select_set_timeout take an extra u64* > > slack, call select_estimate_accuracy before adding in the current time > > and then pass the slack down to do_select and do_sys_poll, also > > through core_sys_select and compat_core_sys_select. > > > > It could be a patch independent from this new syscall. Since it changes > > poll_select_set_timeout it clearly has a conflict with the planned next > > revision of this. I can include it in the next patchset to decide whether > > it's worth it. > > Yes, that sounds good, not sure how much rework this would require. > > It would be easier to do if we first merged the native and compat > native versions of select/pselect/ppoll by moving the > in_compat_syscall() check into combined get_sigset() > and get_fd_set() helpers. I would assume you have enough > on your plate already and don't want to add that to it. Thanks for the suggestion. I do have an initial patchset. As expected, it does involve quite a bit of code churn to pass slack through the callers. I'll take a look at your suggestion to simplify it. As is, the patchset is not ready to send to the list for possible merge. In the meantime, I did push the patchset to github at https://github.com/wdebruij/linux/commits/epoll-nstimeo-1 . I can send a version marked RFC to the list if that's easier. I made the slack specific changes in two separate patches, one to fs/select.c and one to fs/eventpoll.c, and placed these at the end of the patchset. So we could first finish the syscall and then send this as a separate patchset if it proves complex enough. Btw, the other change, to convert epoll implementation to timespec64 before adding the syscall, equally adds some code churn compared to patch v3. But perhaps the end state is cleaner and more consistent. > > > I don't see a problem with an s64 timeout if that makes the interface > > > simpler by avoiding differences between the 32-bit and 64-bit ABIs. > > > > > > More importantly, I think it should differ from poll/select by calculating > > > and writing back the remaining timeout. > > > > > > I don't know what the latest view on absolute timeouts at the syscall > > > ABI is, it would probably simplify the implementation, but make it > > > less consistent with the others. Futex uses absolute timeouts, but > > > is itself inconsistent about that. > > > > If the implementation internally uses poll_select_set_timeout and > > passes around timespec64 *, it won't matter much in terms of > > performance or implementation. Then there seems to be no downside to > > following the consistency argument. > > Ok. So to clarify, you would stay with relative __kernel_timespec > pointers and not copy back the remaining time, correct? That's my understanding, and the current implementation. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-20 16:01 ` Willem de Bruijn @ 2020-11-20 19:23 ` Arnd Bergmann 2020-11-20 22:28 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-11-20 19:23 UTC (permalink / raw) To: Willem de Bruijn Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Fri, Nov 20, 2020 at 5:01 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > On Fri, Nov 20, 2020 at 3:13 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > > On Thu, Nov 19, 2020 at 9:13 PM Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> wrote: > > > On Thu, Nov 19, 2020 at 10:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > Thanks for the suggestion. > > I do have an initial patchset. As expected, it does involve quite a > bit of code churn to pass slack through the callers. I'll take a look > at your suggestion to simplify it. > > As is, the patchset is not ready to send to the list for possible > merge. In the meantime, I did push the patchset to github at > https://github.com/wdebruij/linux/commits/epoll-nstimeo-1 . I can send > a version marked RFC to the list if that's easier. Looks all good to me, just two small things I noticed that you can address before sending the new series: * The div_u64_rem() in ep_timeout_to_timespec() looks wrong, as you are actually dividing a 'long' that does not need it. * In "epoll: wire up syscall epoll_pwait2", the alpha syscall has the wrong number, it should be 110 higher than the others, not 109. > Btw, the other change, to convert epoll implementation to timespec64 > before adding the syscall, equally adds some code churn compared to > patch v3. But perhaps the end state is cleaner and more consistent. Right, that's what I meant. If it causes too much churn, don't worry about it it. Arndd ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-20 19:23 ` Arnd Bergmann @ 2020-11-20 22:28 ` Willem de Bruijn 2020-11-21 9:27 ` Arnd Bergmann 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-11-20 22:28 UTC (permalink / raw) To: Arnd Bergmann Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Fri, Nov 20, 2020 at 2:23 PM Arnd Bergmann <arnd@kernel.org> wrote: > > On Fri, Nov 20, 2020 at 5:01 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > On Fri, Nov 20, 2020 at 3:13 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > > > > On Thu, Nov 19, 2020 at 9:13 PM Willem de Bruijn > > > <willemdebruijn.kernel@gmail.com> wrote: > > > > On Thu, Nov 19, 2020 at 10:45 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > Thanks for the suggestion. > > > > I do have an initial patchset. As expected, it does involve quite a > > bit of code churn to pass slack through the callers. I'll take a look > > at your suggestion to simplify it. > > > > As is, the patchset is not ready to send to the list for possible > > merge. In the meantime, I did push the patchset to github at > > https://github.com/wdebruij/linux/commits/epoll-nstimeo-1 . I can send > > a version marked RFC to the list if that's easier. > > Looks all good to me, just two small things I noticed that you can > address before sending the new series: > > * The div_u64_rem() in ep_timeout_to_timespec() looks wrong, as > you are actually dividing a 'long' that does not need it. > > * In "epoll: wire up syscall epoll_pwait2", the alpha syscall has the > wrong number, it > should be 110 higher than the others, not 109. Thanks! I'll fix these up. > > Btw, the other change, to convert epoll implementation to timespec64 > > before adding the syscall, equally adds some code churn compared to > > patch v3. But perhaps the end state is cleaner and more consistent. > > Right, that's what I meant. If it causes too much churn, don't worry > about it it. I think it'll be better to split the patchsets: epoll: convert internal api to timespec64 epoll: add syscall epoll_pwait2 epoll: wire up syscall epoll_pwait2 selftests/filesystems: expand epoll with epoll_pwait2 and select: compute slack based on relative time epoll: compute slack based on relative time and judge the slack conversion on its own merit. I also would rather not tie this up with the compat deduplication. Happy to take a stab at that though. On that note, when combining functions like int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timespec64 *end_time, u64 slack) and static int compat_core_sys_select(int n, compat_ulong_t __user *inp, compat_ulong_t __user *outp, compat_ulong_t __user *exp, struct timespec64 *end_time, u64 slack) by branching on in_compat_syscall() inside get_fd_set/set_fd_set and deprecating their compat_.. counterparts, what would the argument pointers look like? Or is that not the approach you have in mind? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-20 22:28 ` Willem de Bruijn @ 2020-11-21 9:27 ` Arnd Bergmann 2020-12-10 17:33 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-11-21 9:27 UTC (permalink / raw) To: Willem de Bruijn Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man, Willem de Bruijn On Fri, Nov 20, 2020 at 11:28 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Fri, Nov 20, 2020 at 2:23 PM Arnd Bergmann <arnd@kernel.org> wrote: > > On Fri, Nov 20, 2020 at 5:01 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > I think it'll be better to split the patchsets: > > epoll: convert internal api to timespec64 > epoll: add syscall epoll_pwait2 > epoll: wire up syscall epoll_pwait2 > selftests/filesystems: expand epoll with epoll_pwait2 > > and > > select: compute slack based on relative time > epoll: compute slack based on relative time > > and judge the slack conversion on its own merit. Yes, makes sense. > I also would rather not tie this up with the compat deduplication. > Happy to take a stab at that though. On that note, when combining > functions like > > int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp, > fd_set __user *exp, struct timespec64 *end_time, > u64 slack) > > and > > static int compat_core_sys_select(int n, compat_ulong_t __user *inp, > compat_ulong_t __user *outp, compat_ulong_t __user *exp, > struct timespec64 *end_time, u64 slack) > > by branching on in_compat_syscall() inside get_fd_set/set_fd_set and > deprecating their compat_.. counterparts, what would the argument > pointers look like? Or is that not the approach you have in mind? In this case, the top-level entry points becomes unified, and you get the prototype from core_sys_select() with the native arguments. I would imagine this can be done like the way I proposed for get_bitmap() in sys_migrate_pages: https://lore.kernel.org/lkml/20201102123151.2860165-4-arnd@kernel.org/ Arnd ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-21 9:27 ` Arnd Bergmann @ 2020-12-10 17:33 ` Willem de Bruijn 2020-12-10 20:34 ` Arnd Bergmann 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-12-10 17:33 UTC (permalink / raw) To: Arnd Bergmann Cc: Willem de Bruijn, Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Sat, Nov 21, 2020 at 4:27 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Fri, Nov 20, 2020 at 11:28 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > On Fri, Nov 20, 2020 at 2:23 PM Arnd Bergmann <arnd@kernel.org> wrote: > > > On Fri, Nov 20, 2020 at 5:01 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > > > I think it'll be better to split the patchsets: > > > > epoll: convert internal api to timespec64 > > epoll: add syscall epoll_pwait2 > > epoll: wire up syscall epoll_pwait2 > > selftests/filesystems: expand epoll with epoll_pwait2 > > > > and > > > > select: compute slack based on relative time > > epoll: compute slack based on relative time > > > > and judge the slack conversion on its own merit. > > Yes, makes sense. > > > I also would rather not tie this up with the compat deduplication. > > Happy to take a stab at that though. On that note, when combining > > functions like > > > > int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp, > > fd_set __user *exp, struct timespec64 *end_time, > > u64 slack) > > > > and > > > > static int compat_core_sys_select(int n, compat_ulong_t __user *inp, > > compat_ulong_t __user *outp, compat_ulong_t __user *exp, > > struct timespec64 *end_time, u64 slack) > > > > by branching on in_compat_syscall() inside get_fd_set/set_fd_set and > > deprecating their compat_.. counterparts, what would the argument > > pointers look like? Or is that not the approach you have in mind? > > In this case, the top-level entry points becomes unified, and you get > the prototype from core_sys_select() with the native arguments. > > I would imagine this can be done like the way I proposed > for get_bitmap() in sys_migrate_pages: > > https://lore.kernel.org/lkml/20201102123151.2860165-4-arnd@kernel.org/ Coming back to this. Current patchset includes new select and poll selftests to verify the changes. I need to send a small kselftest patch for that first. Assuming there's no time pressure, I will finish up and send the main changes after the merge window, for the next release then. Current state against linux-next at https://github.com/wdebruij/linux-next-mirror/tree/select-compat-1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-12-10 17:33 ` Willem de Bruijn @ 2020-12-10 20:34 ` Arnd Bergmann 2020-12-10 22:59 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-12-10 20:34 UTC (permalink / raw) To: Willem de Bruijn Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Thu, Dec 10, 2020 at 6:33 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > On Sat, Nov 21, 2020 at 4:27 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Fri, Nov 20, 2020 at 11:28 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > I would imagine this can be done like the way I proposed > > for get_bitmap() in sys_migrate_pages: > > > > https://lore.kernel.org/lkml/20201102123151.2860165-4-arnd@kernel.org/ > > Coming back to this. Current patchset includes new select and poll > selftests to verify the changes. I need to send a small kselftest > patch for that first. > > Assuming there's no time pressure, I will finish up and send the main > changes after the merge window, for the next release then. > > Current state against linux-next at > https://github.com/wdebruij/linux-next-mirror/tree/select-compat-1 Ok, sounds good to me. I've had a (very brief) look and have one suggestion: instead of open-coding the compat vs native mode in multiple places like if (!in_compat_syscall())  return copy_from_user(fdset, ufdset, FDS_BYTES(nr)) ? -EFAULT : 0; else  return compat_get_bitmap(fdset, ufdset, nr); maybe move this into a separate function and call that where needed. I've done this for the get_bitmap() function in my series at https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/commit/?h=compat-alloc-user-space-7&id=b1b23ebb12b635654a2060df49455167a142c5d2 The definition is slightly differrent for cpumask, nodemask and fd_set, so we'd need to try out the best way to structure the code to end up with the most readable version, but it should be possible when there are only three callers (and duplicating the function would be the end of the world either) Arnd ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-12-10 20:34 ` Arnd Bergmann @ 2020-12-10 22:59 ` Willem de Bruijn 2021-01-11 20:06 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-12-10 22:59 UTC (permalink / raw) To: Arnd Bergmann Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Thu, Dec 10, 2020 at 3:34 PM Arnd Bergmann <arnd@kernel.org> wrote: > > On Thu, Dec 10, 2020 at 6:33 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > On Sat, Nov 21, 2020 at 4:27 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > On Fri, Nov 20, 2020 at 11:28 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > > I would imagine this can be done like the way I proposed > > > for get_bitmap() in sys_migrate_pages: > > > > > > https://lore.kernel.org/lkml/20201102123151.2860165-4-arnd@kernel.org/ > > > > Coming back to this. Current patchset includes new select and poll > > selftests to verify the changes. I need to send a small kselftest > > patch for that first. > > > > Assuming there's no time pressure, I will finish up and send the main > > changes after the merge window, for the next release then. > > > > Current state against linux-next at > > https://github.com/wdebruij/linux-next-mirror/tree/select-compat-1 > > Ok, sounds good to me. I've had a (very brief) look and have one > suggestion: instead of open-coding the compat vs native mode > in multiple places like > > if (!in_compat_syscall()) >  return copy_from_user(fdset, ufdset, FDS_BYTES(nr)) ? -EFAULT : 0; > else >  return compat_get_bitmap(fdset, ufdset, nr); > > maybe move this into a separate function and call that where needed. > > I've done this for the get_bitmap() function in my series at > > https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/commit/?h=compat-alloc-user-space-7&id=b1b23ebb12b635654a2060df49455167a142c5d2 > > The definition is slightly differrent for cpumask, nodemask and fd_set, > so we'd need to try out the best way to structure the code to end > up with the most readable version, but it should be possible when > there are only three callers (and duplicating the function would > be the end of the world either) For fd_set there is only a single caller for each direction. Do you prefer helpers even so? For sigmask, with three callers, something along the lines of this? @@ -1138,10 +1135,7 @@ static int do_ppoll(struct pollfd __user *ufds, unsigned int nfds, return -EINVAL; } - if (!in_compat_syscall()) - ret = set_user_sigmask(sigmask, sigsetsize); - else - ret = set_compat_user_sigmask(sigmask, sigsetsize); + ret = set_maybe_compat_user_sigmask(sigmask, sigsetsize); if (ret) return ret; --- a/include/linux/compat.h +++ b/include/linux/compat.h @@ -942,6 +942,17 @@ static inline bool in_compat_syscall(void) { return false; } +static inline int set_maybe_compat_user_sigmask(const void __user *sigmask, + size_t sigsetsize) +{ +#if defined CONFIG_COMPAT + if (unlikely(in_compat_syscall())) + return set_compat_user_sigmask(sigmask, sigsetsize); +#endif + + return set_user_sigmask(sigmask, sigsetsize); +} ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-12-10 22:59 ` Willem de Bruijn @ 2021-01-11 20:06 ` Willem de Bruijn 0 siblings, 0 replies; 25+ messages in thread From: Willem de Bruijn @ 2021-01-11 20:06 UTC (permalink / raw) To: Arnd Bergmann Cc: Matthew Wilcox, David Laight, Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Thu, Dec 10, 2020 at 5:59 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > On Thu, Dec 10, 2020 at 3:34 PM Arnd Bergmann <arnd@kernel.org> wrote: > > > > On Thu, Dec 10, 2020 at 6:33 PM Willem de Bruijn > > <willemdebruijn.kernel@gmail.com> wrote: > > > On Sat, Nov 21, 2020 at 4:27 AM Arnd Bergmann <arnd@kernel.org> wrote: > > > > On Fri, Nov 20, 2020 at 11:28 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > > > I would imagine this can be done like the way I proposed > > > > for get_bitmap() in sys_migrate_pages: > > > > > > > > https://lore.kernel.org/lkml/20201102123151.2860165-4-arnd@kernel.org/ > > > > > > Coming back to this. Current patchset includes new select and poll > > > selftests to verify the changes. I need to send a small kselftest > > > patch for that first. > > > > > > Assuming there's no time pressure, I will finish up and send the main > > > changes after the merge window, for the next release then. > > > > > > Current state against linux-next at > > > https://github.com/wdebruij/linux-next-mirror/tree/select-compat-1 > > > > Ok, sounds good to me. I've had a (very brief) look and have one > > suggestion: instead of open-coding the compat vs native mode > > in multiple places like > > > > if (!in_compat_syscall()) > >  return copy_from_user(fdset, ufdset, FDS_BYTES(nr)) ? -EFAULT : 0; > > else > >  return compat_get_bitmap(fdset, ufdset, nr); > > > > maybe move this into a separate function and call that where needed. > > > > I've done this for the get_bitmap() function in my series at > > > > https://git.kernel.org/pub/scm/linux/kernel/git/arnd/playground.git/commit/?h=compat-alloc-user-space-7&id=b1b23ebb12b635654a2060df49455167a142c5d2 > > > > The definition is slightly differrent for cpumask, nodemask and fd_set, > > so we'd need to try out the best way to structure the code to end > > up with the most readable version, but it should be possible when > > there are only three callers (and duplicating the function would > > be the end of the world either) > > For fd_set there is only a single caller for each direction. Do you > prefer helpers even so? > > For sigmask, with three callers, something along the lines of this? > > @@ -1138,10 +1135,7 @@ static int do_ppoll(struct pollfd __user > *ufds, unsigned int nfds, > return -EINVAL; > } > > - if (!in_compat_syscall()) > - ret = set_user_sigmask(sigmask, sigsetsize); > - else > - ret = set_compat_user_sigmask(sigmask, sigsetsize); > + ret = set_maybe_compat_user_sigmask(sigmask, sigsetsize); > if (ret) > return ret; > > --- a/include/linux/compat.h > +++ b/include/linux/compat.h > @@ -942,6 +942,17 @@ static inline bool in_compat_syscall(void) { > return false; } > > +static inline int set_maybe_compat_user_sigmask(const void __user *sigmask, > + size_t sigsetsize) > +{ > +#if defined CONFIG_COMPAT > + if (unlikely(in_compat_syscall())) > + return set_compat_user_sigmask(sigmask, sigsetsize); > +#endif > + > + return set_user_sigmask(sigmask, sigsetsize); > +} set_user_sigmask is the only open-coded variant that is used more than once. Because it is used in both select.c and eventpoll.c, a helper would have to live in compat.h. This then needs a new dependency on sched_signal.h. So given that this is a simple branch, it might just make logic more complex, instead of less. I can add this change in a separate patch on top of the original three, to judge whether it is worthwhile. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 14:46 ` [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 Willem de Bruijn 2020-11-18 15:00 ` Matthew Wilcox @ 2020-11-18 16:21 ` Willem de Bruijn 2020-11-18 16:50 ` Arnd Bergmann 1 sibling, 1 reply; 25+ messages in thread From: Willem de Bruijn @ 2020-11-18 16:21 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Wed, Nov 18, 2020 at 9:46 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > From: Willem de Bruijn <willemb@google.com> > > Add syscall epoll_pwait2, an epoll_wait variant with nsec resolution > that replaces int timeout with struct timespec. It is equivalent > otherwise. > > int epoll_pwait2(int fd, struct epoll_event *events, > int maxevents, > const struct timespec *timeout, > const sigset_t *sigset); > > The underlying hrtimer is already programmed with nsec resolution. > pselect and ppoll also set nsec resolution timeout with timespec. > > The sigset_t in epoll_pwait has a compat variant. epoll_pwait2 needs > the same. > > For timespec, only support this new interface on 2038 aware platforms > that define __kernel_timespec_t. So no CONFIG_COMPAT_32BIT_TIME. > > Changes > v3: > - rewrite: add epoll_pwait2 syscall instead of epoll_create1 flag > v2: > - cast to s64: avoid overflow on 32-bit platforms (Shuo Chen) > - minor commit message rewording > > Signed-off-by: Willem de Bruijn <willemb@google.com> > > --- > diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl > index 109e6681b8fa..9a4e8ec207fc 100644 > --- a/arch/x86/entry/syscalls/syscall_32.tbl > +++ b/arch/x86/entry/syscalls/syscall_32.tbl > @@ -447,3 +447,4 @@ > 440 i386 process_madvise sys_process_madvise > 441 i386 watch_mount sys_watch_mount > 442 i386 memfd_secret sys_memfd_secret > +443 i386 epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 I should have caught this sooner, but this does not work as intended. x86 will still call epoll_pwait2 with old_timespec32. One approach is a separate epoll_pwait2_time64 syscall, similar to ppoll_time64. But that was added to work around legacy 32-bit ppoll. Not needed for a new API. In libc, ppoll_time64 is declared with type struct __timespec64. That type is not defined in Linux uapi. Will need to look at this some more. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 16:21 ` Willem de Bruijn @ 2020-11-18 16:50 ` Arnd Bergmann 2020-11-19 3:22 ` Willem de Bruijn 0 siblings, 1 reply; 25+ messages in thread From: Arnd Bergmann @ 2020-11-18 16:50 UTC (permalink / raw) To: Willem de Bruijn Cc: Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Wed, Nov 18, 2020 at 5:21 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl > > index 109e6681b8fa..9a4e8ec207fc 100644 > > --- a/arch/x86/entry/syscalls/syscall_32.tbl > > +++ b/arch/x86/entry/syscalls/syscall_32.tbl > > @@ -447,3 +447,4 @@ > > 440 i386 process_madvise sys_process_madvise > > 441 i386 watch_mount sys_watch_mount > > 442 i386 memfd_secret sys_memfd_secret > > +443 i386 epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 > > I should have caught this sooner, but this does not work as intended. > > x86 will still call epoll_pwait2 with old_timespec32. > > One approach is a separate epoll_pwait2_time64 syscall, similar to > ppoll_time64. But that was added to work around legacy 32-bit ppoll. > Not needed for a new API. > > In libc, ppoll_time64 is declared with type struct __timespec64. That > type is not defined in Linux uapi. Will need to look at this some > more. The libc __timespec64 corresponds to the __kernel_timespec64 structure in uapi. It is defined to only have 'long' nanoseconds member because that's what c99 and posix require, but the bits are in the position that matches the lower 32 bits of the 64-bit tv_nsec in the kernel, and get_timespec64() performs the necessary conversion to either check or zero the upper bits. I think all you need in user space is to pass the timeout as a __timespec64 structure and add a conversion in the exported library interface. Arnd ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 2020-11-18 16:50 ` Arnd Bergmann @ 2020-11-19 3:22 ` Willem de Bruijn 0 siblings, 0 replies; 25+ messages in thread From: Willem de Bruijn @ 2020-11-19 3:22 UTC (permalink / raw) To: Arnd Bergmann Cc: Linux FS-devel Mailing List, linux-kernel, Al Viro, Andrew Morton, Soheil Hassas Yeganeh, Arnd Bergmann, Shuo Chen, linux-man On Wed, Nov 18, 2020 at 11:50 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Wed, Nov 18, 2020 at 5:21 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl > > > index 109e6681b8fa..9a4e8ec207fc 100644 > > > --- a/arch/x86/entry/syscalls/syscall_32.tbl > > > +++ b/arch/x86/entry/syscalls/syscall_32.tbl > > > @@ -447,3 +447,4 @@ > > > 440 i386 process_madvise sys_process_madvise > > > 441 i386 watch_mount sys_watch_mount > > > 442 i386 memfd_secret sys_memfd_secret > > > +443 i386 epoll_pwait2 sys_epoll_pwait2 compat_sys_epoll_pwait2 > > > > I should have caught this sooner, but this does not work as intended. > > > > x86 will still call epoll_pwait2 with old_timespec32. > > > > One approach is a separate epoll_pwait2_time64 syscall, similar to > > ppoll_time64. But that was added to work around legacy 32-bit ppoll. > > Not needed for a new API. > > > > In libc, ppoll_time64 is declared with type struct __timespec64. That > > type is not defined in Linux uapi. Will need to look at this some > > more. > > The libc __timespec64 corresponds to the __kernel_timespec64 > structure in uapi. It is defined to only have 'long' nanoseconds > member because that's what c99 and posix require, but the bits > are in the position that matches the lower 32 bits of the 64-bit > tv_nsec in the kernel, and get_timespec64() performs the > necessary conversion to either check or zero the upper bits. > > I think all you need in user space is to pass the timeout as a > __timespec64 structure and add a conversion in the exported > library interface. Indeed, that resolves it, thanks. I'll define that struct in the selftest and update the definition of sys_epoll_pwait2 there. ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH manpages RFC] epoll_wait.2: add epoll_pwait2 2020-11-18 14:46 [PATCH v3 0/2] add epoll_pwait2 syscall Willem de Bruijn 2020-11-18 14:46 ` [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 Willem de Bruijn @ 2020-11-18 14:46 ` Willem de Bruijn 2020-11-18 14:46 ` [PATCH v3 2/2] selftests/filesystems: expand epoll with epoll_pwait2 Willem de Bruijn 2 siblings, 0 replies; 25+ messages in thread From: Willem de Bruijn @ 2020-11-18 14:46 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, viro, akpm, soheil.kdev, arnd, shuochen, linux-man, Willem de Bruijn From: Willem de Bruijn <willemb@google.com> Expand the epoll_wait page with epoll_pwait2, an epoll_wait variant that takes a struct timespec to enable nanosecond resolution timeout. int epoll_pwait2(int fd, struct epoll_event *events, int maxevents, const struct timespec *timeout, const sigset_t *sigset); Signed-off-by: Willem de Bruijn <willemb@google.com> --- man2/epoll_wait.2 | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/man2/epoll_wait.2 b/man2/epoll_wait.2 index 55890c82a53a..01047df28cb1 100644 --- a/man2/epoll_wait.2 +++ b/man2/epoll_wait.2 @@ -22,7 +22,7 @@ .\" .TH EPOLL_WAIT 2 2020-04-11 "Linux" "Linux Programmer's Manual" .SH NAME -epoll_wait, epoll_pwait \- wait for an I/O event on an epoll file descriptor +epoll_wait, epoll_pwait, epoll_pwait2 \- wait for an I/O event on an epoll file descriptor .SH SYNOPSIS .nf .B #include <sys/epoll.h> @@ -32,6 +32,9 @@ epoll_wait, epoll_pwait \- wait for an I/O event on an epoll file descriptor .BI "int epoll_pwait(int " epfd ", struct epoll_event *" events , .BI " int " maxevents ", int " timeout , .BI " const sigset_t *" sigmask ); +.BI "int epoll_pwait2(int " epfd ", struct epoll_event *" events , +.BI " int " maxevents ", const struct timespec *" timeout , +.BI " const sigset_t *" sigmask ); .fi .SH DESCRIPTION The @@ -170,6 +173,25 @@ argument may be specified as NULL, in which case .BR epoll_pwait () is equivalent to .BR epoll_wait (). +.SS epoll_pwait2 () +The +.BR epoll_pwait2 () +system call is equivalent to +.BR epoll_pwait () +except for the +.I timeout +argument. It takes an argument of type +.I timespec +to be able to specify nanosecond resolution timeouts. This argument functions +the same as in +.BR pselect (2) +and +.BR ppoll (2). +If +.I timeout +is NULL, then +.BR epoll_pwait2 () +can block indefinitely. .SH RETURN VALUE When successful, .BR epoll_wait () @@ -217,6 +239,9 @@ Library support is provided in glibc starting with version 2.3.2. .BR epoll_pwait () was added to Linux in kernel 2.6.19. Library support is provided in glibc starting with version 2.6. +.PP +.BR epoll_pwait2 () +was added to Linux in kernel 5.11. .SH CONFORMING TO .BR epoll_wait () is Linux-specific. @@ -267,7 +292,9 @@ this means that timeouts greater than 35.79 minutes are treated as infinity. .SS C library/kernel differences The raw .BR epoll_pwait () -system call has a sixth argument, +and +.BR epoll_pwait2 () +system calls have a sixth argument, .IR "size_t sigsetsize" , which specifies the size in bytes of the .IR sigmask -- 2.29.2.454.gaff20da3a2-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/2] selftests/filesystems: expand epoll with epoll_pwait2 2020-11-18 14:46 [PATCH v3 0/2] add epoll_pwait2 syscall Willem de Bruijn 2020-11-18 14:46 ` [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 Willem de Bruijn 2020-11-18 14:46 ` [PATCH manpages RFC] epoll_wait.2: add epoll_pwait2 Willem de Bruijn @ 2020-11-18 14:46 ` Willem de Bruijn 2 siblings, 0 replies; 25+ messages in thread From: Willem de Bruijn @ 2020-11-18 14:46 UTC (permalink / raw) To: linux-fsdevel Cc: linux-kernel, viro, akpm, soheil.kdev, arnd, shuochen, linux-man, Willem de Bruijn From: Willem de Bruijn <willemb@google.com> Code coverage for the epoll_pwait2 syscall. epoll62: Repeat basic test epoll1, but exercising the new syscall. epoll63: Pass a timespec and exercise the timeout wakeup path. Signed-off-by: Willem de Bruijn <willemb@google.com> --- .../filesystems/epoll/epoll_wakeup_test.c | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c b/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c index 8f82f99f7748..4d5656978746 100644 --- a/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c +++ b/tools/testing/selftests/filesystems/epoll/epoll_wakeup_test.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #define _GNU_SOURCE +#include <asm/unistd.h> #include <poll.h> #include <unistd.h> #include <assert.h> @@ -21,6 +22,18 @@ struct epoll_mtcontext pthread_t waiter; }; +#ifndef __NR_epoll_pwait2 +#define __NR_epoll_pwait2 -1 +#endif + +static inline int sys_epoll_pwait2(int fd, struct epoll_event *events, + int maxevents, + const struct timespec *timeout, + const sigset_t *sigset) +{ + return syscall(__NR_epoll_pwait2, fd, events, maxevents, timeout, sigset); +} + static void signal_handler(int signum) { } @@ -3377,4 +3390,61 @@ TEST(epoll61) close(ctx.evfd); } +/* Equivalent to basic test epoll1, but exercising epoll_pwait2. */ +TEST(epoll62) +{ + int efd; + int sfd[2]; + struct epoll_event e; + + ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, sfd), 0); + + efd = epoll_create(1); + ASSERT_GE(efd, 0); + + e.events = EPOLLIN; + ASSERT_EQ(epoll_ctl(efd, EPOLL_CTL_ADD, sfd[0], &e), 0); + + ASSERT_EQ(write(sfd[1], "w", 1), 1); + + EXPECT_EQ(sys_epoll_pwait2(efd, &e, 1, NULL, NULL), 1); + EXPECT_EQ(sys_epoll_pwait2(efd, &e, 1, NULL, NULL), 1); + + close(efd); + close(sfd[0]); + close(sfd[1]); +} + +/* Epoll_pwait2 basic timeout test. */ +TEST(epoll63) +{ + const int cfg_delay_ms = 10; + unsigned long long tdiff; + struct timespec ts; + int efd; + int sfd[2]; + struct epoll_event e; + + ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM, 0, sfd), 0); + + efd = epoll_create(1); + ASSERT_GE(efd, 0); + + e.events = EPOLLIN; + ASSERT_EQ(epoll_ctl(efd, EPOLL_CTL_ADD, sfd[0], &e), 0); + + ts.tv_sec = 0; + ts.tv_nsec = cfg_delay_ms * 1000 * 1000; + + tdiff = msecs(); + EXPECT_EQ(sys_epoll_pwait2(efd, &e, 1, &ts, NULL), 0); + tdiff = msecs() - tdiff; + + EXPECT_GE(tdiff, cfg_delay_ms); + + close(efd); + close(sfd[0]); + close(sfd[1]); +} + TEST_HARNESS_MAIN -- 2.29.2.454.gaff20da3a2-goog ^ permalink raw reply related [flat|nested] 25+ messages in thread
end of thread, other threads:[~2021-01-11 20:07 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-18 14:46 [PATCH v3 0/2] add epoll_pwait2 syscall Willem de Bruijn 2020-11-18 14:46 ` [PATCH v3 1/2] epoll: add nsec timeout support with epoll_pwait2 Willem de Bruijn 2020-11-18 15:00 ` Matthew Wilcox 2020-11-18 15:10 ` Willem de Bruijn 2020-11-18 15:37 ` Arnd Bergmann 2020-11-18 15:59 ` David Laight 2020-11-19 14:19 ` Willem de Bruijn 2020-11-19 14:31 ` Matthew Wilcox 2020-11-19 15:37 ` Willem de Bruijn 2020-11-19 15:45 ` Arnd Bergmann 2020-11-19 20:13 ` Willem de Bruijn 2020-11-20 8:13 ` Arnd Bergmann 2020-11-20 16:01 ` Willem de Bruijn 2020-11-20 19:23 ` Arnd Bergmann 2020-11-20 22:28 ` Willem de Bruijn 2020-11-21 9:27 ` Arnd Bergmann 2020-12-10 17:33 ` Willem de Bruijn 2020-12-10 20:34 ` Arnd Bergmann 2020-12-10 22:59 ` Willem de Bruijn 2021-01-11 20:06 ` Willem de Bruijn 2020-11-18 16:21 ` Willem de Bruijn 2020-11-18 16:50 ` Arnd Bergmann 2020-11-19 3:22 ` Willem de Bruijn 2020-11-18 14:46 ` [PATCH manpages RFC] epoll_wait.2: add epoll_pwait2 Willem de Bruijn 2020-11-18 14:46 ` [PATCH v3 2/2] selftests/filesystems: expand epoll with epoll_pwait2 Willem de Bruijn
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.