* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-12 17:33 UTC (permalink / raw)
To: Alan Jenkins, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <6c7ea7da-ab56-f1b8-2399-c0579b4eceec@gmail.com>
On 2/12/19 10:21 AM, Alan Jenkins wrote:
> On 12/02/2019 15:17, Jens Axboe wrote:
>> On 2/12/19 5:29 AM, Alan Jenkins wrote:
>>> On 08/02/2019 15:13, Jens Axboe wrote:
>>>> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>>>>> On 08/02/2019 12:57, Jens Axboe wrote:
>>>>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>>>>> +{
>>>>>>>> +#if defined(CONFIG_NET)
>>>>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>>>>> + struct sk_buff *skb;
>>>>>>>> + int i;
>>>>>>>> +
>>>>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>>>>> + if (!skb)
>>>>>>>> + return -ENOMEM;
>>>>>>>> +
>>>>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>>>>> + skb->destructor = unix_destruct_scm;
>>>>>>>> +
>>>>>>>> + fpl->user = get_uid(ctx->user);
>>>>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>>>>> + get_file(fpl->fp[i]);
>>>>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>>>>> + fput(fpl->fp[i]);
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + UNIXCB(skb).fp = fpl;
>>>>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>>>>
>>>>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>>>>> cause a reference cycle. So there is a garbage collector for unix
>>>>>>> sockets, which we hook into here. */
>>>>>> Yes that's a good idea, I've added a comment as to why we go through the
>>>>>> trouble of doing this socket + skb dance.
>>>>> Great, thanks.
>>>>>
>>>>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>>>>> was intended to bound kernel memory allocation, at least in principle.
>>>>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>>>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>>>>> 1024 on the number of registered files. This is important to cap the
>>>>>> memory allocation attempt as well.
>>>>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>>>>> there's no specific limit on the number of io_urings you can open (only
>>>>> the standard limits on fds). So this would let you allocate hundreds of
>>>>> times more files than the previous limit RLIMIT_NOFILE...
>>>> But there is, the io_uring itself is under the memlock rlimit.
>>>>
>>>>> static inline bool too_many_unix_fds(struct task_struct *p)
>>>>> {
>>>>> struct user_struct *user = current_user();
>>>>>
>>>>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>>>>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>>>>> return false;
>>>>> }
>>>>>
>>>>> RLIMIT_NOFILE is technically per-task, but here it is capping
>>>>> unix_inflight per-user. So the way I look at this, the number of file
>>>>> descriptors per user is bounded by NOFILE * NPROC. Then
>>>>> user->unix_inflight can have one additional process' worth (NOFILE) of
>>>>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>>>>> called once per SCM_RIGHTS).
>>>>>
>>>>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>>>>> you have about 253 (or 1024) more process' worth of open files. That
>>>>> could be big proportionally when RLIMIT_NPROC is low.
>>>>>
>>>>> I don't know if it matters. It maybe reads like an oversight though.
>>>>>
>>>>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>>>>> to get rid of the "slop". Since that may be different between af_unix
>>>>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>>>>> number of inflight files we want to add.)
>>>> I don't think it matters. The files in the fixed file set have already
>>>> been opened by the application, so it counts towards the number of open
>>>> files that is allowed to have. I don't think we should impose further
>>>> limits on top of that.
>>> A process can open one io_uring and 199 other files. Register the 199
>>> files in the io_uring, then close their file descriptors. The main
>>> NOFILE limit only counts file descriptors. So then you can open one
>>> io_uring, 198 other files, and repeat.
>>>
>>> You're right, I had forgotten the memlock limit on io_uring. That makes
>>> it much less of a practical problem.
>>>
>>> But it raises a second point. It's not just that it lets users allocate
>>> more files. You might not want to be limited by user->unix_inflight.
>>> But you are calling unix_inflight(), which increments it! Then if
>>> unix->inflight exceeds the NOFILE limit, you will avoid seeing any
>>> errors with io_uring, but the user will not be able to send files over
>>> unix sockets.
>>>
>>> So I think this is confusing to read, and confusing to troubleshoot if
>>> the limit is ever hit.
>>>
>>> I would be happy if io_uring didn't increment user->unix_inflight. I'm
>>> not sure what the best way is to arrange that.
>> How about we just do something like the below? I think that's the saner
>> approach, rather than bypass user->unix_inflight. It's literally the
>> same thing.
>>
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index a4973af1c272..5196b3aa935e 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -2041,6 +2041,13 @@ static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
>> struct sk_buff *skb;
>> int i;
>>
>> + if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
>> + struct user_struct *user = ctx->user;
>> +
>> + if (user->unix_inflight > task_rlimit(current, RLIMIT_NOFILE))
>> + return -EMFILE;
>> + }
>> +
>> fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
>> if (!fpl)
>> return -ENOMEM;
>>
>>
>
> Welp, you gave me exactly what I asked for. So now I'd better be
> positive about it :-D.
;-)
> I hope this will be documented accurately, at least where the EMFILE
> result is explained for this syscall.
How's this:
http://git.kernel.dk/cgit/liburing/commit/?id=37e48698a09aa1e37690f8fa6dfd8da69a48ee60
> Because EMFILE is different from the errno in af_unix.c, I will add a
> wish for the existing documentation of ETOOMANYREFS in unix(7) to
> reference this.
>
> I'll stop bikeshedding there. EMFILE sounds ok. strerror() calls
> ETOOMANYREFS "Too many references: cannot splice"; it doesn't seem to be
> particularly helpful or well-known.
Agree
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* [PATCH 1/4] glibc: Perform rseq(2) registration at C startup and thread creation (v7)
From: Mathieu Desnoyers @ 2019-02-12 19:42 UTC (permalink / raw)
To: Carlos O'Donell
Cc: Florian Weimer, Joseph Myers, Szabolcs Nagy, libc-alpha,
Mathieu Desnoyers, Thomas Gleixner, Ben Maurer, Peter Zijlstra,
Paul E. McKenney, Boqun Feng, Will Deacon, Dave Watson,
Paul Turner, Rich Felker, linux-kernel, linux-api
In-Reply-To: <20190212194253.1951-1-mathieu.desnoyers@efficios.com>
Register rseq(2) TLS for each thread (including main), and unregister
for each thread (excluding main). "rseq" stands for Restartable
Sequences.
See the rseq(2) man page proposed here:
https://lkml.org/lkml/2018/9/19/647
This patch is based on glibc-2.29. The rseq(2) system call was merged
into Linux 4.18.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Paul Turner <pjt@google.com>
CC: Rich Felker <dalias@libc.org>
CC: libc-alpha@sourceware.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
Changes since v1:
- Move __rseq_refcount to an extra field at the end of __rseq_abi to
eliminate one symbol.
All libraries/programs which try to register rseq (glibc,
early-adopter applications, early-adopter libraries) should use the
rseq refcount. It becomes part of the ABI within a user-space
process, but it's not part of the ABI shared with the kernel per se.
- Restructure how this code is organized so glibc keeps building on
non-Linux targets.
- Use non-weak symbol for __rseq_abi.
- Move rseq registration/unregistration implementation into its own
nptl/rseq.c compile unit.
- Move __rseq_abi symbol under GLIBC_2.29.
Changes since v2:
- Move __rseq_refcount to its own symbol, which is less ugly than
trying to play tricks with the rseq uapi.
- Move __rseq_abi from nptl to csu (C start up), so it can be used
across glibc, including memory allocator and sched_getcpu(). The
__rseq_refcount symbol is kept in nptl, because there is no reason
to use it elsewhere in glibc.
Changes since v3:
- Set __rseq_refcount TLS to 1 on register/set to 0 on unregister
because glibc is the first/last user.
- Unconditionally register/unregister rseq at thread start/exit, because
glibc is the first/last user.
- Add missing abilist items.
- Rebase on glibc master commit a502c5294.
- Add NEWS entry.
Changes since v4:
- Do not use "weak" symbols for __rseq_abi and __rseq_refcount. Based on
"System V Application Binary Interface", weak only affects the link
editor, not the dynamic linker.
- Install a new sys/rseq.h system header on Linux, which contains the
RSEQ_SIG definition, __rseq_abi declaration and __rseq_refcount
declaration. Move those definition/declarations from rseq-internal.h
to the installed sys/rseq.h header.
- Considering that rseq is only available on Linux, move csu/rseq.c to
sysdeps/unix/sysv/linux/rseq-sym.c.
- Move __rseq_refcount from nptl/rseq.c to
sysdeps/unix/sysv/linux/rseq-sym.c, so it is only defined on Linux.
- Move both ABI definitions for __rseq_abi and __rseq_refcount to
sysdeps/unix/sysv/linux/Versions, so they only appear on Linux.
- Document __rseq_abi and __rseq_refcount volatile.
- Document the RSEQ_SIG signature define.
- Move registration functions from rseq.c to rseq-internal.h static
inline functions. Introduce empty stubs in misc/rseq-internal.h,
which can be overridden by architecture code in
sysdeps/unix/sysv/linux/rseq-internal.h.
- Rename __rseq_register_current_thread and __rseq_unregister_current_thread
to rseq_register_current_thread and rseq_unregister_current_thread,
now that those are only visible as internal static inline functions.
- Invoke rseq_register_current_thread() from libc-start.c LIBC_START_MAIN
rather than nptl init, so applications not linked against
libpthread.so have rseq registered for their main() thread. Note that
it is invoked separately for SHARED and !SHARED builds.
Changes since v5:
- Replace __rseq_refcount by __rseq_lib_abi, which contains two
uint32_t: register_state and refcount. The "register_state" field
allows inhibiting rseq registration from signal handlers nested on top
of glibc registration and occuring after rseq unregistration by glibc.
- Introduce enum rseq_register_state, which contains the states allowed
for the struct rseq_lib_abi register_state field.
Changes since v6:
- Introduce bits/rseq.h to define RSEQ_SIG for each architecture.
The generic bits/rseq.h does not define RSEQ_SIG, meaning that each
architecture implementing rseq needs to implement bits/rseq.h.
- Rename enum item RSEQ_REGISTER_NESTED to RSEQ_REGISTER_ONGOING.
- Port to glibc-2.29.
---
NEWS | 11 +++
csu/libc-start.c | 12 ++-
misc/Makefile | 3 +-
misc/rseq-internal.h | 34 +++++++
nptl/pthread_create.c | 9 ++
sysdeps/unix/sysv/linux/Makefile | 4 +-
sysdeps/unix/sysv/linux/Versions | 4 +
sysdeps/unix/sysv/linux/aarch64/bits/rseq.h | 24 +++++
sysdeps/unix/sysv/linux/aarch64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/alpha/libc.abilist | 2 +
sysdeps/unix/sysv/linux/arm/bits/rseq.h | 24 +++++
sysdeps/unix/sysv/linux/arm/libc.abilist | 2 +
sysdeps/unix/sysv/linux/bits/rseq.h | 24 +++++
sysdeps/unix/sysv/linux/hppa/libc.abilist | 2 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 2 +
sysdeps/unix/sysv/linux/ia64/libc.abilist | 2 +
.../sysv/linux/m68k/coldfire/libc.abilist | 2 +
.../unix/sysv/linux/m68k/m680x0/libc.abilist | 2 +
.../unix/sysv/linux/microblaze/libc.abilist | 2 +
sysdeps/unix/sysv/linux/mips/bits/rseq.h | 24 +++++
.../sysv/linux/mips/mips32/fpu/libc.abilist | 2 +
.../sysv/linux/mips/mips32/nofpu/libc.abilist | 2 +
.../sysv/linux/mips/mips64/n32/libc.abilist | 2 +
.../sysv/linux/mips/mips64/n64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/nios2/libc.abilist | 2 +
sysdeps/unix/sysv/linux/powerpc/bits/rseq.h | 24 +++++
.../linux/powerpc/powerpc32/fpu/libc.abilist | 2 +
.../powerpc/powerpc32/nofpu/libc.abilist | 2 +
.../linux/powerpc/powerpc64/be/libc.abilist | 2 +
.../linux/powerpc/powerpc64/le/libc.abilist | 2 +
.../unix/sysv/linux/riscv/rv64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/rseq-internal.h | 91 +++++++++++++++++++
sysdeps/unix/sysv/linux/rseq-sym.c | 54 +++++++++++
sysdeps/unix/sysv/linux/s390/bits/rseq.h | 24 +++++
.../unix/sysv/linux/s390/s390-32/libc.abilist | 2 +
.../unix/sysv/linux/s390/s390-64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sh/libc.abilist | 2 +
.../sysv/linux/sparc/sparc32/libc.abilist | 2 +
.../sysv/linux/sparc/sparc64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sys/rseq.h | 65 +++++++++++++
sysdeps/unix/sysv/linux/x86/bits/rseq.h | 24 +++++
.../unix/sysv/linux/x86_64/64/libc.abilist | 2 +
.../unix/sysv/linux/x86_64/x32/libc.abilist | 2 +
43 files changed, 501 insertions(+), 6 deletions(-)
create mode 100644 misc/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/arm/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/mips/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/rseq-internal.h
create mode 100644 sysdeps/unix/sysv/linux/rseq-sym.c
create mode 100644 sysdeps/unix/sysv/linux/s390/bits/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/sys/rseq.h
create mode 100644 sysdeps/unix/sysv/linux/x86/bits/rseq.h
diff --git a/NEWS b/NEWS
index 912a9bdc0f..0608c60f7d 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,17 @@ See the end for copying conditions.
Please send GNU C library bug reports via <https://sourceware.org/bugzilla/>
using `glibc' in the "product" field.
\f
+Version 2.30
+
+Major new features:
+
+* Support for automatically registering threads with the Linux rseq(2)
+ system call has been added. This system call is implemented starting
+ from Linux 4.18. In order to be activated, it requires that glibc is built
+ against kernel headers that include this system call, and that glibc
+ detects availability of that system call at runtime.
+
+\f
Version 2.29
Major new features:
diff --git a/csu/libc-start.c b/csu/libc-start.c
index 5d9c3675fa..8680afc0ef 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -22,6 +22,7 @@
#include <ldsodefs.h>
#include <exit-thread.h>
#include <libc-internal.h>
+#include <rseq-internal.h>
#include <elf/dl-tunables.h>
@@ -140,7 +141,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
__libc_multiple_libcs = &_dl_starting_up && !_dl_starting_up;
-#ifndef SHARED
+#ifdef SHARED
+ /* Register rseq ABI to the kernel. */
+ (void) rseq_register_current_thread ();
+#else
_dl_relocate_static_pie ();
char **ev = &argv[argc + 1];
@@ -218,6 +222,9 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
}
# endif
+ /* Register rseq ABI to the kernel. */
+ (void) rseq_register_current_thread ();
+
/* Initialize libpthread if linked in. */
if (__pthread_initialize_minimal != NULL)
__pthread_initialize_minimal ();
@@ -230,8 +237,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
# else
__pointer_chk_guard_local = pointer_chk_guard;
# endif
-
-#endif /* !SHARED */
+#endif
/* Register the destructor of the dynamic linker if there is any. */
if (__glibc_likely (rtld_fini != NULL))
diff --git a/misc/Makefile b/misc/Makefile
index cf0daa1161..0ae1dbaf80 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -36,7 +36,8 @@ headers := sys/uio.h bits/uio-ext.h bits/uio_lim.h \
syslog.h sys/syslog.h \
bits/syslog.h bits/syslog-ldbl.h bits/syslog-path.h bits/error.h \
bits/select2.h bits/hwcap.h sys/auxv.h \
- sys/sysmacros.h bits/sysmacros.h bits/types/struct_iovec.h
+ sys/sysmacros.h bits/sysmacros.h bits/types/struct_iovec.h \
+ rseq-internal.h
routines := brk sbrk sstk ioctl \
readv writev preadv preadv64 pwritev pwritev64 \
diff --git a/misc/rseq-internal.h b/misc/rseq-internal.h
new file mode 100644
index 0000000000..915122e4bf
--- /dev/null
+++ b/misc/rseq-internal.h
@@ -0,0 +1,34 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef RSEQ_INTERNAL_H
+#define RSEQ_INTERNAL_H
+
+static inline int
+rseq_register_current_thread (void)
+{
+ return -1;
+}
+
+static inline int
+rseq_unregister_current_thread (void)
+{
+ return -1;
+}
+
+#endif /* rseq-internal.h */
diff --git a/nptl/pthread_create.c b/nptl/pthread_create.c
index 2bd2b10727..90b3419390 100644
--- a/nptl/pthread_create.c
+++ b/nptl/pthread_create.c
@@ -33,6 +33,7 @@
#include <default-sched.h>
#include <futex-internal.h>
#include <tls-setup.h>
+#include <rseq-internal.h>
#include "libioP.h"
#include <shlib-compat.h>
@@ -378,6 +379,7 @@ __free_tcb (struct pthread *pd)
START_THREAD_DEFN
{
struct pthread *pd = START_THREAD_SELF;
+ bool has_rseq = false;
#if HP_TIMING_AVAIL
/* Remember the time when the thread was started. */
@@ -396,6 +398,9 @@ START_THREAD_DEFN
if (__glibc_unlikely (atomic_exchange_acq (&pd->setxid_futex, 0) == -2))
futex_wake (&pd->setxid_futex, 1, FUTEX_PRIVATE);
+ /* Register rseq TLS to the kernel. */
+ has_rseq = !rseq_register_current_thread ();
+
#ifdef __NR_set_robust_list
# ifndef __ASSUME_SET_ROBUST_LIST
if (__set_robust_list_avail >= 0)
@@ -573,6 +578,10 @@ START_THREAD_DEFN
}
#endif
+ /* Unregister rseq TLS from kernel. */
+ if (has_rseq && rseq_unregister_current_thread ())
+ abort();
+
advise_stack_range (pd->stackblock, pd->stackblock_size, (uintptr_t) pd,
pd->guardsize);
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 5f8c2c7c7d..5b541469ec 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -1,5 +1,5 @@
ifeq ($(subdir),csu)
-sysdep_routines += errno-loc
+sysdep_routines += errno-loc rseq-sym
endif
ifeq ($(subdir),assert)
@@ -48,7 +48,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
bits/termios-c_iflag.h bits/termios-c_oflag.h \
bits/termios-baud.h bits/termios-c_cflag.h \
bits/termios-c_lflag.h bits/termios-tcflow.h \
- bits/termios-misc.h
+ bits/termios-misc.h sys/rseq.h bits/rseq.h
tests += tst-clone tst-clone2 tst-clone3 tst-fanotify tst-personality \
tst-quota tst-sync_file_range tst-sysconf-iov_max tst-ttyname \
diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
index f1e12d9c69..ad88c2b7ff 100644
--- a/sysdeps/unix/sysv/linux/Versions
+++ b/sysdeps/unix/sysv/linux/Versions
@@ -174,6 +174,10 @@ libc {
GLIBC_2.29 {
getcpu;
}
+ GLIBC_2.30 {
+ __rseq_abi;
+ __rseq_lib_abi;
+ }
GLIBC_PRIVATE {
# functions used in other libraries
__syscall_rt_sigqueueinfo;
diff --git a/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
new file mode 100644
index 0000000000..543bc5388a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/aarch64/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Signature required before each abort handler code. */
+#define RSEQ_SIG 0xd428bc00 /* BRK #0x45E0. */
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
index 9c330f325e..bc937f585d 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
@@ -2141,3 +2141,5 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
index f630fa4c6f..89cc8b1cfb 100644
--- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
+++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
@@ -2036,6 +2036,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/arm/bits/rseq.h b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
new file mode 100644
index 0000000000..19d3755837
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Signature required before each abort handler code. */
+#define RSEQ_SIG 0x53053053
diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist
index b96f45590f..e5055f2d4e 100644
--- a/sysdeps/unix/sysv/linux/arm/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/libc.abilist
@@ -126,6 +126,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
GLIBC_2.4 _IO_2_1_stdin_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/bits/rseq.h b/sysdeps/unix/sysv/linux/bits/rseq.h
new file mode 100644
index 0000000000..d60f02cfeb
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Each architecture supporting rseq should define RSEQ_SIG as a 32-bit
+ signature inserted before each rseq abort label in the code section. */
diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
index 088a8ee369..546d073cdb 100644
--- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
+++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
@@ -1883,6 +1883,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index f7ff2c57b9..ac1de6e4b3 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2048,6 +2048,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
index becd8b1033..cc3445b958 100644
--- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
@@ -1917,6 +1917,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
index 74e42a5209..f7e28bd5a0 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
@@ -127,6 +127,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0x98
GLIBC_2.4 _IO_2_1_stdin_ D 0x98
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
index 4af5a74e8a..b8f00f6111 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
@@ -1992,6 +1992,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/microblaze/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/libc.abilist
index ccef673fd2..19f191434f 100644
--- a/sysdeps/unix/sysv/linux/microblaze/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/libc.abilist
@@ -2133,3 +2133,5 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
diff --git a/sysdeps/unix/sysv/linux/mips/bits/rseq.h b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
new file mode 100644
index 0000000000..19d3755837
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Signature required before each abort handler code. */
+#define RSEQ_SIG 0x53053053
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
index 1054bb599e..fe43507f55 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
@@ -1970,6 +1970,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
index 4f5b5ffebf..b247c6ea9b 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
@@ -1968,6 +1968,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
index 943aee58d4..5339ca52b6 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
@@ -1976,6 +1976,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
index 17a5d17ef9..11f24eb440 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
@@ -1971,6 +1971,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
index 4d62a540fd..fd223bfc44 100644
--- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
+++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
@@ -2174,3 +2174,5 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
diff --git a/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
new file mode 100644
index 0000000000..19d3755837
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/powerpc/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Signature required before each abort handler code. */
+#define RSEQ_SIG 0x53053053
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
index ecc2d6fa13..cc53178e81 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
@@ -1996,6 +1996,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
index f5830f9c33..2de3134bc7 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
@@ -2000,6 +2000,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
index 633d8f4792..aae3def700 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/be/libc.abilist
@@ -126,6 +126,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 _Exit F
GLIBC_2.3 _IO_2_1_stderr_ D 0xe0
GLIBC_2.3 _IO_2_1_stdin_ D 0xe0
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
index 2c712636ef..8d582a3a9b 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/le/libc.abilist
@@ -2231,3 +2231,5 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
diff --git a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
index 195bc8b2cf..155953f6cf 100644
--- a/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/riscv/rv64/libc.abilist
@@ -2103,3 +2103,5 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
diff --git a/sysdeps/unix/sysv/linux/rseq-internal.h b/sysdeps/unix/sysv/linux/rseq-internal.h
new file mode 100644
index 0000000000..d676da3701
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/rseq-internal.h
@@ -0,0 +1,91 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef RSEQ_INTERNAL_H
+#define RSEQ_INTERNAL_H
+
+#include <sysdep.h>
+
+#ifdef __NR_rseq
+
+#include <errno.h>
+#include <sys/rseq.h>
+
+static inline int
+rseq_register_current_thread (void)
+{
+ int rc, ret = 0;
+ INTERNAL_SYSCALL_DECL (err);
+
+ if (__rseq_abi.cpu_id == RSEQ_CPU_ID_REGISTRATION_FAILED)
+ return -1;
+ /* Temporarily prevent nested signal handlers from registering rseq. */
+ __rseq_lib_abi.register_state = RSEQ_REGISTER_ONGOING;
+ if (__rseq_lib_abi.refcount == UINT_MAX)
+ {
+ ret = -1;
+ goto end;
+ }
+ if (__rseq_lib_abi.refcount++)
+ goto end;
+ rc = INTERNAL_SYSCALL_CALL (rseq, err, &__rseq_abi, sizeof (struct rseq),
+ 0, RSEQ_SIG);
+ if (!rc)
+ goto end;
+ if (INTERNAL_SYSCALL_ERRNO (rc, err) != EBUSY)
+ __rseq_abi.cpu_id = RSEQ_CPU_ID_REGISTRATION_FAILED;
+ ret = -1;
+end:
+ __rseq_lib_abi.register_state = RSEQ_REGISTER_ALLOWED;
+ return ret;
+}
+
+static inline int
+rseq_unregister_current_thread (void)
+{
+ int rc, ret = 0;
+ INTERNAL_SYSCALL_DECL (err);
+
+ /* Setting __rseq_register_state = RSEQ_REGISTER_EXITING for the rest of the
+ thread lifetime. Ensures signal handlers nesting just before thread exit
+ don't try to register rseq. */
+ __rseq_lib_abi.register_state = RSEQ_REGISTER_EXITING;
+ __rseq_lib_abi.refcount = 0;
+ rc = INTERNAL_SYSCALL_CALL (rseq, err, &__rseq_abi, sizeof (struct rseq),
+ RSEQ_FLAG_UNREGISTER, RSEQ_SIG);
+ if (!rc)
+ goto end;
+ ret = -1;
+end:
+ return ret;
+}
+#else
+static inline int
+rseq_register_current_thread (void)
+{
+ return -1;
+}
+
+static inline int
+rseq_unregister_current_thread (void)
+{
+ return -1;
+}
+#endif
+
+#endif /* rseq-internal.h */
diff --git a/sysdeps/unix/sysv/linux/rseq-sym.c b/sysdeps/unix/sysv/linux/rseq-sym.c
new file mode 100644
index 0000000000..99b277e9d6
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/rseq-sym.c
@@ -0,0 +1,54 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2018.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <sys/syscall.h>
+#include <stdint.h>
+
+#ifdef __NR_rseq
+#include <sys/rseq.h>
+#else
+
+enum rseq_cpu_id_state {
+ RSEQ_CPU_ID_UNINITIALIZED = -1,
+ RSEQ_CPU_ID_REGISTRATION_FAILED = -2,
+};
+
+/* linux/rseq.h defines struct rseq as aligned on 32 bytes. The kernel ABI
+ size is 20 bytes. */
+struct rseq {
+ uint32_t cpu_id_start;
+ uint32_t cpu_id;
+ uint64_t rseq_cs;
+ uint32_t flags;
+} __attribute__ ((aligned(4 * sizeof(uint64_t))));
+
+struct rseq_lib_abi
+{
+ uint32_t register_state;
+ uint32_t refcount;
+};
+
+#endif
+
+/* volatile because fields can be read/updated by the kernel. */
+__thread volatile struct rseq __rseq_abi = {
+ .cpu_id = RSEQ_CPU_ID_UNINITIALIZED,
+};
+
+/* volatile because fields can be read/updated by signal handlers. */
+__thread volatile struct rseq_lib_abi __rseq_lib_abi;
diff --git a/sysdeps/unix/sysv/linux/s390/bits/rseq.h b/sysdeps/unix/sysv/linux/s390/bits/rseq.h
new file mode 100644
index 0000000000..19d3755837
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Signature required before each abort handler code. */
+#define RSEQ_SIG 0x53053053
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
index 334def033c..42316d8666 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
@@ -2005,6 +2005,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
index 536f4c4ced..c6c4e55a77 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
@@ -1911,6 +1911,8 @@ GLIBC_2.29 __fentry__ F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/sh/libc.abilist b/sysdeps/unix/sysv/linux/sh/libc.abilist
index 30ae3b6ebb..8652dfea59 100644
--- a/sysdeps/unix/sysv/linux/sh/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/libc.abilist
@@ -1887,6 +1887,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
index 68b107d080..95b58dfa67 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
@@ -1999,6 +1999,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
index e5b6a4da50..bfd24f9d1c 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
@@ -1940,6 +1940,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/sys/rseq.h b/sysdeps/unix/sysv/linux/sys/rseq.h
new file mode 100644
index 0000000000..83c8976f50
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/sys/rseq.h
@@ -0,0 +1,65 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+#define _SYS_RSEQ_H 1
+
+/* We use the structures declarations from the kernel headers. */
+#include <linux/rseq.h>
+/* Architecture-specific rseq signature. */
+#include <bits/rseq.h>
+#include <stdint.h>
+
+enum rseq_register_state
+{
+ /* Value RSEQ_REGISTER_ALLOWED means it is allowed to update
+ the refcount field and to register/unregister rseq. */
+ RSEQ_REGISTER_ALLOWED = 0,
+ /* Value RSEQ_REGISTER_ONGOING means a rseq registration is in progress,
+ so it is temporarily forbidden to update the refcount field or to
+ register/unregister rseq for this thread or signal handlers nested
+ on this thread. */
+ RSEQ_REGISTER_ONGOING = 1,
+ /* Value RSEQ_REGISTER_EXITING means it is forbidden to update the
+ refcount field or to register/unregister rseq for the rest of the
+ thread's lifetime. */
+ RSEQ_REGISTER_EXITING = 2,
+};
+
+struct rseq_lib_abi
+{
+ uint32_t register_state; /* enum rseq_register_state. */
+ /* The refcount field keeps track of rseq users, so early adopters
+ of rseq can cooperate amongst each other and with glibc to
+ share rseq thread registration. The refcount field can only be
+ updated when allowed by the value of field register_state.
+ Registering rseq should be performed when incrementing refcount
+ from 0 to 1, and unregistering rseq should be performed when
+ decrementing refcount from 1 to 0. */
+ uint32_t refcount;
+};
+
+/* volatile because fields can be read/updated by the kernel. */
+extern __thread volatile struct rseq __rseq_abi
+__attribute__ ((tls_model ("initial-exec")));
+
+/* volatile because fields can be read/updated by signal handlers. */
+extern __thread volatile struct rseq_lib_abi __rseq_lib_abi
+__attribute__ ((tls_model ("initial-exec")));
+
+#endif /* sys/rseq.h */
diff --git a/sysdeps/unix/sysv/linux/x86/bits/rseq.h b/sysdeps/unix/sysv/linux/x86/bits/rseq.h
new file mode 100644
index 0000000000..19d3755837
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/bits/rseq.h
@@ -0,0 +1,24 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+ Contributed by Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, 2019.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _SYS_RSEQ_H
+# error "Never use <bits/rseq.h> directly; include <sys/rseq.h> instead."
+#endif
+
+/* Signature required before each abort handler code. */
+#define RSEQ_SIG 0x53053053
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index 86dfb0c94d..e9f8411fb2 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -1898,6 +1898,8 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
GLIBC_2.3 __ctype_toupper_loc F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index dd688263aa..f9432d07f1 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2149,3 +2149,5 @@ GLIBC_2.28 thrd_yield F
GLIBC_2.29 getcpu F
GLIBC_2.29 posix_spawn_file_actions_addchdir_np F
GLIBC_2.29 posix_spawn_file_actions_addfchdir_np F
+GLIBC_2.30 __rseq_abi T 0x20
+GLIBC_2.30 __rseq_lib_abi T 0x8
--
2.17.1
^ permalink raw reply related
* [PATCH 2/4] glibc: sched_getcpu(): use rseq cpu_id TLS on Linux
From: Mathieu Desnoyers @ 2019-02-12 19:42 UTC (permalink / raw)
To: Carlos O'Donell
Cc: Florian Weimer, Joseph Myers, Szabolcs Nagy, libc-alpha,
Mathieu Desnoyers, Thomas Gleixner, Ben Maurer, Peter Zijlstra,
Paul E. McKenney, Boqun Feng, Will Deacon, Dave Watson,
Paul Turner, linux-kernel, linux-api
In-Reply-To: <20190212194253.1951-1-mathieu.desnoyers@efficios.com>
When available, use the cpu_id field from __rseq_abi on Linux to
implement sched_getcpu(). Fall-back on the vgetcpu vDSO if unavailable.
Benchmarks:
x86-64: Intel E5-2630 v3@2.40GHz, 16-core, hyperthreading
glibc sched_getcpu(): 13.7 ns (baseline)
glibc sched_getcpu() using rseq: 2.5 ns (speedup: 5.5x)
inline load cpuid from __rseq_abi TLS: 0.8 ns (speedup: 17.1x)
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Carlos O'Donell <carlos@redhat.com>
CC: Florian Weimer <fweimer@redhat.com>
CC: Joseph Myers <joseph@codesourcery.com>
CC: Szabolcs Nagy <szabolcs.nagy@arm.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ben Maurer <bmaurer@fb.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Paul Turner <pjt@google.com>
CC: libc-alpha@sourceware.org
CC: linux-kernel@vger.kernel.org
CC: linux-api@vger.kernel.org
---
sysdeps/unix/sysv/linux/sched_getcpu.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
index fb0d317f83..8bfb03778b 100644
--- a/sysdeps/unix/sysv/linux/sched_getcpu.c
+++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
@@ -24,8 +24,8 @@
#endif
#include <sysdep-vdso.h>
-int
-sched_getcpu (void)
+static int
+vsyscall_sched_getcpu (void)
{
#ifdef __NR_getcpu
unsigned int cpu;
@@ -37,3 +37,24 @@ sched_getcpu (void)
return -1;
#endif
}
+
+#ifdef __NR_rseq
+#include <linux/rseq.h>
+
+extern __attribute__ ((tls_model ("initial-exec")))
+__thread volatile struct rseq __rseq_abi;
+
+int
+sched_getcpu (void)
+{
+ int cpu_id = __rseq_abi.cpu_id;
+
+ return cpu_id >= 0 ? cpu_id : vsyscall_sched_getcpu ();
+}
+#else
+int
+sched_getcpu (void)
+{
+ return vsyscall_sched_getcpu ();
+}
+#endif
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 13/18] io_uring: add file set registration
From: Alan Jenkins @ 2019-02-12 20:23 UTC (permalink / raw)
To: Jens Axboe, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <a326cb39-d55d-8047-a916-1562b33af76a@kernel.dk>
On 12/02/2019 17:33, Jens Axboe wrote:
> On 2/12/19 10:21 AM, Alan Jenkins wrote:
>> On 12/02/2019 15:17, Jens Axboe wrote:
>>> On 2/12/19 5:29 AM, Alan Jenkins wrote:
>>>> On 08/02/2019 15:13, Jens Axboe wrote:
>>>>> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>>>>>> On 08/02/2019 12:57, Jens Axboe wrote:
>>>>>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>>>>>> +{
>>>>>>>>> +#if defined(CONFIG_NET)
>>>>>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>>>>>> + struct sk_buff *skb;
>>>>>>>>> + int i;
>>>>>>>>> +
>>>>>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>>>>>> + if (!skb)
>>>>>>>>> + return -ENOMEM;
>>>>>>>>> +
>>>>>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>>>>>> + skb->destructor = unix_destruct_scm;
>>>>>>>>> +
>>>>>>>>> + fpl->user = get_uid(ctx->user);
>>>>>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>>>>>> + get_file(fpl->fp[i]);
>>>>>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>>>>>> + fput(fpl->fp[i]);
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> + UNIXCB(skb).fp = fpl;
>>>>>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>>>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>>>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>>>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>>>>>
>>>>>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>>>>>> cause a reference cycle. So there is a garbage collector for unix
>>>>>>>> sockets, which we hook into here. */
>>>>>>> Yes that's a good idea, I've added a comment as to why we go through the
>>>>>>> trouble of doing this socket + skb dance.
>>>>>> Great, thanks.
>>>>>>
>>>>>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>>>>>> was intended to bound kernel memory allocation, at least in principle.
>>>>>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>>>>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>>>>>> 1024 on the number of registered files. This is important to cap the
>>>>>>> memory allocation attempt as well.
>>>>>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>>>>>> there's no specific limit on the number of io_urings you can open (only
>>>>>> the standard limits on fds). So this would let you allocate hundreds of
>>>>>> times more files than the previous limit RLIMIT_NOFILE...
>>>>> But there is, the io_uring itself is under the memlock rlimit.
>>>>>
>>>>>> static inline bool too_many_unix_fds(struct task_struct *p)
>>>>>> {
>>>>>> struct user_struct *user = current_user();
>>>>>>
>>>>>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>>>>>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>>>>>> return false;
>>>>>> }
>>>>>>
>>>>>> RLIMIT_NOFILE is technically per-task, but here it is capping
>>>>>> unix_inflight per-user. So the way I look at this, the number of file
>>>>>> descriptors per user is bounded by NOFILE * NPROC. Then
>>>>>> user->unix_inflight can have one additional process' worth (NOFILE) of
>>>>>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>>>>>> called once per SCM_RIGHTS).
>>>>>>
>>>>>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>>>>>> you have about 253 (or 1024) more process' worth of open files. That
>>>>>> could be big proportionally when RLIMIT_NPROC is low.
>>>>>>
>>>>>> I don't know if it matters. It maybe reads like an oversight though.
>>>>>>
>>>>>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>>>>>> to get rid of the "slop". Since that may be different between af_unix
>>>>>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>>>>>> number of inflight files we want to add.)
>>>>> I don't think it matters. The files in the fixed file set have already
>>>>> been opened by the application, so it counts towards the number of open
>>>>> files that is allowed to have. I don't think we should impose further
>>>>> limits on top of that.
>>>> A process can open one io_uring and 199 other files. Register the 199
>>>> files in the io_uring, then close their file descriptors. The main
>>>> NOFILE limit only counts file descriptors. So then you can open one
>>>> io_uring, 198 other files, and repeat.
>>>>
>>>> You're right, I had forgotten the memlock limit on io_uring. That makes
>>>> it much less of a practical problem.
>>>>
>>>> But it raises a second point. It's not just that it lets users allocate
>>>> more files. You might not want to be limited by user->unix_inflight.
>>>> But you are calling unix_inflight(), which increments it! Then if
>>>> unix->inflight exceeds the NOFILE limit, you will avoid seeing any
>>>> errors with io_uring, but the user will not be able to send files over
>>>> unix sockets.
>>>>
>>>> So I think this is confusing to read, and confusing to troubleshoot if
>>>> the limit is ever hit.
>>>>
>>>> I would be happy if io_uring didn't increment user->unix_inflight. I'm
>>>> not sure what the best way is to arrange that.
>>> How about we just do something like the below? I think that's the saner
>>> approach, rather than bypass user->unix_inflight. It's literally the
>>> same thing.
>>>
>>>
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index a4973af1c272..5196b3aa935e 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -2041,6 +2041,13 @@ static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
>>> struct sk_buff *skb;
>>> int i;
>>>
>>> + if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
>>> + struct user_struct *user = ctx->user;
>>> +
>>> + if (user->unix_inflight > task_rlimit(current, RLIMIT_NOFILE))
>>> + return -EMFILE;
>>> + }
>>> +
>>> fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
>>> if (!fpl)
>>> return -ENOMEM;
>>>
>>>
>> Welp, you gave me exactly what I asked for. So now I'd better be
>> positive about it :-D.
> ;-)
>
>> I hope this will be documented accurately, at least where the EMFILE
>> result is explained for this syscall.
> How's this:
>
> http://git.kernel.dk/cgit/liburing/commit/?id=37e48698a09aa1e37690f8fa6dfd8da69a48ee60
+.B EMFILE
+.BR IORING_REGISTER_FILES
+was specified and adding
+.I nr_args
+file references would exceed the maximum allowed number of files the process
+is allowed to have according to the
+.B
+RLIMIT_NOFILE
+resource limit and the caller does not have
+.B CAP_SYS_RESOURCE
+capability.
+.TP
I was struggling with this. The POSIX part of RLIMIT_NOFILE is applied
per-process. But the part we're talking about here, the Linux-specific
"unix_inflight" resource, is actually accounted per-user. It's like
RLIMIT_NPROC. The value of RLIMIT_NPROC is per-process, but the
resource it limits is counted in user->processes.
This subtlety of the NOFILE limit is not made clear in the text above,
nor in unix(7), nor in getrlimit(2). I would interpret all these docs
as saying this limit is a per-process thing - I think they are misleading.
IORING_MAX_FIXED_FILES is being raised to 1024, which is the same as the
(soft limit) value for RLIMIT_NOFILE which the kernel sets for the init
process. I have an unjustifiable nervousness, that there will be some
`fio` command, or a test written that maxes out IORING_REGISTER_FILES.
When you do that, it will provoke unexpected failures e.g. in GUI apps.
If we can't rule that out, the next best thing is a friendly man page.
Regards
Alan
>> Because EMFILE is different from the errno in af_unix.c, I will add a
>> wish for the existing documentation of ETOOMANYREFS in unix(7) to
>> reference this.
>>
>> I'll stop bikeshedding there. EMFILE sounds ok. strerror() calls
>> ETOOMANYREFS "Too many references: cannot splice"; it doesn't seem to be
>> particularly helpful or well-known.
> Agree
>
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 13/18] io_uring: add file set registration
From: Jens Axboe @ 2019-02-12 21:10 UTC (permalink / raw)
To: Alan Jenkins, linux-aio, linux-block, linux-api
Cc: hch, jmoyer, avi, jannh, viro
In-Reply-To: <b8555602-b2e7-c73b-b9bb-3b5f5569cfc7@gmail.com>
On 2/12/19 1:23 PM, Alan Jenkins wrote:
> On 12/02/2019 17:33, Jens Axboe wrote:
>> On 2/12/19 10:21 AM, Alan Jenkins wrote:
>>> On 12/02/2019 15:17, Jens Axboe wrote:
>>>> On 2/12/19 5:29 AM, Alan Jenkins wrote:
>>>>> On 08/02/2019 15:13, Jens Axboe wrote:
>>>>>> On 2/8/19 7:02 AM, Alan Jenkins wrote:
>>>>>>> On 08/02/2019 12:57, Jens Axboe wrote:
>>>>>>>> On 2/8/19 5:17 AM, Alan Jenkins wrote:
>>>>>>>>>> +static int io_sqe_files_scm(struct io_ring_ctx *ctx)
>>>>>>>>>> +{
>>>>>>>>>> +#if defined(CONFIG_NET)
>>>>>>>>>> + struct scm_fp_list *fpl = ctx->user_files;
>>>>>>>>>> + struct sk_buff *skb;
>>>>>>>>>> + int i;
>>>>>>>>>> +
>>>>>>>>>> + skb = __alloc_skb(0, GFP_KERNEL, 0, NUMA_NO_NODE);
>>>>>>>>>> + if (!skb)
>>>>>>>>>> + return -ENOMEM;
>>>>>>>>>> +
>>>>>>>>>> + skb->sk = ctx->ring_sock->sk;
>>>>>>>>>> + skb->destructor = unix_destruct_scm;
>>>>>>>>>> +
>>>>>>>>>> + fpl->user = get_uid(ctx->user);
>>>>>>>>>> + for (i = 0; i < fpl->count; i++) {
>>>>>>>>>> + get_file(fpl->fp[i]);
>>>>>>>>>> + unix_inflight(fpl->user, fpl->fp[i]);
>>>>>>>>>> + fput(fpl->fp[i]);
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + UNIXCB(skb).fp = fpl;
>>>>>>>>>> + skb_queue_head(&ctx->ring_sock->sk->sk_receive_queue, skb);
>>>>>>>>> This code sounds elegant if you know about the existence of unix_gc(),
>>>>>>>>> but quite mysterious if you don't. (E.g. why "inflight"?) Could we
>>>>>>>>> have a brief comment, to comfort mortal readers on their journey?
>>>>>>>>>
>>>>>>>>> /* A message on a unix socket can hold a reference to a file. This can
>>>>>>>>> cause a reference cycle. So there is a garbage collector for unix
>>>>>>>>> sockets, which we hook into here. */
>>>>>>>> Yes that's a good idea, I've added a comment as to why we go through the
>>>>>>>> trouble of doing this socket + skb dance.
>>>>>>> Great, thanks.
>>>>>>>
>>>>>>>>> I think this is bypassing too_many_unix_fds() though? I understood that
>>>>>>>>> was intended to bound kernel memory allocation, at least in principle.
>>>>>>>> As the code stands above, it'll cap it at 253. I'm just now reworking it
>>>>>>>> to NOT be limited to the SCM max fd count, but still impose a limit of
>>>>>>>> 1024 on the number of registered files. This is important to cap the
>>>>>>>> memory allocation attempt as well.
>>>>>>> I saw you were limiting to SCM_MAX_FD per io_uring. On the other hand,
>>>>>>> there's no specific limit on the number of io_urings you can open (only
>>>>>>> the standard limits on fds). So this would let you allocate hundreds of
>>>>>>> times more files than the previous limit RLIMIT_NOFILE...
>>>>>> But there is, the io_uring itself is under the memlock rlimit.
>>>>>>
>>>>>>> static inline bool too_many_unix_fds(struct task_struct *p)
>>>>>>> {
>>>>>>> struct user_struct *user = current_user();
>>>>>>>
>>>>>>> if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
>>>>>>> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
>>>>>>> return false;
>>>>>>> }
>>>>>>>
>>>>>>> RLIMIT_NOFILE is technically per-task, but here it is capping
>>>>>>> unix_inflight per-user. So the way I look at this, the number of file
>>>>>>> descriptors per user is bounded by NOFILE * NPROC. Then
>>>>>>> user->unix_inflight can have one additional process' worth (NOFILE) of
>>>>>>> "inflight" files. (Plus SCM_MAX_FD slop, because too_many_fds() is only
>>>>>>> called once per SCM_RIGHTS).
>>>>>>>
>>>>>>> Because io_uring doesn't check too_many_unix_fds(), I think it will let
>>>>>>> you have about 253 (or 1024) more process' worth of open files. That
>>>>>>> could be big proportionally when RLIMIT_NPROC is low.
>>>>>>>
>>>>>>> I don't know if it matters. It maybe reads like an oversight though.
>>>>>>>
>>>>>>> (If it does matter, it might be cleanest to change too_many_unix_fds()
>>>>>>> to get rid of the "slop". Since that may be different between af_unix
>>>>>>> and io_uring; 253 v.s. 1024 or whatever. E.g. add a parameter for the
>>>>>>> number of inflight files we want to add.)
>>>>>> I don't think it matters. The files in the fixed file set have already
>>>>>> been opened by the application, so it counts towards the number of open
>>>>>> files that is allowed to have. I don't think we should impose further
>>>>>> limits on top of that.
>>>>> A process can open one io_uring and 199 other files. Register the 199
>>>>> files in the io_uring, then close their file descriptors. The main
>>>>> NOFILE limit only counts file descriptors. So then you can open one
>>>>> io_uring, 198 other files, and repeat.
>>>>>
>>>>> You're right, I had forgotten the memlock limit on io_uring. That makes
>>>>> it much less of a practical problem.
>>>>>
>>>>> But it raises a second point. It's not just that it lets users allocate
>>>>> more files. You might not want to be limited by user->unix_inflight.
>>>>> But you are calling unix_inflight(), which increments it! Then if
>>>>> unix->inflight exceeds the NOFILE limit, you will avoid seeing any
>>>>> errors with io_uring, but the user will not be able to send files over
>>>>> unix sockets.
>>>>>
>>>>> So I think this is confusing to read, and confusing to troubleshoot if
>>>>> the limit is ever hit.
>>>>>
>>>>> I would be happy if io_uring didn't increment user->unix_inflight. I'm
>>>>> not sure what the best way is to arrange that.
>>>> How about we just do something like the below? I think that's the saner
>>>> approach, rather than bypass user->unix_inflight. It's literally the
>>>> same thing.
>>>>
>>>>
>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>> index a4973af1c272..5196b3aa935e 100644
>>>> --- a/fs/io_uring.c
>>>> +++ b/fs/io_uring.c
>>>> @@ -2041,6 +2041,13 @@ static int __io_sqe_files_scm(struct io_ring_ctx *ctx, int nr, int offset)
>>>> struct sk_buff *skb;
>>>> int i;
>>>>
>>>> + if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
>>>> + struct user_struct *user = ctx->user;
>>>> +
>>>> + if (user->unix_inflight > task_rlimit(current, RLIMIT_NOFILE))
>>>> + return -EMFILE;
>>>> + }
>>>> +
>>>> fpl = kzalloc(sizeof(*fpl), GFP_KERNEL);
>>>> if (!fpl)
>>>> return -ENOMEM;
>>>>
>>>>
>>> Welp, you gave me exactly what I asked for. So now I'd better be
>>> positive about it :-D.
>> ;-)
>>
>>> I hope this will be documented accurately, at least where the EMFILE
>>> result is explained for this syscall.
>> How's this:
>>
>> http://git.kernel.dk/cgit/liburing/commit/?id=37e48698a09aa1e37690f8fa6dfd8da69a48ee60
>
> +.B EMFILE
> +.BR IORING_REGISTER_FILES
> +was specified and adding
> +.I nr_args
> +file references would exceed the maximum allowed number of files the process
> +is allowed to have according to the
> +.B
> +RLIMIT_NOFILE
> +resource limit and the caller does not have
> +.B CAP_SYS_RESOURCE
> +capability.
> +.TP
>
> I was struggling with this. The POSIX part of RLIMIT_NOFILE is applied
> per-process. But the part we're talking about here, the Linux-specific
> "unix_inflight" resource, is actually accounted per-user. It's like
> RLIMIT_NPROC. The value of RLIMIT_NPROC is per-process, but the
> resource it limits is counted in user->processes.
>
> This subtlety of the NOFILE limit is not made clear in the text above,
> nor in unix(7), nor in getrlimit(2). I would interpret all these docs
> as saying this limit is a per-process thing - I think they are misleading.
Fair point, I'll add an update to clearly state it's a per process
limit.
> IORING_MAX_FIXED_FILES is being raised to 1024, which is the same as the
> (soft limit) value for RLIMIT_NOFILE which the kernel sets for the init
> process. I have an unjustifiable nervousness, that there will be some
> `fio` command, or a test written that maxes out IORING_REGISTER_FILES.
> When you do that, it will provoke unexpected failures e.g. in GUI apps.
> If we can't rule that out, the next best thing is a friendly man page.
If we apply the limit to sendmsg and friends, it should be applied here
as well.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jann Horn @ 2019-02-12 21:42 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <42eea00c-81fb-2e28-d884-03be5bb229c8@kernel.dk>
On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
> On 2/8/19 3:12 PM, Jann Horn wrote:
> > On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
> >> The submission queue (SQ) and completion queue (CQ) rings are shared
> >> between the application and the kernel. This eliminates the need to
> >> copy data back and forth to submit and complete IO.
> >>
> >> IO submissions use the io_uring_sqe data structure, and completions
> >> are generated in the form of io_uring_cqe data structures. The SQ
> >> ring is an index into the io_uring_sqe array, which makes it possible
> >> to submit a batch of IOs without them being contiguous in the ring.
> >> The CQ ring is always contiguous, as completion events are inherently
> >> unordered, and hence any io_uring_cqe entry can point back to an
> >> arbitrary submission.
> >>
> >> Two new system calls are added for this:
> >>
> >> io_uring_setup(entries, params)
> >> Sets up an io_uring instance for doing async IO. On success,
> >> returns a file descriptor that the application can mmap to
> >> gain access to the SQ ring, CQ ring, and io_uring_sqes.
> >>
> >> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> >> Initiates IO against the rings mapped to this fd, or waits for
> >> them to complete, or both. The behavior is controlled by the
> >> parameters passed in. If 'to_submit' is non-zero, then we'll
> >> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> >> kernel will wait for 'min_complete' events, if they aren't
> >> already available. It's valid to set IORING_ENTER_GETEVENTS
> >> and 'min_complete' == 0 at the same time, this allows the
> >> kernel to return already completed events without waiting
> >> for them. This is useful only for polling, as for IRQ
> >> driven IO, the application can just check the CQ ring
> >> without entering the kernel.
> >>
> >> With this setup, it's possible to do async IO with a single system
> >> call. Future developments will enable polled IO with this interface,
> >> and polled submission as well. The latter will enable an application
> >> to do IO without doing ANY system calls at all.
> >>
> >> For IRQ driven IO, an application only needs to enter the kernel for
> >> completions if it wants to wait for them to occur.
> >>
> >> Each io_uring is backed by a workqueue, to support buffered async IO
> >> as well. We will only punt to an async context if the command would
> >> need to wait for IO on the device side. Any data that can be accessed
> >> directly in the page cache is done inline. This avoids the slowness
> >> issue of usual threadpools, since cached data is accessed as quickly
> >> as a sync interface.
[...]
> >> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
> >> +{
> >> + struct io_kiocb *req;
> >> + ssize_t ret;
> >> +
> >> + /* enforce forwards compatibility on users */
> >> + if (unlikely(s->sqe->flags))
> >> + return -EINVAL;
> >> +
> >> + req = io_get_req(ctx);
> >> + if (unlikely(!req))
> >> + return -EAGAIN;
> >> +
> >> + req->rw.ki_filp = NULL;
> >> +
> >> + ret = __io_submit_sqe(ctx, req, s, true);
> >> + if (ret == -EAGAIN) {
> >> + memcpy(&req->submit, s, sizeof(*s));
> >> + INIT_WORK(&req->work, io_sq_wq_submit_work);
> >> + queue_work(ctx->sqo_wq, &req->work);
> >> + ret = 0;
> >> + }
> >> + if (ret)
> >> + io_free_req(req);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void io_commit_sqring(struct io_ring_ctx *ctx)
> >> +{
> >> + struct io_sq_ring *ring = ctx->sq_ring;
> >> +
> >> + if (ctx->cached_sq_head != ring->r.head) {
> >> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> >> + /* write side barrier of head update, app has read side */
> >> + smp_wmb();
> >
> > Can you elaborate on what this memory barrier is doing? Don't you need
> > some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> > nobody sees the updated head before you're done reading the submission
> > queue entry? Or is that barrier elsewhere?
>
> The matching read barrier is in the application, it must do that before
> reading ->head for the SQ ring.
>
> For the other barrier, since the ring->r.head now has a READ_ONCE(),
> that should be all we need to ensure that loads are done.
READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
ordering with regard to concurrent execution on other cores. They are
only compiler barriers, influencing the order in which the compiler
emits things. (Well, unless you're on alpha, where READ_ONCE() implies
a memory barrier that prevents reordering of dependent reads.)
As far as I can tell, between the READ_ONCE(ring->array[...]) in
io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
no *hardware* memory barrier that prevents reordering against
concurrently running userspace code. As far as I can tell, the
following could happen:
- The kernel reads from ring->array in io_get_sqring(), then updates
the head in io_commit_sqring(). The CPU reorders the memory accesses
such that the write to the head becomes visible before the read from
ring->array has completed.
- Userspace observes the write to the head and reuses the array slots
the kernel has freed with the write, clobbering ring->array before the
kernel reads from ring->array.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 22:03 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez2c=7f34AX_FFKTFFnNqJojULs9GwdqaMv=WO2tYLZE3g@mail.gmail.com>
On 2/12/19 2:42 PM, Jann Horn wrote:
> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>> between the application and the kernel. This eliminates the need to
>>>> copy data back and forth to submit and complete IO.
>>>>
>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>> The CQ ring is always contiguous, as completion events are inherently
>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>> arbitrary submission.
>>>>
>>>> Two new system calls are added for this:
>>>>
>>>> io_uring_setup(entries, params)
>>>> Sets up an io_uring instance for doing async IO. On success,
>>>> returns a file descriptor that the application can mmap to
>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>
>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>> them to complete, or both. The behavior is controlled by the
>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>> kernel will wait for 'min_complete' events, if they aren't
>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>> and 'min_complete' == 0 at the same time, this allows the
>>>> kernel to return already completed events without waiting
>>>> for them. This is useful only for polling, as for IRQ
>>>> driven IO, the application can just check the CQ ring
>>>> without entering the kernel.
>>>>
>>>> With this setup, it's possible to do async IO with a single system
>>>> call. Future developments will enable polled IO with this interface,
>>>> and polled submission as well. The latter will enable an application
>>>> to do IO without doing ANY system calls at all.
>>>>
>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>> completions if it wants to wait for them to occur.
>>>>
>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>> as well. We will only punt to an async context if the command would
>>>> need to wait for IO on the device side. Any data that can be accessed
>>>> directly in the page cache is done inline. This avoids the slowness
>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>> as a sync interface.
> [...]
>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>> +{
>>>> + struct io_kiocb *req;
>>>> + ssize_t ret;
>>>> +
>>>> + /* enforce forwards compatibility on users */
>>>> + if (unlikely(s->sqe->flags))
>>>> + return -EINVAL;
>>>> +
>>>> + req = io_get_req(ctx);
>>>> + if (unlikely(!req))
>>>> + return -EAGAIN;
>>>> +
>>>> + req->rw.ki_filp = NULL;
>>>> +
>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>> + if (ret == -EAGAIN) {
>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>> + ret = 0;
>>>> + }
>>>> + if (ret)
>>>> + io_free_req(req);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>> +{
>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>> +
>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>> + /* write side barrier of head update, app has read side */
>>>> + smp_wmb();
>>>
>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>> nobody sees the updated head before you're done reading the submission
>>> queue entry? Or is that barrier elsewhere?
>>
>> The matching read barrier is in the application, it must do that before
>> reading ->head for the SQ ring.
>>
>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>> that should be all we need to ensure that loads are done.
>
> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
> ordering with regard to concurrent execution on other cores. They are
> only compiler barriers, influencing the order in which the compiler
> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
> a memory barrier that prevents reordering of dependent reads.)
>
> As far as I can tell, between the READ_ONCE(ring->array[...]) in
> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
> no *hardware* memory barrier that prevents reordering against
> concurrently running userspace code. As far as I can tell, the
> following could happen:
>
> - The kernel reads from ring->array in io_get_sqring(), then updates
> the head in io_commit_sqring(). The CPU reorders the memory accesses
> such that the write to the head becomes visible before the read from
> ring->array has completed.
> - Userspace observes the write to the head and reuses the array slots
> the kernel has freed with the write, clobbering ring->array before the
> kernel reads from ring->array.
I'd say this is highly theoretical for the normal use case, as we
will have submitted IO in between. Hence the load must have been done.
The only case that needs it is the sq thread case, since we bundle
those up. This should do it:
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9c5f93f6e3d9..b291796081b6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1889,6 +1889,9 @@ static int io_sq_thread(void *data)
break;
} while (io_get_sqring(ctx, &sqes[i]));
+ /* order the below ring head store with the SQE loads */
+ smp_mb();
+
io_commit_sqring(ctx);
/* Unless all new commands are FIXED regions, grab mm */
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 22:06 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <1ca9f039-c6f0-cae7-8484-7db0a4e4e213@kernel.dk>
On 2/12/19 3:03 PM, Jens Axboe wrote:
> On 2/12/19 2:42 PM, Jann Horn wrote:
>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>> between the application and the kernel. This eliminates the need to
>>>>> copy data back and forth to submit and complete IO.
>>>>>
>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>> arbitrary submission.
>>>>>
>>>>> Two new system calls are added for this:
>>>>>
>>>>> io_uring_setup(entries, params)
>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>> returns a file descriptor that the application can mmap to
>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>
>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>> them to complete, or both. The behavior is controlled by the
>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>> kernel to return already completed events without waiting
>>>>> for them. This is useful only for polling, as for IRQ
>>>>> driven IO, the application can just check the CQ ring
>>>>> without entering the kernel.
>>>>>
>>>>> With this setup, it's possible to do async IO with a single system
>>>>> call. Future developments will enable polled IO with this interface,
>>>>> and polled submission as well. The latter will enable an application
>>>>> to do IO without doing ANY system calls at all.
>>>>>
>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>> completions if it wants to wait for them to occur.
>>>>>
>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>> as well. We will only punt to an async context if the command would
>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>> as a sync interface.
>> [...]
>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>> +{
>>>>> + struct io_kiocb *req;
>>>>> + ssize_t ret;
>>>>> +
>>>>> + /* enforce forwards compatibility on users */
>>>>> + if (unlikely(s->sqe->flags))
>>>>> + return -EINVAL;
>>>>> +
>>>>> + req = io_get_req(ctx);
>>>>> + if (unlikely(!req))
>>>>> + return -EAGAIN;
>>>>> +
>>>>> + req->rw.ki_filp = NULL;
>>>>> +
>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>> + if (ret == -EAGAIN) {
>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>> + ret = 0;
>>>>> + }
>>>>> + if (ret)
>>>>> + io_free_req(req);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>> +{
>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>> +
>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>> + /* write side barrier of head update, app has read side */
>>>>> + smp_wmb();
>>>>
>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>> nobody sees the updated head before you're done reading the submission
>>>> queue entry? Or is that barrier elsewhere?
>>>
>>> The matching read barrier is in the application, it must do that before
>>> reading ->head for the SQ ring.
>>>
>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>> that should be all we need to ensure that loads are done.
>>
>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>> ordering with regard to concurrent execution on other cores. They are
>> only compiler barriers, influencing the order in which the compiler
>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>> a memory barrier that prevents reordering of dependent reads.)
>>
>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>> no *hardware* memory barrier that prevents reordering against
>> concurrently running userspace code. As far as I can tell, the
>> following could happen:
>>
>> - The kernel reads from ring->array in io_get_sqring(), then updates
>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>> such that the write to the head becomes visible before the read from
>> ring->array has completed.
>> - Userspace observes the write to the head and reuses the array slots
>> the kernel has freed with the write, clobbering ring->array before the
>> kernel reads from ring->array.
>
> I'd say this is highly theoretical for the normal use case, as we
> will have submitted IO in between. Hence the load must have been done.
> The only case that needs it is the sq thread case, since we bundle
> those up. This should do it:
Actually, I take that back, as in this particular case the sq thread
is the only one that reads it. Hence it'll have done a full submission
of the read SQE entries before reading a new round. Not that it matters
for that case, as a preempt would have implied a full barrier anyway.
The non-sq thread case does not need the store-vs-load ordering
barrier, as SQEs are either discarded or submitted before we commit
the sqring. Since that's the case, by definition all loads are done.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jann Horn @ 2019-02-12 22:40 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <f20b8e79-d10f-6316-561f-3c77cab71ee0@kernel.dk>
On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 2/12/19 3:03 PM, Jens Axboe wrote:
> > On 2/12/19 2:42 PM, Jann Horn wrote:
> >> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>> On 2/8/19 3:12 PM, Jann Horn wrote:
> >>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
> >>>>> between the application and the kernel. This eliminates the need to
> >>>>> copy data back and forth to submit and complete IO.
> >>>>>
> >>>>> IO submissions use the io_uring_sqe data structure, and completions
> >>>>> are generated in the form of io_uring_cqe data structures. The SQ
> >>>>> ring is an index into the io_uring_sqe array, which makes it possible
> >>>>> to submit a batch of IOs without them being contiguous in the ring.
> >>>>> The CQ ring is always contiguous, as completion events are inherently
> >>>>> unordered, and hence any io_uring_cqe entry can point back to an
> >>>>> arbitrary submission.
> >>>>>
> >>>>> Two new system calls are added for this:
> >>>>>
> >>>>> io_uring_setup(entries, params)
> >>>>> Sets up an io_uring instance for doing async IO. On success,
> >>>>> returns a file descriptor that the application can mmap to
> >>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
> >>>>>
> >>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> >>>>> Initiates IO against the rings mapped to this fd, or waits for
> >>>>> them to complete, or both. The behavior is controlled by the
> >>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
> >>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> >>>>> kernel will wait for 'min_complete' events, if they aren't
> >>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
> >>>>> and 'min_complete' == 0 at the same time, this allows the
> >>>>> kernel to return already completed events without waiting
> >>>>> for them. This is useful only for polling, as for IRQ
> >>>>> driven IO, the application can just check the CQ ring
> >>>>> without entering the kernel.
> >>>>>
> >>>>> With this setup, it's possible to do async IO with a single system
> >>>>> call. Future developments will enable polled IO with this interface,
> >>>>> and polled submission as well. The latter will enable an application
> >>>>> to do IO without doing ANY system calls at all.
> >>>>>
> >>>>> For IRQ driven IO, an application only needs to enter the kernel for
> >>>>> completions if it wants to wait for them to occur.
> >>>>>
> >>>>> Each io_uring is backed by a workqueue, to support buffered async IO
> >>>>> as well. We will only punt to an async context if the command would
> >>>>> need to wait for IO on the device side. Any data that can be accessed
> >>>>> directly in the page cache is done inline. This avoids the slowness
> >>>>> issue of usual threadpools, since cached data is accessed as quickly
> >>>>> as a sync interface.
> >> [...]
> >>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
> >>>>> +{
> >>>>> + struct io_kiocb *req;
> >>>>> + ssize_t ret;
> >>>>> +
> >>>>> + /* enforce forwards compatibility on users */
> >>>>> + if (unlikely(s->sqe->flags))
> >>>>> + return -EINVAL;
> >>>>> +
> >>>>> + req = io_get_req(ctx);
> >>>>> + if (unlikely(!req))
> >>>>> + return -EAGAIN;
> >>>>> +
> >>>>> + req->rw.ki_filp = NULL;
> >>>>> +
> >>>>> + ret = __io_submit_sqe(ctx, req, s, true);
> >>>>> + if (ret == -EAGAIN) {
> >>>>> + memcpy(&req->submit, s, sizeof(*s));
> >>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
> >>>>> + queue_work(ctx->sqo_wq, &req->work);
> >>>>> + ret = 0;
> >>>>> + }
> >>>>> + if (ret)
> >>>>> + io_free_req(req);
> >>>>> +
> >>>>> + return ret;
> >>>>> +}
> >>>>> +
> >>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
> >>>>> +{
> >>>>> + struct io_sq_ring *ring = ctx->sq_ring;
> >>>>> +
> >>>>> + if (ctx->cached_sq_head != ring->r.head) {
> >>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> >>>>> + /* write side barrier of head update, app has read side */
> >>>>> + smp_wmb();
> >>>>
> >>>> Can you elaborate on what this memory barrier is doing? Don't you need
> >>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> >>>> nobody sees the updated head before you're done reading the submission
> >>>> queue entry? Or is that barrier elsewhere?
> >>>
> >>> The matching read barrier is in the application, it must do that before
> >>> reading ->head for the SQ ring.
> >>>
> >>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
> >>> that should be all we need to ensure that loads are done.
> >>
> >> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
> >> ordering with regard to concurrent execution on other cores. They are
> >> only compiler barriers, influencing the order in which the compiler
> >> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
> >> a memory barrier that prevents reordering of dependent reads.)
> >>
> >> As far as I can tell, between the READ_ONCE(ring->array[...]) in
> >> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
> >> no *hardware* memory barrier that prevents reordering against
> >> concurrently running userspace code. As far as I can tell, the
> >> following could happen:
> >>
> >> - The kernel reads from ring->array in io_get_sqring(), then updates
> >> the head in io_commit_sqring(). The CPU reorders the memory accesses
> >> such that the write to the head becomes visible before the read from
> >> ring->array has completed.
> >> - Userspace observes the write to the head and reuses the array slots
> >> the kernel has freed with the write, clobbering ring->array before the
> >> kernel reads from ring->array.
> >
> > I'd say this is highly theoretical for the normal use case, as we
> > will have submitted IO in between. Hence the load must have been done.
Sorry, I'm confused. Who is "we", and which load are you referring to?
io_sq_thread() goes directly from io_get_sqring() to
io_commit_sqring(), with only a conditional io_sqe_needs_user() in
between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
"submitting IO" in the middle.
> > The only case that needs it is the sq thread case, since we bundle
> > those up. This should do it:
>
> Actually, I take that back, as in this particular case the sq thread
> is the only one that reads it.
What is "it"? The head pointer is written by the sq thread and read by
userspace, not the other way around. Are you talking about
ring->array? Sorry, I'm lost.
> Hence it'll have done a full submission
> of the read SQE entries before reading a new round. Not that it matters
> for that case, as a preempt would have implied a full barrier anyway.
> The non-sq thread case does not need the store-vs-load ordering
> barrier, as SQEs are either discarded or submitted before we commit
> the sqring. Since that's the case, by definition all loads are done.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 22:45 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez0rJU8bx4EFJwzXOpUX1C2J86pDFHtwEdvKf2K2tsWuig@mail.gmail.com>
On 2/12/19 3:40 PM, Jann Horn wrote:
> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>
>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>> arbitrary submission.
>>>>>>>
>>>>>>> Two new system calls are added for this:
>>>>>>>
>>>>>>> io_uring_setup(entries, params)
>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>
>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>> kernel to return already completed events without waiting
>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>> without entering the kernel.
>>>>>>>
>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>
>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>
>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>> as a sync interface.
>>>> [...]
>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>> +{
>>>>>>> + struct io_kiocb *req;
>>>>>>> + ssize_t ret;
>>>>>>> +
>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>> + return -EINVAL;
>>>>>>> +
>>>>>>> + req = io_get_req(ctx);
>>>>>>> + if (unlikely(!req))
>>>>>>> + return -EAGAIN;
>>>>>>> +
>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>> +
>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>> + if (ret == -EAGAIN) {
>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>> + ret = 0;
>>>>>>> + }
>>>>>>> + if (ret)
>>>>>>> + io_free_req(req);
>>>>>>> +
>>>>>>> + return ret;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>> +{
>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>> +
>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>> + smp_wmb();
>>>>>>
>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>
>>>>> The matching read barrier is in the application, it must do that before
>>>>> reading ->head for the SQ ring.
>>>>>
>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>> that should be all we need to ensure that loads are done.
>>>>
>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>> ordering with regard to concurrent execution on other cores. They are
>>>> only compiler barriers, influencing the order in which the compiler
>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>
>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>> no *hardware* memory barrier that prevents reordering against
>>>> concurrently running userspace code. As far as I can tell, the
>>>> following could happen:
>>>>
>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>> such that the write to the head becomes visible before the read from
>>>> ring->array has completed.
>>>> - Userspace observes the write to the head and reuses the array slots
>>>> the kernel has freed with the write, clobbering ring->array before the
>>>> kernel reads from ring->array.
>>>
>>> I'd say this is highly theoretical for the normal use case, as we
>>> will have submitted IO in between. Hence the load must have been done.
>
> Sorry, I'm confused. Who is "we", and which load are you referring to?
> io_sq_thread() goes directly from io_get_sqring() to
> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
> "submitting IO" in the middle.
You are right, the patch I sent IS needed for the sq thread case! It's
only true for the "normal" case that we don't need the smp_mb() before
writing the sq ring head, as sqes are fully consumed at that point.
>>> The only case that needs it is the sq thread case, since we bundle
>>> those up. This should do it:
>>
>> Actually, I take that back, as in this particular case the sq thread
>> is the only one that reads it.
>
> What is "it"? The head pointer is written by the sq thread and read by
> userspace, not the other way around. Are you talking about
> ring->array? Sorry, I'm lost.
I think we're on the same page, even if it doesn't necessarily sound
like it. We do need the smp_mb() before witing io_commit_sqring()
for the thread case.
I guess what confused me is that your commenting on the main patch,
the case that needs it isn't introduced until later in the series.
I'll fold the fix into that patch.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 22:52 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <041f1c67-b62e-a593-fdc0-b44e35a4da4e@kernel.dk>
On 2/12/19 3:45 PM, Jens Axboe wrote:
> On 2/12/19 3:40 PM, Jann Horn wrote:
>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>
>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>> arbitrary submission.
>>>>>>>>
>>>>>>>> Two new system calls are added for this:
>>>>>>>>
>>>>>>>> io_uring_setup(entries, params)
>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>
>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>> kernel to return already completed events without waiting
>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>> without entering the kernel.
>>>>>>>>
>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>
>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>
>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>> as a sync interface.
>>>>> [...]
>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>> +{
>>>>>>>> + struct io_kiocb *req;
>>>>>>>> + ssize_t ret;
>>>>>>>> +
>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + req = io_get_req(ctx);
>>>>>>>> + if (unlikely(!req))
>>>>>>>> + return -EAGAIN;
>>>>>>>> +
>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>> +
>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>> + ret = 0;
>>>>>>>> + }
>>>>>>>> + if (ret)
>>>>>>>> + io_free_req(req);
>>>>>>>> +
>>>>>>>> + return ret;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>> +{
>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>> +
>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>> + smp_wmb();
>>>>>>>
>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>
>>>>>> The matching read barrier is in the application, it must do that before
>>>>>> reading ->head for the SQ ring.
>>>>>>
>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>> that should be all we need to ensure that loads are done.
>>>>>
>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>> only compiler barriers, influencing the order in which the compiler
>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>
>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>> no *hardware* memory barrier that prevents reordering against
>>>>> concurrently running userspace code. As far as I can tell, the
>>>>> following could happen:
>>>>>
>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>> such that the write to the head becomes visible before the read from
>>>>> ring->array has completed.
>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>> kernel reads from ring->array.
>>>>
>>>> I'd say this is highly theoretical for the normal use case, as we
>>>> will have submitted IO in between. Hence the load must have been done.
>>
>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>> io_sq_thread() goes directly from io_get_sqring() to
>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>> "submitting IO" in the middle.
>
> You are right, the patch I sent IS needed for the sq thread case! It's
> only true for the "normal" case that we don't need the smp_mb() before
> writing the sq ring head, as sqes are fully consumed at that point.
>
>>>> The only case that needs it is the sq thread case, since we bundle
>>>> those up. This should do it:
>>>
>>> Actually, I take that back, as in this particular case the sq thread
>>> is the only one that reads it.
>>
>> What is "it"? The head pointer is written by the sq thread and read by
>> userspace, not the other way around. Are you talking about
>> ring->array? Sorry, I'm lost.
>
> I think we're on the same page, even if it doesn't necessarily sound
> like it. We do need the smp_mb() before witing io_commit_sqring()
> for the thread case.
>
> I guess what confused me is that your commenting on the main patch,
> the case that needs it isn't introduced until later in the series.
> I'll fold the fix into that patch.
A better fix is to let the sq thread have the same behavior as the
application driven path, simply committing the sq ring once we've
consumed the sqes instead. That's just moving the io_sqring_commit()
below io_submit_sqes().
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jann Horn @ 2019-02-12 22:57 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <7149d509-25a1-eb3b-b4c6-6bb2d7a87465@kernel.dk>
On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 2/12/19 3:45 PM, Jens Axboe wrote:
> > On 2/12/19 3:40 PM, Jann Horn wrote:
> >> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>
> >>> On 2/12/19 3:03 PM, Jens Axboe wrote:
> >>>> On 2/12/19 2:42 PM, Jann Horn wrote:
> >>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
> >>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
> >>>>>>>> between the application and the kernel. This eliminates the need to
> >>>>>>>> copy data back and forth to submit and complete IO.
> >>>>>>>>
> >>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
> >>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
> >>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
> >>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
> >>>>>>>> The CQ ring is always contiguous, as completion events are inherently
> >>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
> >>>>>>>> arbitrary submission.
> >>>>>>>>
> >>>>>>>> Two new system calls are added for this:
> >>>>>>>>
> >>>>>>>> io_uring_setup(entries, params)
> >>>>>>>> Sets up an io_uring instance for doing async IO. On success,
> >>>>>>>> returns a file descriptor that the application can mmap to
> >>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
> >>>>>>>>
> >>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> >>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
> >>>>>>>> them to complete, or both. The behavior is controlled by the
> >>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
> >>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> >>>>>>>> kernel will wait for 'min_complete' events, if they aren't
> >>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
> >>>>>>>> and 'min_complete' == 0 at the same time, this allows the
> >>>>>>>> kernel to return already completed events without waiting
> >>>>>>>> for them. This is useful only for polling, as for IRQ
> >>>>>>>> driven IO, the application can just check the CQ ring
> >>>>>>>> without entering the kernel.
> >>>>>>>>
> >>>>>>>> With this setup, it's possible to do async IO with a single system
> >>>>>>>> call. Future developments will enable polled IO with this interface,
> >>>>>>>> and polled submission as well. The latter will enable an application
> >>>>>>>> to do IO without doing ANY system calls at all.
> >>>>>>>>
> >>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
> >>>>>>>> completions if it wants to wait for them to occur.
> >>>>>>>>
> >>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
> >>>>>>>> as well. We will only punt to an async context if the command would
> >>>>>>>> need to wait for IO on the device side. Any data that can be accessed
> >>>>>>>> directly in the page cache is done inline. This avoids the slowness
> >>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
> >>>>>>>> as a sync interface.
> >>>>> [...]
> >>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
> >>>>>>>> +{
> >>>>>>>> + struct io_kiocb *req;
> >>>>>>>> + ssize_t ret;
> >>>>>>>> +
> >>>>>>>> + /* enforce forwards compatibility on users */
> >>>>>>>> + if (unlikely(s->sqe->flags))
> >>>>>>>> + return -EINVAL;
> >>>>>>>> +
> >>>>>>>> + req = io_get_req(ctx);
> >>>>>>>> + if (unlikely(!req))
> >>>>>>>> + return -EAGAIN;
> >>>>>>>> +
> >>>>>>>> + req->rw.ki_filp = NULL;
> >>>>>>>> +
> >>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
> >>>>>>>> + if (ret == -EAGAIN) {
> >>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
> >>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
> >>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
> >>>>>>>> + ret = 0;
> >>>>>>>> + }
> >>>>>>>> + if (ret)
> >>>>>>>> + io_free_req(req);
> >>>>>>>> +
> >>>>>>>> + return ret;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
> >>>>>>>> +{
> >>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
> >>>>>>>> +
> >>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
> >>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> >>>>>>>> + /* write side barrier of head update, app has read side */
> >>>>>>>> + smp_wmb();
> >>>>>>>
> >>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
> >>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> >>>>>>> nobody sees the updated head before you're done reading the submission
> >>>>>>> queue entry? Or is that barrier elsewhere?
> >>>>>>
> >>>>>> The matching read barrier is in the application, it must do that before
> >>>>>> reading ->head for the SQ ring.
> >>>>>>
> >>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
> >>>>>> that should be all we need to ensure that loads are done.
> >>>>>
> >>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
> >>>>> ordering with regard to concurrent execution on other cores. They are
> >>>>> only compiler barriers, influencing the order in which the compiler
> >>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
> >>>>> a memory barrier that prevents reordering of dependent reads.)
> >>>>>
> >>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
> >>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
> >>>>> no *hardware* memory barrier that prevents reordering against
> >>>>> concurrently running userspace code. As far as I can tell, the
> >>>>> following could happen:
> >>>>>
> >>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
> >>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
> >>>>> such that the write to the head becomes visible before the read from
> >>>>> ring->array has completed.
> >>>>> - Userspace observes the write to the head and reuses the array slots
> >>>>> the kernel has freed with the write, clobbering ring->array before the
> >>>>> kernel reads from ring->array.
> >>>>
> >>>> I'd say this is highly theoretical for the normal use case, as we
> >>>> will have submitted IO in between. Hence the load must have been done.
> >>
> >> Sorry, I'm confused. Who is "we", and which load are you referring to?
> >> io_sq_thread() goes directly from io_get_sqring() to
> >> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
> >> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
> >> "submitting IO" in the middle.
> >
> > You are right, the patch I sent IS needed for the sq thread case! It's
> > only true for the "normal" case that we don't need the smp_mb() before
> > writing the sq ring head, as sqes are fully consumed at that point.
Hmm... does that actually matter? As long as you don't have an
explicit barrier for this, the CPU could still reorder things, right?
Pull the store in front of everything else?
> >>>> The only case that needs it is the sq thread case, since we bundle
> >>>> those up. This should do it:
> >>>
> >>> Actually, I take that back, as in this particular case the sq thread
> >>> is the only one that reads it.
> >>
> >> What is "it"? The head pointer is written by the sq thread and read by
> >> userspace, not the other way around. Are you talking about
> >> ring->array? Sorry, I'm lost.
> >
> > I think we're on the same page, even if it doesn't necessarily sound
> > like it. We do need the smp_mb() before witing io_commit_sqring()
> > for the thread case.
> >
> > I guess what confused me is that your commenting on the main patch,
> > the case that needs it isn't introduced until later in the series.
Ah, yes, I guess that was a bit confusing.
> > I'll fold the fix into that patch.
> A better fix is to let the sq thread have the same behavior as the
> application driven path, simply committing the sq ring once we've
> consumed the sqes instead. That's just moving the io_sqring_commit()
> below io_submit_sqes().
Hmm. How does that help?
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 23:00 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez09zTXuxEJE4KKfv-9yeYPGbFm_b9YnYK70csTQ1UXiuA@mail.gmail.com>
On 2/12/19 3:57 PM, Jann Horn wrote:
> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 2/12/19 3:45 PM, Jens Axboe wrote:
>>> On 2/12/19 3:40 PM, Jann Horn wrote:
>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>
>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>>>
>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>>>> arbitrary submission.
>>>>>>>>>>
>>>>>>>>>> Two new system calls are added for this:
>>>>>>>>>>
>>>>>>>>>> io_uring_setup(entries, params)
>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>>>
>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>>>> kernel to return already completed events without waiting
>>>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>>>> without entering the kernel.
>>>>>>>>>>
>>>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>>>
>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>>>
>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>>>> as a sync interface.
>>>>>>> [...]
>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>>>> +{
>>>>>>>>>> + struct io_kiocb *req;
>>>>>>>>>> + ssize_t ret;
>>>>>>>>>> +
>>>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>>>> + return -EINVAL;
>>>>>>>>>> +
>>>>>>>>>> + req = io_get_req(ctx);
>>>>>>>>>> + if (unlikely(!req))
>>>>>>>>>> + return -EAGAIN;
>>>>>>>>>> +
>>>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>>>> +
>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>>>> + ret = 0;
>>>>>>>>>> + }
>>>>>>>>>> + if (ret)
>>>>>>>>>> + io_free_req(req);
>>>>>>>>>> +
>>>>>>>>>> + return ret;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>>>> +{
>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>>>> +
>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>>>> + smp_wmb();
>>>>>>>>>
>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>>>
>>>>>>>> The matching read barrier is in the application, it must do that before
>>>>>>>> reading ->head for the SQ ring.
>>>>>>>>
>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>>>> that should be all we need to ensure that loads are done.
>>>>>>>
>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>>>> only compiler barriers, influencing the order in which the compiler
>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>>>
>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>>>> no *hardware* memory barrier that prevents reordering against
>>>>>>> concurrently running userspace code. As far as I can tell, the
>>>>>>> following could happen:
>>>>>>>
>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>>>> such that the write to the head becomes visible before the read from
>>>>>>> ring->array has completed.
>>>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>>>> kernel reads from ring->array.
>>>>>>
>>>>>> I'd say this is highly theoretical for the normal use case, as we
>>>>>> will have submitted IO in between. Hence the load must have been done.
>>>>
>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>>>> io_sq_thread() goes directly from io_get_sqring() to
>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>>>> "submitting IO" in the middle.
>>>
>>> You are right, the patch I sent IS needed for the sq thread case! It's
>>> only true for the "normal" case that we don't need the smp_mb() before
>>> writing the sq ring head, as sqes are fully consumed at that point.
>
> Hmm... does that actually matter? As long as you don't have an
> explicit barrier for this, the CPU could still reorder things, right?
> Pull the store in front of everything else?
If the IO has been submitted, by definition the loads have completed.
At that point it should be fine to commit the ring head that the
application sees.
>>> I'll fold the fix into that patch.
>> A better fix is to let the sq thread have the same behavior as the
>> application driven path, simply committing the sq ring once we've
>> consumed the sqes instead. That's just moving the io_sqring_commit()
>> below io_submit_sqes().
>
> Hmm. How does that help?
Because then it'll have submitted the IO, and hence loads from the sqes
in question must have been done.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jann Horn @ 2019-02-12 23:11 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <beb052ba-2c4a-0201-d673-c7b8eef294dd@kernel.dk>
On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 2/12/19 3:57 PM, Jann Horn wrote:
> > On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 2/12/19 3:45 PM, Jens Axboe wrote:
> >>> On 2/12/19 3:40 PM, Jann Horn wrote:
> >>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>
> >>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
> >>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
> >>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
> >>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
> >>>>>>>>>> between the application and the kernel. This eliminates the need to
> >>>>>>>>>> copy data back and forth to submit and complete IO.
> >>>>>>>>>>
> >>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
> >>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
> >>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
> >>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
> >>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
> >>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
> >>>>>>>>>> arbitrary submission.
> >>>>>>>>>>
> >>>>>>>>>> Two new system calls are added for this:
> >>>>>>>>>>
> >>>>>>>>>> io_uring_setup(entries, params)
> >>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
> >>>>>>>>>> returns a file descriptor that the application can mmap to
> >>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
> >>>>>>>>>>
> >>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> >>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
> >>>>>>>>>> them to complete, or both. The behavior is controlled by the
> >>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
> >>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> >>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
> >>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
> >>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
> >>>>>>>>>> kernel to return already completed events without waiting
> >>>>>>>>>> for them. This is useful only for polling, as for IRQ
> >>>>>>>>>> driven IO, the application can just check the CQ ring
> >>>>>>>>>> without entering the kernel.
> >>>>>>>>>>
> >>>>>>>>>> With this setup, it's possible to do async IO with a single system
> >>>>>>>>>> call. Future developments will enable polled IO with this interface,
> >>>>>>>>>> and polled submission as well. The latter will enable an application
> >>>>>>>>>> to do IO without doing ANY system calls at all.
> >>>>>>>>>>
> >>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
> >>>>>>>>>> completions if it wants to wait for them to occur.
> >>>>>>>>>>
> >>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
> >>>>>>>>>> as well. We will only punt to an async context if the command would
> >>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
> >>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
> >>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
> >>>>>>>>>> as a sync interface.
> >>>>>>> [...]
> >>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
> >>>>>>>>>> +{
> >>>>>>>>>> + struct io_kiocb *req;
> >>>>>>>>>> + ssize_t ret;
> >>>>>>>>>> +
> >>>>>>>>>> + /* enforce forwards compatibility on users */
> >>>>>>>>>> + if (unlikely(s->sqe->flags))
> >>>>>>>>>> + return -EINVAL;
> >>>>>>>>>> +
> >>>>>>>>>> + req = io_get_req(ctx);
> >>>>>>>>>> + if (unlikely(!req))
> >>>>>>>>>> + return -EAGAIN;
> >>>>>>>>>> +
> >>>>>>>>>> + req->rw.ki_filp = NULL;
> >>>>>>>>>> +
> >>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
> >>>>>>>>>> + if (ret == -EAGAIN) {
> >>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
> >>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
> >>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
> >>>>>>>>>> + ret = 0;
> >>>>>>>>>> + }
> >>>>>>>>>> + if (ret)
> >>>>>>>>>> + io_free_req(req);
> >>>>>>>>>> +
> >>>>>>>>>> + return ret;
> >>>>>>>>>> +}
> >>>>>>>>>> +
> >>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
> >>>>>>>>>> +{
> >>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
> >>>>>>>>>> +
> >>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
> >>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> >>>>>>>>>> + /* write side barrier of head update, app has read side */
> >>>>>>>>>> + smp_wmb();
> >>>>>>>>>
> >>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
> >>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> >>>>>>>>> nobody sees the updated head before you're done reading the submission
> >>>>>>>>> queue entry? Or is that barrier elsewhere?
> >>>>>>>>
> >>>>>>>> The matching read barrier is in the application, it must do that before
> >>>>>>>> reading ->head for the SQ ring.
> >>>>>>>>
> >>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
> >>>>>>>> that should be all we need to ensure that loads are done.
> >>>>>>>
> >>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
> >>>>>>> ordering with regard to concurrent execution on other cores. They are
> >>>>>>> only compiler barriers, influencing the order in which the compiler
> >>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
> >>>>>>> a memory barrier that prevents reordering of dependent reads.)
> >>>>>>>
> >>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
> >>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
> >>>>>>> no *hardware* memory barrier that prevents reordering against
> >>>>>>> concurrently running userspace code. As far as I can tell, the
> >>>>>>> following could happen:
> >>>>>>>
> >>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
> >>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
> >>>>>>> such that the write to the head becomes visible before the read from
> >>>>>>> ring->array has completed.
> >>>>>>> - Userspace observes the write to the head and reuses the array slots
> >>>>>>> the kernel has freed with the write, clobbering ring->array before the
> >>>>>>> kernel reads from ring->array.
> >>>>>>
> >>>>>> I'd say this is highly theoretical for the normal use case, as we
> >>>>>> will have submitted IO in between. Hence the load must have been done.
> >>>>
> >>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
> >>>> io_sq_thread() goes directly from io_get_sqring() to
> >>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
> >>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
> >>>> "submitting IO" in the middle.
> >>>
> >>> You are right, the patch I sent IS needed for the sq thread case! It's
> >>> only true for the "normal" case that we don't need the smp_mb() before
> >>> writing the sq ring head, as sqes are fully consumed at that point.
> >
> > Hmm... does that actually matter? As long as you don't have an
> > explicit barrier for this, the CPU could still reorder things, right?
> > Pull the store in front of everything else?
>
> If the IO has been submitted, by definition the loads have completed.
> At that point it should be fine to commit the ring head that the
> application sees.
What exactly do you mean by "the IO has been submitted"? Are you
talking about interaction with hardware, or about the end of the
syscall, or something else?
> >>> I'll fold the fix into that patch.
> >> A better fix is to let the sq thread have the same behavior as the
> >> application driven path, simply committing the sq ring once we've
> >> consumed the sqes instead. That's just moving the io_sqring_commit()
> >> below io_submit_sqes().
> >
> > Hmm. How does that help?
>
> Because then it'll have submitted the IO, and hence loads from the sqes
> in question must have been done.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 23:19 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez22G1orwZ43tZ6VK3rngQitar0qH9zJXdTv8kX2e2HqOQ@mail.gmail.com>
On 2/12/19 4:11 PM, Jann Horn wrote:
> On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 2/12/19 3:57 PM, Jann Horn wrote:
>>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
>>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
>>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>
>>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>>>>>
>>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>>>>>> arbitrary submission.
>>>>>>>>>>>>
>>>>>>>>>>>> Two new system calls are added for this:
>>>>>>>>>>>>
>>>>>>>>>>>> io_uring_setup(entries, params)
>>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>>>>>
>>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>>>>>> kernel to return already completed events without waiting
>>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>>>>>> without entering the kernel.
>>>>>>>>>>>>
>>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>>>>>
>>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>>>>>
>>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>>>>>> as a sync interface.
>>>>>>>>> [...]
>>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>>>>>> +{
>>>>>>>>>>>> + struct io_kiocb *req;
>>>>>>>>>>>> + ssize_t ret;
>>>>>>>>>>>> +
>>>>>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>>> +
>>>>>>>>>>>> + req = io_get_req(ctx);
>>>>>>>>>>>> + if (unlikely(!req))
>>>>>>>>>>>> + return -EAGAIN;
>>>>>>>>>>>> +
>>>>>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>>>>>> +
>>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>> + }
>>>>>>>>>>>> + if (ret)
>>>>>>>>>>>> + io_free_req(req);
>>>>>>>>>>>> +
>>>>>>>>>>>> + return ret;
>>>>>>>>>>>> +}
>>>>>>>>>>>> +
>>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>>>>>> +{
>>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>>>>>> +
>>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>>>>>> + smp_wmb();
>>>>>>>>>>>
>>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>>>>>
>>>>>>>>>> The matching read barrier is in the application, it must do that before
>>>>>>>>>> reading ->head for the SQ ring.
>>>>>>>>>>
>>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>>>>>> that should be all we need to ensure that loads are done.
>>>>>>>>>
>>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>>>>>> only compiler barriers, influencing the order in which the compiler
>>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>>>>>
>>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>>>>>> no *hardware* memory barrier that prevents reordering against
>>>>>>>>> concurrently running userspace code. As far as I can tell, the
>>>>>>>>> following could happen:
>>>>>>>>>
>>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>>>>>> such that the write to the head becomes visible before the read from
>>>>>>>>> ring->array has completed.
>>>>>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>>>>>> kernel reads from ring->array.
>>>>>>>>
>>>>>>>> I'd say this is highly theoretical for the normal use case, as we
>>>>>>>> will have submitted IO in between. Hence the load must have been done.
>>>>>>
>>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>>>>>> io_sq_thread() goes directly from io_get_sqring() to
>>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>>>>>> "submitting IO" in the middle.
>>>>>
>>>>> You are right, the patch I sent IS needed for the sq thread case! It's
>>>>> only true for the "normal" case that we don't need the smp_mb() before
>>>>> writing the sq ring head, as sqes are fully consumed at that point.
>>>
>>> Hmm... does that actually matter? As long as you don't have an
>>> explicit barrier for this, the CPU could still reorder things, right?
>>> Pull the store in front of everything else?
>>
>> If the IO has been submitted, by definition the loads have completed.
>> At that point it should be fine to commit the ring head that the
>> application sees.
>
> What exactly do you mean by "the IO has been submitted"? Are you
> talking about interaction with hardware, or about the end of the
> syscall, or something else?
I mean that the loads from the sqe, which the IO is made of, have been
done. That's what we care about here, right? The sqe has either been
turned into an io request and has been submitted, or it has been copied.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jann Horn @ 2019-02-12 23:28 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <0641e74d-0277-9cdb-2b13-63ee60f9196d@kernel.dk>
On Wed, Feb 13, 2019 at 12:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> On 2/12/19 4:11 PM, Jann Horn wrote:
> > On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> On 2/12/19 3:57 PM, Jann Horn wrote:
> >>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>
> >>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
> >>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
> >>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>
> >>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
> >>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
> >>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
> >>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
> >>>>>>>>>>>> between the application and the kernel. This eliminates the need to
> >>>>>>>>>>>> copy data back and forth to submit and complete IO.
> >>>>>>>>>>>>
> >>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
> >>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
> >>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
> >>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
> >>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
> >>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
> >>>>>>>>>>>> arbitrary submission.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Two new system calls are added for this:
> >>>>>>>>>>>>
> >>>>>>>>>>>> io_uring_setup(entries, params)
> >>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
> >>>>>>>>>>>> returns a file descriptor that the application can mmap to
> >>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
> >>>>>>>>>>>>
> >>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> >>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
> >>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
> >>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
> >>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> >>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
> >>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
> >>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
> >>>>>>>>>>>> kernel to return already completed events without waiting
> >>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
> >>>>>>>>>>>> driven IO, the application can just check the CQ ring
> >>>>>>>>>>>> without entering the kernel.
> >>>>>>>>>>>>
> >>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
> >>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
> >>>>>>>>>>>> and polled submission as well. The latter will enable an application
> >>>>>>>>>>>> to do IO without doing ANY system calls at all.
> >>>>>>>>>>>>
> >>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
> >>>>>>>>>>>> completions if it wants to wait for them to occur.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
> >>>>>>>>>>>> as well. We will only punt to an async context if the command would
> >>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
> >>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
> >>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
> >>>>>>>>>>>> as a sync interface.
> >>>>>>>>> [...]
> >>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> + struct io_kiocb *req;
> >>>>>>>>>>>> + ssize_t ret;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> + /* enforce forwards compatibility on users */
> >>>>>>>>>>>> + if (unlikely(s->sqe->flags))
> >>>>>>>>>>>> + return -EINVAL;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> + req = io_get_req(ctx);
> >>>>>>>>>>>> + if (unlikely(!req))
> >>>>>>>>>>>> + return -EAGAIN;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> + req->rw.ki_filp = NULL;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
> >>>>>>>>>>>> + if (ret == -EAGAIN) {
> >>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
> >>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
> >>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
> >>>>>>>>>>>> + ret = 0;
> >>>>>>>>>>>> + }
> >>>>>>>>>>>> + if (ret)
> >>>>>>>>>>>> + io_free_req(req);
> >>>>>>>>>>>> +
> >>>>>>>>>>>> + return ret;
> >>>>>>>>>>>> +}
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
> >>>>>>>>>>>> +
> >>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
> >>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> >>>>>>>>>>>> + /* write side barrier of head update, app has read side */
> >>>>>>>>>>>> + smp_wmb();
> >>>>>>>>>>>
> >>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
> >>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> >>>>>>>>>>> nobody sees the updated head before you're done reading the submission
> >>>>>>>>>>> queue entry? Or is that barrier elsewhere?
> >>>>>>>>>>
> >>>>>>>>>> The matching read barrier is in the application, it must do that before
> >>>>>>>>>> reading ->head for the SQ ring.
> >>>>>>>>>>
> >>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
> >>>>>>>>>> that should be all we need to ensure that loads are done.
> >>>>>>>>>
> >>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
> >>>>>>>>> ordering with regard to concurrent execution on other cores. They are
> >>>>>>>>> only compiler barriers, influencing the order in which the compiler
> >>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
> >>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
> >>>>>>>>>
> >>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
> >>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
> >>>>>>>>> no *hardware* memory barrier that prevents reordering against
> >>>>>>>>> concurrently running userspace code. As far as I can tell, the
> >>>>>>>>> following could happen:
> >>>>>>>>>
> >>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
> >>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
> >>>>>>>>> such that the write to the head becomes visible before the read from
> >>>>>>>>> ring->array has completed.
> >>>>>>>>> - Userspace observes the write to the head and reuses the array slots
> >>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
> >>>>>>>>> kernel reads from ring->array.
> >>>>>>>>
> >>>>>>>> I'd say this is highly theoretical for the normal use case, as we
> >>>>>>>> will have submitted IO in between. Hence the load must have been done.
> >>>>>>
> >>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
> >>>>>> io_sq_thread() goes directly from io_get_sqring() to
> >>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
> >>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
> >>>>>> "submitting IO" in the middle.
> >>>>>
> >>>>> You are right, the patch I sent IS needed for the sq thread case! It's
> >>>>> only true for the "normal" case that we don't need the smp_mb() before
> >>>>> writing the sq ring head, as sqes are fully consumed at that point.
> >>>
> >>> Hmm... does that actually matter? As long as you don't have an
> >>> explicit barrier for this, the CPU could still reorder things, right?
> >>> Pull the store in front of everything else?
> >>
> >> If the IO has been submitted, by definition the loads have completed.
> >> At that point it should be fine to commit the ring head that the
> >> application sees.
> >
> > What exactly do you mean by "the IO has been submitted"? Are you
> > talking about interaction with hardware, or about the end of the
> > syscall, or something else?
>
> I mean that the loads from the sqe, which the IO is made of, have been
> done. That's what we care about here, right? The sqe has either been
> turned into an io request and has been submitted, or it has been copied.
But they might not actually be done. AFAIU the CPU is allowed to do
the WRITE_ONCE of the head before doing any of the reads from the sqe
- loads and stores you do, as observed by a concurrently executing
thread, can happen in an order independent of the order in which you
write them in your code unless you use memory barriers. So the CPU
might decide to first write the new head, then do the read for
io_get_sqring(), and then do the __io_submit_sqe(), potentially
reading e.g. a IORING_OP_NOP opcode that has been written by
concurrently executing userspace after userspace has observed the
bumped head.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 23:46 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <CAG48ez3PESTFsySiZ1T2w-+4xo6mqkHRBv+Hgs2BKy-4w+7yug@mail.gmail.com>
On 2/12/19 4:28 PM, Jann Horn wrote:
> On Wed, Feb 13, 2019 at 12:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> On 2/12/19 4:11 PM, Jann Horn wrote:
>>> On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> On 2/12/19 3:57 PM, Jann Horn wrote:
>>>>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>
>>>>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
>>>>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
>>>>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>
>>>>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>>>>>>>> arbitrary submission.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Two new system calls are added for this:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> io_uring_setup(entries, params)
>>>>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>>>>>>>> kernel to return already completed events without waiting
>>>>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>>>>>>>> without entering the kernel.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>>>>>>>> as a sync interface.
>>>>>>>>>>> [...]
>>>>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> + struct io_kiocb *req;
>>>>>>>>>>>>>> + ssize_t ret;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> + req = io_get_req(ctx);
>>>>>>>>>>>>>> + if (unlikely(!req))
>>>>>>>>>>>>>> + return -EAGAIN;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>> + if (ret)
>>>>>>>>>>>>>> + io_free_req(req);
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>>>>>>>> + smp_wmb();
>>>>>>>>>>>>>
>>>>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>>>>>>>
>>>>>>>>>>>> The matching read barrier is in the application, it must do that before
>>>>>>>>>>>> reading ->head for the SQ ring.
>>>>>>>>>>>>
>>>>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>>>>>>>> that should be all we need to ensure that loads are done.
>>>>>>>>>>>
>>>>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>>>>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>>>>>>>> only compiler barriers, influencing the order in which the compiler
>>>>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>>>>>>>
>>>>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>>>>>>>> no *hardware* memory barrier that prevents reordering against
>>>>>>>>>>> concurrently running userspace code. As far as I can tell, the
>>>>>>>>>>> following could happen:
>>>>>>>>>>>
>>>>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>>>>>>>> such that the write to the head becomes visible before the read from
>>>>>>>>>>> ring->array has completed.
>>>>>>>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>>>>>>>> kernel reads from ring->array.
>>>>>>>>>>
>>>>>>>>>> I'd say this is highly theoretical for the normal use case, as we
>>>>>>>>>> will have submitted IO in between. Hence the load must have been done.
>>>>>>>>
>>>>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>>>>>>>> io_sq_thread() goes directly from io_get_sqring() to
>>>>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>>>>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>>>>>>>> "submitting IO" in the middle.
>>>>>>>
>>>>>>> You are right, the patch I sent IS needed for the sq thread case! It's
>>>>>>> only true for the "normal" case that we don't need the smp_mb() before
>>>>>>> writing the sq ring head, as sqes are fully consumed at that point.
>>>>>
>>>>> Hmm... does that actually matter? As long as you don't have an
>>>>> explicit barrier for this, the CPU could still reorder things, right?
>>>>> Pull the store in front of everything else?
>>>>
>>>> If the IO has been submitted, by definition the loads have completed.
>>>> At that point it should be fine to commit the ring head that the
>>>> application sees.
>>>
>>> What exactly do you mean by "the IO has been submitted"? Are you
>>> talking about interaction with hardware, or about the end of the
>>> syscall, or something else?
>>
>> I mean that the loads from the sqe, which the IO is made of, have been
>> done. That's what we care about here, right? The sqe has either been
>> turned into an io request and has been submitted, or it has been copied.
>
> But they might not actually be done. AFAIU the CPU is allowed to do
> the WRITE_ONCE of the head before doing any of the reads from the sqe
> - loads and stores you do, as observed by a concurrently executing
> thread, can happen in an order independent of the order in which you
> write them in your code unless you use memory barriers. So the CPU
> might decide to first write the new head, then do the read for
> io_get_sqring(), and then do the __io_submit_sqe(), potentially
> reading e.g. a IORING_OP_NOP opcode that has been written by
> concurrently executing userspace after userspace has observed the
> bumped head.
For that to be possible, we'd need NO ordering in between the IO
submission and when we write the sq ring head. A single spin lock
should do it, right?
It's not that I'm set against adding an smp_mb() to io_commit_sqring(),
but I think we're going off the deep end a little bit here on
theoretical vs what can practically happen.
For the regular IO cases, we will have done at least one lock/unlock
cycle. This is true for nops as well, and poll. The only case that could
potentially NOT have one is the fsync, for the case where we punt and
don't add it to existing work, we don't have any locking in between.
I'll add the smp_mb() for peace of mind.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-12 23:53 UTC (permalink / raw)
To: Jann Horn
Cc: linux-aio, linux-block, Linux API, hch, jmoyer, Avi Kivity,
Al Viro
In-Reply-To: <e70a7451-2fb4-5800-500d-6ae46fbfe5f4@kernel.dk>
On 2/12/19 4:46 PM, Jens Axboe wrote:
> On 2/12/19 4:28 PM, Jann Horn wrote:
>> On Wed, Feb 13, 2019 at 12:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>
>>> On 2/12/19 4:11 PM, Jann Horn wrote:
>>>> On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>
>>>>> On 2/12/19 3:57 PM, Jann Horn wrote:
>>>>>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>
>>>>>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
>>>>>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
>>>>>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>
>>>>>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>>>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>>>>>>>>> arbitrary submission.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Two new system calls are added for this:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> io_uring_setup(entries, params)
>>>>>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>>>>>>>>> kernel to return already completed events without waiting
>>>>>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>>>>>>>>> without entering the kernel.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>>>>>>>>> as a sync interface.
>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>> + struct io_kiocb *req;
>>>>>>>>>>>>>>> + ssize_t ret;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + req = io_get_req(ctx);
>>>>>>>>>>>>>>> + if (unlikely(!req))
>>>>>>>>>>>>>>> + return -EAGAIN;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>> + if (ret)
>>>>>>>>>>>>>>> + io_free_req(req);
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>>>>>>>>> + smp_wmb();
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>>>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>>>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>>>>>>>>
>>>>>>>>>>>>> The matching read barrier is in the application, it must do that before
>>>>>>>>>>>>> reading ->head for the SQ ring.
>>>>>>>>>>>>>
>>>>>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>>>>>>>>> that should be all we need to ensure that loads are done.
>>>>>>>>>>>>
>>>>>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>>>>>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>>>>>>>>> only compiler barriers, influencing the order in which the compiler
>>>>>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>>>>>>>>
>>>>>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>>>>>>>>> no *hardware* memory barrier that prevents reordering against
>>>>>>>>>>>> concurrently running userspace code. As far as I can tell, the
>>>>>>>>>>>> following could happen:
>>>>>>>>>>>>
>>>>>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>>>>>>>>> such that the write to the head becomes visible before the read from
>>>>>>>>>>>> ring->array has completed.
>>>>>>>>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>>>>>>>>> kernel reads from ring->array.
>>>>>>>>>>>
>>>>>>>>>>> I'd say this is highly theoretical for the normal use case, as we
>>>>>>>>>>> will have submitted IO in between. Hence the load must have been done.
>>>>>>>>>
>>>>>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>>>>>>>>> io_sq_thread() goes directly from io_get_sqring() to
>>>>>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>>>>>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>>>>>>>>> "submitting IO" in the middle.
>>>>>>>>
>>>>>>>> You are right, the patch I sent IS needed for the sq thread case! It's
>>>>>>>> only true for the "normal" case that we don't need the smp_mb() before
>>>>>>>> writing the sq ring head, as sqes are fully consumed at that point.
>>>>>>
>>>>>> Hmm... does that actually matter? As long as you don't have an
>>>>>> explicit barrier for this, the CPU could still reorder things, right?
>>>>>> Pull the store in front of everything else?
>>>>>
>>>>> If the IO has been submitted, by definition the loads have completed.
>>>>> At that point it should be fine to commit the ring head that the
>>>>> application sees.
>>>>
>>>> What exactly do you mean by "the IO has been submitted"? Are you
>>>> talking about interaction with hardware, or about the end of the
>>>> syscall, or something else?
>>>
>>> I mean that the loads from the sqe, which the IO is made of, have been
>>> done. That's what we care about here, right? The sqe has either been
>>> turned into an io request and has been submitted, or it has been copied.
>>
>> But they might not actually be done. AFAIU the CPU is allowed to do
>> the WRITE_ONCE of the head before doing any of the reads from the sqe
>> - loads and stores you do, as observed by a concurrently executing
>> thread, can happen in an order independent of the order in which you
>> write them in your code unless you use memory barriers. So the CPU
>> might decide to first write the new head, then do the read for
>> io_get_sqring(), and then do the __io_submit_sqe(), potentially
>> reading e.g. a IORING_OP_NOP opcode that has been written by
>> concurrently executing userspace after userspace has observed the
>> bumped head.
>
> For that to be possible, we'd need NO ordering in between the IO
> submission and when we write the sq ring head. A single spin lock
> should do it, right?
>
> It's not that I'm set against adding an smp_mb() to io_commit_sqring(),
> but I think we're going off the deep end a little bit here on
> theoretical vs what can practically happen.
>
> For the regular IO cases, we will have done at least one lock/unlock
> cycle. This is true for nops as well, and poll. The only case that could
> potentially NOT have one is the fsync, for the case where we punt and
> don't add it to existing work, we don't have any locking in between.
>
> I'll add the smp_mb() for peace of mind.
For reference, folded in:
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8d68569f9ba9..755ff8f411da 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1690,6 +1690,13 @@ static void io_commit_sqring(struct io_ring_ctx *ctx)
struct io_sq_ring *ring = ctx->sq_ring;
if (ctx->cached_sq_head != READ_ONCE(ring->r.head)) {
+ /*
+ * Ensure any loads from the SQEs are done at this point,
+ * since once we write the new head, the application could
+ * write new data to them.
+ */
+ smp_mb();
+
WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
/*
* write side barrier of head update, app has read side. See
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply related
* Re: [PATCH 05/19] Add io_uring IO interface
From: Andy Lutomirski @ 2019-02-13 0:07 UTC (permalink / raw)
To: Jens Axboe
Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer,
Avi Kivity, Al Viro
In-Reply-To: <7452c409-f232-2017-9101-0cd6c6946d64@kernel.dk>
> On Feb 12, 2019, at 3:53 PM, Jens Axboe <axboe@kernel.dk> wrote:
>
>> On 2/12/19 4:46 PM, Jens Axboe wrote:
>>> On 2/12/19 4:28 PM, Jann Horn wrote:
>>>> On Wed, Feb 13, 2019 at 12:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>>> On 2/12/19 4:11 PM, Jann Horn wrote:
>>>>>> On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>
>>>>>>> On 2/12/19 3:57 PM, Jann Horn wrote:
>>>>>>>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>
>>>>>>>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
>>>>>>>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
>>>>>>>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>>>>>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>>>>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>>>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>>>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>>>>>>>>>> arbitrary submission.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Two new system calls are added for this:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> io_uring_setup(entries, params)
>>>>>>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>>>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>>>>>>>>>> kernel to return already completed events without waiting
>>>>>>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>>>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>>>>>>>>>> without entering the kernel.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>>>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>>>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>>>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>>>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>>>>>>>>>> as a sync interface.
>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>> + struct io_kiocb *req;
>>>>>>>>>>>>>>>> + ssize_t ret;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>>>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> + req = io_get_req(ctx);
>>>>>>>>>>>>>>>> + if (unlikely(!req))
>>>>>>>>>>>>>>>> + return -EAGAIN;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>>>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>> + if (ret)
>>>>>>>>>>>>>>>> + io_free_req(req);
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>>>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>>>>>>>>>> + smp_wmb();
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>>>>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>>>>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The matching read barrier is in the application, it must do that before
>>>>>>>>>>>>>> reading ->head for the SQ ring.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>>>>>>>>>> that should be all we need to ensure that loads are done.
>>>>>>>>>>>>>
>>>>>>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>>>>>>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>>>>>>>>>> only compiler barriers, influencing the order in which the compiler
>>>>>>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>>>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>>>>>>>>>
>>>>>>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>>>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>>>>>>>>>> no *hardware* memory barrier that prevents reordering against
>>>>>>>>>>>>> concurrently running userspace code. As far as I can tell, the
>>>>>>>>>>>>> following could happen:
>>>>>>>>>>>>>
>>>>>>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>>>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>>>>>>>>>> such that the write to the head becomes visible before the read from
>>>>>>>>>>>>> ring->array has completed.
>>>>>>>>>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>>>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>>>>>>>>>> kernel reads from ring->array.
>>>>>>>>>>>>
>>>>>>>>>>>> I'd say this is highly theoretical for the normal use case, as we
>>>>>>>>>>>> will have submitted IO in between. Hence the load must have been done.
>>>>>>>>>>
>>>>>>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>>>>>>>>>> io_sq_thread() goes directly from io_get_sqring() to
>>>>>>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>>>>>>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>>>>>>>>>> "submitting IO" in the middle.
>>>>>>>>>
>>>>>>>>> You are right, the patch I sent IS needed for the sq thread case! It's
>>>>>>>>> only true for the "normal" case that we don't need the smp_mb() before
>>>>>>>>> writing the sq ring head, as sqes are fully consumed at that point.
>>>>>>>
>>>>>>> Hmm... does that actually matter? As long as you don't have an
>>>>>>> explicit barrier for this, the CPU could still reorder things, right?
>>>>>>> Pull the store in front of everything else?
>>>>>>
>>>>>> If the IO has been submitted, by definition the loads have completed.
>>>>>> At that point it should be fine to commit the ring head that the
>>>>>> application sees.
>>>>>
>>>>> What exactly do you mean by "the IO has been submitted"? Are you
>>>>> talking about interaction with hardware, or about the end of the
>>>>> syscall, or something else?
>>>>
>>>> I mean that the loads from the sqe, which the IO is made of, have been
>>>> done. That's what we care about here, right? The sqe has either been
>>>> turned into an io request and has been submitted, or it has been copied.
>>>
>>> But they might not actually be done. AFAIU the CPU is allowed to do
>>> the WRITE_ONCE of the head before doing any of the reads from the sqe
>>> - loads and stores you do, as observed by a concurrently executing
>>> thread, can happen in an order independent of the order in which you
>>> write them in your code unless you use memory barriers. So the CPU
>>> might decide to first write the new head, then do the read for
>>> io_get_sqring(), and then do the __io_submit_sqe(), potentially
>>> reading e.g. a IORING_OP_NOP opcode that has been written by
>>> concurrently executing userspace after userspace has observed the
>>> bumped head.
>>
>> For that to be possible, we'd need NO ordering in between the IO
>> submission and when we write the sq ring head. A single spin lock
>> should do it, right?
>>
>> It's not that I'm set against adding an smp_mb() to io_commit_sqring(),
>> but I think we're going off the deep end a little bit here on
>> theoretical vs what can practically happen.
>>
>> For the regular IO cases, we will have done at least one lock/unlock
>> cycle. This is true for nops as well, and poll. The only case that could
>> potentially NOT have one is the fsync, for the case where we punt and
>> don't add it to existing work, we don't have any locking in between.
>>
>> I'll add the smp_mb() for peace of mind.
>
> For reference, folded in:
>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 8d68569f9ba9..755ff8f411da 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1690,6 +1690,13 @@ static void io_commit_sqring(struct io_ring_ctx *ctx)
> struct io_sq_ring *ring = ctx->sq_ring;
>
> if (ctx->cached_sq_head != READ_ONCE(ring->r.head)) {
> + /*
> + * Ensure any loads from the SQEs are done at this point,
> + * since once we write the new head, the application could
> + * write new data to them.
> + */
> + smp_mb();
> +
> WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> /*
> * write side barrier of head update, app has read side. See
>
>
I haven’t followed the full set of machinations here, but would smp_store_release() be sufficient? It is a *lot* faster on some architectures.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jann Horn @ 2019-02-13 0:14 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jens Axboe, linux-aio, linux-block, Linux API, hch, jmoyer,
Avi Kivity, Al Viro
In-Reply-To: <08141EFF-E805-405B-9970-ABFE9A1C3F58@amacapital.net>
On Wed, Feb 13, 2019 at 1:07 AM Andy Lutomirski <luto@amacapital.net> wrote:
> > On Feb 12, 2019, at 3:53 PM, Jens Axboe <axboe@kernel.dk> wrote:
> >> On 2/12/19 4:46 PM, Jens Axboe wrote:
> >>> On 2/12/19 4:28 PM, Jann Horn wrote:
> >>>> On Wed, Feb 13, 2019 at 12:19 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>> On 2/12/19 4:11 PM, Jann Horn wrote:
> >>>>>> On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>> On 2/12/19 3:57 PM, Jann Horn wrote:
> >>>>>>>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
> >>>>>>>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
> >>>>>>>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
> >>>>>>>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
> >>>>>>>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
> >>>>>>>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
> >>>>>>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
> >>>>>>>>>>>>>>>> between the application and the kernel. This eliminates the need to
> >>>>>>>>>>>>>>>> copy data back and forth to submit and complete IO.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
> >>>>>>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
> >>>>>>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
> >>>>>>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
> >>>>>>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
> >>>>>>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
> >>>>>>>>>>>>>>>> arbitrary submission.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Two new system calls are added for this:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> io_uring_setup(entries, params)
> >>>>>>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
> >>>>>>>>>>>>>>>> returns a file descriptor that the application can mmap to
> >>>>>>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
> >>>>>>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
> >>>>>>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
> >>>>>>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
> >>>>>>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
> >>>>>>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
> >>>>>>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
> >>>>>>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
> >>>>>>>>>>>>>>>> kernel to return already completed events without waiting
> >>>>>>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
> >>>>>>>>>>>>>>>> driven IO, the application can just check the CQ ring
> >>>>>>>>>>>>>>>> without entering the kernel.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
> >>>>>>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
> >>>>>>>>>>>>>>>> and polled submission as well. The latter will enable an application
> >>>>>>>>>>>>>>>> to do IO without doing ANY system calls at all.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
> >>>>>>>>>>>>>>>> completions if it wants to wait for them to occur.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
> >>>>>>>>>>>>>>>> as well. We will only punt to an async context if the command would
> >>>>>>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
> >>>>>>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
> >>>>>>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
> >>>>>>>>>>>>>>>> as a sync interface.
> >>>>>>>>>>>>> [...]
> >>>>>>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
> >>>>>>>>>>>>>>>> +{
> >>>>>>>>>>>>>>>> + struct io_kiocb *req;
> >>>>>>>>>>>>>>>> + ssize_t ret;
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> + /* enforce forwards compatibility on users */
> >>>>>>>>>>>>>>>> + if (unlikely(s->sqe->flags))
> >>>>>>>>>>>>>>>> + return -EINVAL;
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> + req = io_get_req(ctx);
> >>>>>>>>>>>>>>>> + if (unlikely(!req))
> >>>>>>>>>>>>>>>> + return -EAGAIN;
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> + req->rw.ki_filp = NULL;
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
> >>>>>>>>>>>>>>>> + if (ret == -EAGAIN) {
> >>>>>>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
> >>>>>>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
> >>>>>>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
> >>>>>>>>>>>>>>>> + ret = 0;
> >>>>>>>>>>>>>>>> + }
> >>>>>>>>>>>>>>>> + if (ret)
> >>>>>>>>>>>>>>>> + io_free_req(req);
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> + return ret;
> >>>>>>>>>>>>>>>> +}
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
> >>>>>>>>>>>>>>>> +{
> >>>>>>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
> >>>>>>>>>>>>>>>> +
> >>>>>>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
> >>>>>>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> >>>>>>>>>>>>>>>> + /* write side barrier of head update, app has read side */
> >>>>>>>>>>>>>>>> + smp_wmb();
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
> >>>>>>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
> >>>>>>>>>>>>>>> nobody sees the updated head before you're done reading the submission
> >>>>>>>>>>>>>>> queue entry? Or is that barrier elsewhere?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> The matching read barrier is in the application, it must do that before
> >>>>>>>>>>>>>> reading ->head for the SQ ring.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
> >>>>>>>>>>>>>> that should be all we need to ensure that loads are done.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
> >>>>>>>>>>>>> ordering with regard to concurrent execution on other cores. They are
> >>>>>>>>>>>>> only compiler barriers, influencing the order in which the compiler
> >>>>>>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
> >>>>>>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
> >>>>>>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
> >>>>>>>>>>>>> no *hardware* memory barrier that prevents reordering against
> >>>>>>>>>>>>> concurrently running userspace code. As far as I can tell, the
> >>>>>>>>>>>>> following could happen:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
> >>>>>>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
> >>>>>>>>>>>>> such that the write to the head becomes visible before the read from
> >>>>>>>>>>>>> ring->array has completed.
> >>>>>>>>>>>>> - Userspace observes the write to the head and reuses the array slots
> >>>>>>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
> >>>>>>>>>>>>> kernel reads from ring->array.
> >>>>>>>>>>>>
> >>>>>>>>>>>> I'd say this is highly theoretical for the normal use case, as we
> >>>>>>>>>>>> will have submitted IO in between. Hence the load must have been done.
> >>>>>>>>>>
> >>>>>>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
> >>>>>>>>>> io_sq_thread() goes directly from io_get_sqring() to
> >>>>>>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
> >>>>>>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
> >>>>>>>>>> "submitting IO" in the middle.
> >>>>>>>>>
> >>>>>>>>> You are right, the patch I sent IS needed for the sq thread case! It's
> >>>>>>>>> only true for the "normal" case that we don't need the smp_mb() before
> >>>>>>>>> writing the sq ring head, as sqes are fully consumed at that point.
> >>>>>>>
> >>>>>>> Hmm... does that actually matter? As long as you don't have an
> >>>>>>> explicit barrier for this, the CPU could still reorder things, right?
> >>>>>>> Pull the store in front of everything else?
> >>>>>>
> >>>>>> If the IO has been submitted, by definition the loads have completed.
> >>>>>> At that point it should be fine to commit the ring head that the
> >>>>>> application sees.
> >>>>>
> >>>>> What exactly do you mean by "the IO has been submitted"? Are you
> >>>>> talking about interaction with hardware, or about the end of the
> >>>>> syscall, or something else?
> >>>>
> >>>> I mean that the loads from the sqe, which the IO is made of, have been
> >>>> done. That's what we care about here, right? The sqe has either been
> >>>> turned into an io request and has been submitted, or it has been copied.
> >>>
> >>> But they might not actually be done. AFAIU the CPU is allowed to do
> >>> the WRITE_ONCE of the head before doing any of the reads from the sqe
> >>> - loads and stores you do, as observed by a concurrently executing
> >>> thread, can happen in an order independent of the order in which you
> >>> write them in your code unless you use memory barriers. So the CPU
> >>> might decide to first write the new head, then do the read for
> >>> io_get_sqring(), and then do the __io_submit_sqe(), potentially
> >>> reading e.g. a IORING_OP_NOP opcode that has been written by
> >>> concurrently executing userspace after userspace has observed the
> >>> bumped head.
> >>
> >> For that to be possible, we'd need NO ordering in between the IO
> >> submission and when we write the sq ring head. A single spin lock
> >> should do it, right?
> >>
> >> It's not that I'm set against adding an smp_mb() to io_commit_sqring(),
> >> but I think we're going off the deep end a little bit here on
> >> theoretical vs what can practically happen.
> >>
> >> For the regular IO cases, we will have done at least one lock/unlock
> >> cycle. This is true for nops as well, and poll. The only case that could
> >> potentially NOT have one is the fsync, for the case where we punt and
> >> don't add it to existing work, we don't have any locking in between.
> >>
> >> I'll add the smp_mb() for peace of mind.
> >
> > For reference, folded in:
> >
> >
> > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > index 8d68569f9ba9..755ff8f411da 100644
> > --- a/fs/io_uring.c
> > +++ b/fs/io_uring.c
> > @@ -1690,6 +1690,13 @@ static void io_commit_sqring(struct io_ring_ctx *ctx)
> > struct io_sq_ring *ring = ctx->sq_ring;
> >
> > if (ctx->cached_sq_head != READ_ONCE(ring->r.head)) {
> > + /*
> > + * Ensure any loads from the SQEs are done at this point,
> > + * since once we write the new head, the application could
> > + * write new data to them.
> > + */
> > + smp_mb();
> > +
> > WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
> > /*
> > * write side barrier of head update, app has read side. See
> >
> >
>
> I haven’t followed the full set of machinations here, but would smp_store_release() be sufficient? It is a *lot* faster on some architectures.
Ah, yeah, that should work... I forgot that that exists.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* Re: [PATCH 05/19] Add io_uring IO interface
From: Jens Axboe @ 2019-02-13 0:24 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jann Horn, linux-aio, linux-block, Linux API, hch, jmoyer,
Avi Kivity, Al Viro
In-Reply-To: <08141EFF-E805-405B-9970-ABFE9A1C3F58@amacapital.net>
On 2/12/19 5:07 PM, Andy Lutomirski wrote:
>
>
>> On Feb 12, 2019, at 3:53 PM, Jens Axboe <axboe@kernel.dk> wrote:
>>
>>> On 2/12/19 4:46 PM, Jens Axboe wrote:
>>>> On 2/12/19 4:28 PM, Jann Horn wrote:
>>>>> On Wed, Feb 13, 2019 at 12:19 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>
>>>>>> On 2/12/19 4:11 PM, Jann Horn wrote:
>>>>>>> On Wed, Feb 13, 2019 at 12:00 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>
>>>>>>>> On 2/12/19 3:57 PM, Jann Horn wrote:
>>>>>>>>> On Tue, Feb 12, 2019 at 11:52 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>
>>>>>>>>>> On 2/12/19 3:45 PM, Jens Axboe wrote:
>>>>>>>>>>> On 2/12/19 3:40 PM, Jann Horn wrote:
>>>>>>>>>>>> On Tue, Feb 12, 2019 at 11:06 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> On 2/12/19 3:03 PM, Jens Axboe wrote:
>>>>>>>>>>>>>> On 2/12/19 2:42 PM, Jann Horn wrote:
>>>>>>>>>>>>>>> On Sat, Feb 9, 2019 at 5:15 AM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>>>>> On 2/8/19 3:12 PM, Jann Horn wrote:
>>>>>>>>>>>>>>>>> On Fri, Feb 8, 2019 at 6:34 PM Jens Axboe <axboe@kernel.dk> wrote:
>>>>>>>>>>>>>>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>>>>>>>>>>>>>>> between the application and the kernel. This eliminates the need to
>>>>>>>>>>>>>>>>> copy data back and forth to submit and complete IO.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> IO submissions use the io_uring_sqe data structure, and completions
>>>>>>>>>>>>>>>>> are generated in the form of io_uring_cqe data structures. The SQ
>>>>>>>>>>>>>>>>> ring is an index into the io_uring_sqe array, which makes it possible
>>>>>>>>>>>>>>>>> to submit a batch of IOs without them being contiguous in the ring.
>>>>>>>>>>>>>>>>> The CQ ring is always contiguous, as completion events are inherently
>>>>>>>>>>>>>>>>> unordered, and hence any io_uring_cqe entry can point back to an
>>>>>>>>>>>>>>>>> arbitrary submission.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Two new system calls are added for this:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> io_uring_setup(entries, params)
>>>>>>>>>>>>>>>>> Sets up an io_uring instance for doing async IO. On success,
>>>>>>>>>>>>>>>>> returns a file descriptor that the application can mmap to
>>>>>>>>>>>>>>>>> gain access to the SQ ring, CQ ring, and io_uring_sqes.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> io_uring_enter(fd, to_submit, min_complete, flags, sigset, sigsetsize)
>>>>>>>>>>>>>>>>> Initiates IO against the rings mapped to this fd, or waits for
>>>>>>>>>>>>>>>>> them to complete, or both. The behavior is controlled by the
>>>>>>>>>>>>>>>>> parameters passed in. If 'to_submit' is non-zero, then we'll
>>>>>>>>>>>>>>>>> try and submit new IO. If IORING_ENTER_GETEVENTS is set, the
>>>>>>>>>>>>>>>>> kernel will wait for 'min_complete' events, if they aren't
>>>>>>>>>>>>>>>>> already available. It's valid to set IORING_ENTER_GETEVENTS
>>>>>>>>>>>>>>>>> and 'min_complete' == 0 at the same time, this allows the
>>>>>>>>>>>>>>>>> kernel to return already completed events without waiting
>>>>>>>>>>>>>>>>> for them. This is useful only for polling, as for IRQ
>>>>>>>>>>>>>>>>> driven IO, the application can just check the CQ ring
>>>>>>>>>>>>>>>>> without entering the kernel.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> With this setup, it's possible to do async IO with a single system
>>>>>>>>>>>>>>>>> call. Future developments will enable polled IO with this interface,
>>>>>>>>>>>>>>>>> and polled submission as well. The latter will enable an application
>>>>>>>>>>>>>>>>> to do IO without doing ANY system calls at all.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> For IRQ driven IO, an application only needs to enter the kernel for
>>>>>>>>>>>>>>>>> completions if it wants to wait for them to occur.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Each io_uring is backed by a workqueue, to support buffered async IO
>>>>>>>>>>>>>>>>> as well. We will only punt to an async context if the command would
>>>>>>>>>>>>>>>>> need to wait for IO on the device side. Any data that can be accessed
>>>>>>>>>>>>>>>>> directly in the page cache is done inline. This avoids the slowness
>>>>>>>>>>>>>>>>> issue of usual threadpools, since cached data is accessed as quickly
>>>>>>>>>>>>>>>>> as a sync interface.
>>>>>>>>>>>>>> [...]
>>>>>>>>>>>>>>>>> +static int io_submit_sqe(struct io_ring_ctx *ctx, const struct sqe_submit *s)
>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>> + struct io_kiocb *req;
>>>>>>>>>>>>>>>>> + ssize_t ret;
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> + /* enforce forwards compatibility on users */
>>>>>>>>>>>>>>>>> + if (unlikely(s->sqe->flags))
>>>>>>>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> + req = io_get_req(ctx);
>>>>>>>>>>>>>>>>> + if (unlikely(!req))
>>>>>>>>>>>>>>>>> + return -EAGAIN;
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> + req->rw.ki_filp = NULL;
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> + ret = __io_submit_sqe(ctx, req, s, true);
>>>>>>>>>>>>>>>>> + if (ret == -EAGAIN) {
>>>>>>>>>>>>>>>>> + memcpy(&req->submit, s, sizeof(*s));
>>>>>>>>>>>>>>>>> + INIT_WORK(&req->work, io_sq_wq_submit_work);
>>>>>>>>>>>>>>>>> + queue_work(ctx->sqo_wq, &req->work);
>>>>>>>>>>>>>>>>> + ret = 0;
>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>> + if (ret)
>>>>>>>>>>>>>>>>> + io_free_req(req);
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> + return ret;
>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> +static void io_commit_sqring(struct io_ring_ctx *ctx)
>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>> + struct io_sq_ring *ring = ctx->sq_ring;
>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>> + if (ctx->cached_sq_head != ring->r.head) {
>>>>>>>>>>>>>>>>> + WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>>>>>>>>>>>>>>>>> + /* write side barrier of head update, app has read side */
>>>>>>>>>>>>>>>>> + smp_wmb();
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Can you elaborate on what this memory barrier is doing? Don't you need
>>>>>>>>>>>>>>>> some sort of memory barrier *before* the WRITE_ONCE(), to ensure that
>>>>>>>>>>>>>>>> nobody sees the updated head before you're done reading the submission
>>>>>>>>>>>>>>>> queue entry? Or is that barrier elsewhere?
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The matching read barrier is in the application, it must do that before
>>>>>>>>>>>>>>> reading ->head for the SQ ring.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> For the other barrier, since the ring->r.head now has a READ_ONCE(),
>>>>>>>>>>>>>>> that should be all we need to ensure that loads are done.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> READ_ONCE() / WRITE_ONCE are not hardware memory barriers that enforce
>>>>>>>>>>>>>> ordering with regard to concurrent execution on other cores. They are
>>>>>>>>>>>>>> only compiler barriers, influencing the order in which the compiler
>>>>>>>>>>>>>> emits things. (Well, unless you're on alpha, where READ_ONCE() implies
>>>>>>>>>>>>>> a memory barrier that prevents reordering of dependent reads.)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> As far as I can tell, between the READ_ONCE(ring->array[...]) in
>>>>>>>>>>>>>> io_get_sqring() and the WRITE_ONCE() in io_commit_sqring(), you have
>>>>>>>>>>>>>> no *hardware* memory barrier that prevents reordering against
>>>>>>>>>>>>>> concurrently running userspace code. As far as I can tell, the
>>>>>>>>>>>>>> following could happen:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> - The kernel reads from ring->array in io_get_sqring(), then updates
>>>>>>>>>>>>>> the head in io_commit_sqring(). The CPU reorders the memory accesses
>>>>>>>>>>>>>> such that the write to the head becomes visible before the read from
>>>>>>>>>>>>>> ring->array has completed.
>>>>>>>>>>>>>> - Userspace observes the write to the head and reuses the array slots
>>>>>>>>>>>>>> the kernel has freed with the write, clobbering ring->array before the
>>>>>>>>>>>>>> kernel reads from ring->array.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'd say this is highly theoretical for the normal use case, as we
>>>>>>>>>>>>> will have submitted IO in between. Hence the load must have been done.
>>>>>>>>>>>
>>>>>>>>>>> Sorry, I'm confused. Who is "we", and which load are you referring to?
>>>>>>>>>>> io_sq_thread() goes directly from io_get_sqring() to
>>>>>>>>>>> io_commit_sqring(), with only a conditional io_sqe_needs_user() in
>>>>>>>>>>> between, if the `i == ARRAY_SIZE(sqes)` check triggers. There is no
>>>>>>>>>>> "submitting IO" in the middle.
>>>>>>>>>>
>>>>>>>>>> You are right, the patch I sent IS needed for the sq thread case! It's
>>>>>>>>>> only true for the "normal" case that we don't need the smp_mb() before
>>>>>>>>>> writing the sq ring head, as sqes are fully consumed at that point.
>>>>>>>>
>>>>>>>> Hmm... does that actually matter? As long as you don't have an
>>>>>>>> explicit barrier for this, the CPU could still reorder things, right?
>>>>>>>> Pull the store in front of everything else?
>>>>>>>
>>>>>>> If the IO has been submitted, by definition the loads have completed.
>>>>>>> At that point it should be fine to commit the ring head that the
>>>>>>> application sees.
>>>>>>
>>>>>> What exactly do you mean by "the IO has been submitted"? Are you
>>>>>> talking about interaction with hardware, or about the end of the
>>>>>> syscall, or something else?
>>>>>
>>>>> I mean that the loads from the sqe, which the IO is made of, have been
>>>>> done. That's what we care about here, right? The sqe has either been
>>>>> turned into an io request and has been submitted, or it has been copied.
>>>>
>>>> But they might not actually be done. AFAIU the CPU is allowed to do
>>>> the WRITE_ONCE of the head before doing any of the reads from the sqe
>>>> - loads and stores you do, as observed by a concurrently executing
>>>> thread, can happen in an order independent of the order in which you
>>>> write them in your code unless you use memory barriers. So the CPU
>>>> might decide to first write the new head, then do the read for
>>>> io_get_sqring(), and then do the __io_submit_sqe(), potentially
>>>> reading e.g. a IORING_OP_NOP opcode that has been written by
>>>> concurrently executing userspace after userspace has observed the
>>>> bumped head.
>>>
>>> For that to be possible, we'd need NO ordering in between the IO
>>> submission and when we write the sq ring head. A single spin lock
>>> should do it, right?
>>>
>>> It's not that I'm set against adding an smp_mb() to io_commit_sqring(),
>>> but I think we're going off the deep end a little bit here on
>>> theoretical vs what can practically happen.
>>>
>>> For the regular IO cases, we will have done at least one lock/unlock
>>> cycle. This is true for nops as well, and poll. The only case that could
>>> potentially NOT have one is the fsync, for the case where we punt and
>>> don't add it to existing work, we don't have any locking in between.
>>>
>>> I'll add the smp_mb() for peace of mind.
>>
>> For reference, folded in:
>>
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 8d68569f9ba9..755ff8f411da 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -1690,6 +1690,13 @@ static void io_commit_sqring(struct io_ring_ctx *ctx)
>> struct io_sq_ring *ring = ctx->sq_ring;
>>
>> if (ctx->cached_sq_head != READ_ONCE(ring->r.head)) {
>> + /*
>> + * Ensure any loads from the SQEs are done at this point,
>> + * since once we write the new head, the application could
>> + * write new data to them.
>> + */
>> + smp_mb();
>> +
>> WRITE_ONCE(ring->r.head, ctx->cached_sq_head);
>> /*
>> * write side barrier of head update, app has read side. See
>>
>>
>
> I haven’t followed the full set of machinations here, but would
> smp_store_release() be sufficient? It is a *lot* faster on some
> architectures.
Thanks for the hint, yes that looks more appropriate.
--
Jens Axboe
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org. For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>
^ permalink raw reply
* [PATCH v5 0/5] namei: vfs flags to restrict path resolution
From: Aleksa Sarai @ 2019-02-13 3:08 UTC (permalink / raw)
To: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells
Cc: Aleksa Sarai, Eric Biederman, Andy Lutomirski, Jann Horn,
Christian Brauner, David Drysdale, Tycho Andersen, Kees Cook,
containers, linux-fsdevel, linux-api, Andrew Morton,
Alexei Starovoitov, Chanho Min, Oleg Nesterov, Aleksa Sarai,
linux-kernel, linux-arch
Now that the holiday break is over, it's time to re-send this patch
series (with a few additions, due to new information we got from
CVE-2019-5736 -- which this patchset mostly protected against but had
some holes with regards to #!-style scripts).
Patch changelog:
v5:
* In response to CVE-2019-5736 (one of the vectors showed that
open(2)+fexec(3) cannot be used to scope binfmt_script's implicit
open_exec()), AT_* flags have been re-added and are now piped
through to binfmt_script (and other binfmt_* that use open_exec)
but are only supported for execveat(2) for now.
v4:
* Remove AT_* flag reservations, as they require more discussion.
* Switch to path_is_under() over __d_path() for breakout checking.
* Make O_XDEV no longer block openat("/tmp", "/", O_XDEV) -- dirfd
is now ignored for absolute paths to match other flags.
* Improve the dirfd_path_init() refactor and move it to a separate
commit.
* Remove reference to Linux-capsicum.
* Switch "proclink" name to "magic link".
v3: [resend]
v2:
* Made ".." resolution with AT_THIS_ROOT and AT_BENEATH safe(r) with
some semi-aggressive __d_path checking (see patch 3).
* Disallowed "proclinks" with AT_THIS_ROOT and AT_BENEATH, in the
hopes they can be re-enabled once safe.
* Removed the selftests as they will be reimplemented as xfstests.
* Removed stat(2) support, since you can already get it through
O_PATH and fstatat(2).
The need for some sort of control over VFS's path resolution (to avoid
malicious paths resulting in inadvertent breakouts) has been a very
long-standing desire of many userspace applications. This patchset is a
revival of Al Viro's old AT_NO_JUMPS[1,2] patchset (which was a variant
of David Drysdale's O_BENEATH patchset[3] which was a spin-off of the
Capsicum project[4]) with a few additions and changes made based on the
previous discussion within [5] as well as others I felt were useful.
In line with the conclusions of the original discussion of AT_NO_JUMPS,
the flag has been split up into separate flags:
* O_XDEV blocks all mountpoint crossings (upwards, downwards, or
through absolute links). Absolute pathnames alone in openat(2) do
not trigger this.
* O_NOMAGICLINKS blocks resolution through /proc/$pid/fd-style links.
This is done by blocking the usage of nd_jump_link() during
resolution in a filesystem. The term "magic links" is used to match
with the only reference to these links in Documentation/, but I'm
happy to change the name.
It should be noted that this is different to the scope of O_NOFOLLOW
in that it applies to all path components. However, you can do
open(O_NOFOLLOW|O_NOMAGICLINKS|O_PATH) on a "magic link" and it will
*not* fail (assuming that no parent component was a "magic link"),
and you will have an fd for the "magic link".
* O_BENEATH disallows escapes to outside the starting dirfd's tree,
using techniques such as ".." or absolute links. Absolute paths in
openat(2) are also disallowed. Conceptually this flag is to ensure
you "stay below" a certain point in the filesystem tree -- but this
requires some additional to protect against various races that would
allow escape using ".." (see patch 4 for more detail).
Currently O_BENEATH implies O_NOMAGICLINKS, because it can trivially
beam you around the filesystem (breaking the protection). In future,
there might be similar safety checks as in patch 4, but that
requires more discussion.
In addition, two new flags were added that expand on the above ideas:
* O_NOSYMLINKS does what it says on the tin. No symlink resolution is
allowed at all, including "magic links". Just as with O_NOMAGICLINKS
this can still be used with (O_PATH|O_NOFOLLOW) to open an fd for
the symlink as long as no parent path had a symlink component.
* O_THISROOT is an extension of O_BENEATH that, rather than blocking
attempts to move past the root, forces all such movements to be
scoped to the starting point. This provides chroot(2)-like
protection but without the cost of a chroot(2) for each filesystem
operation, as well as being safe against race attacks that chroot(2)
is not.
If a race is detected (as with O_BENEATH) then an error is
generated, and similar to O_BENEATH it is not permitted to cross
"magic links" with O_THISROOT.
The primary need for this is from container runtimes, which
currently need to do symlink scoping in userspace[6] when opening
paths in a potentially malicious container. There is a long list of
CVEs that could have bene mitigated by having O_THISROOT (such as
CVE-2017-1002101, CVE-2017-1002102, CVE-2018-15664, and
CVE-2019-5736, just to name a few).
In addition, a mirror set of AT_* flags have been added (though
currently these are only supported for execveat(2) -- and not for any
other syscall). The need for these is explained in the final patch in
the series (it's motivated by CVE-2019-5736).
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: David Drysdale <drysdale@google.com>
Cc: Tycho Andersen <tycho@tycho.ws>
Cc: Kees Cook <keescook@chromium.org>
Cc: <containers@lists.linux-foundation.org>
Cc: <linux-fsdevel@vger.kernel.org>
Cc: <linux-api@vger.kernel.org>
[1]: https://lwn.net/Articles/721443/
[2]: https://lore.kernel.org/patchwork/patch/784221/
[3]: https://lwn.net/Articles/619151/
[4]: https://lwn.net/Articles/603929/
[5]: https://lwn.net/Articles/723057/
[6]: https://github.com/cyphar/filepath-securejoin
Aleksa Sarai (5):
namei: split out nd->dfd handling to dirfd_path_init
namei: O_BENEATH-style path resolution flags
namei: O_THISROOT: chroot-like path resolution
namei: aggressively check for nd->root escape on ".." resolution
binfmt_*: scope path resolution of interpreters
fs/binfmt_elf.c | 2 +-
fs/binfmt_elf_fdpic.c | 2 +-
fs/binfmt_em86.c | 4 +-
fs/binfmt_misc.c | 2 +-
fs/binfmt_script.c | 2 +-
fs/exec.c | 26 +++-
fs/fcntl.c | 2 +-
fs/namei.c | 205 ++++++++++++++++++++++---------
fs/open.c | 13 +-
include/linux/binfmts.h | 1 +
include/linux/fcntl.h | 3 +-
include/linux/fs.h | 9 +-
include/linux/namei.h | 8 ++
include/uapi/asm-generic/fcntl.h | 17 +++
include/uapi/linux/fcntl.h | 6 +
15 files changed, 228 insertions(+), 74 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH v5 1/5] namei: split out nd->dfd handling to dirfd_path_init
From: Aleksa Sarai @ 2019-02-13 3:08 UTC (permalink / raw)
To: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells
Cc: Aleksa Sarai, Eric Biederman, Andy Lutomirski, Andrew Morton,
Alexei Starovoitov, Kees Cook, Jann Horn, Christian Brauner,
David Drysdale, Chanho Min, Oleg Nesterov, Aleksa Sarai,
containers, linux-fsdevel, linux-api, linux-kernel, linux-arch
In-Reply-To: <20190213030851.1881-1-cyphar@cyphar.com>
Previously, path_init's handling of *at(dfd, ...) was only done once,
but with O_BENEATH (and O_THISROOT) we have to parse the initial
nd->path at different times (before or after absolute path handling)
depending on whether we have been asked to scope resolution within a
root.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
fs/namei.c | 103 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 59 insertions(+), 44 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index a85deb55d0c9..4fdcb36f7c01 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2168,9 +2168,59 @@ static int link_path_walk(const char *name, struct nameidata *nd)
}
}
+/*
+ * Configure nd->path based on the nd->dfd. This is only used as part of
+ * path_init().
+ */
+static inline int dirfd_path_init(struct nameidata *nd)
+{
+ if (nd->dfd == AT_FDCWD) {
+ if (nd->flags & LOOKUP_RCU) {
+ struct fs_struct *fs = current->fs;
+ unsigned seq;
+
+ do {
+ seq = read_seqcount_begin(&fs->seq);
+ nd->path = fs->pwd;
+ nd->inode = nd->path.dentry->d_inode;
+ nd->seq = __read_seqcount_begin(&nd->path.dentry->d_seq);
+ } while (read_seqcount_retry(&fs->seq, seq));
+ } else {
+ get_fs_pwd(current->fs, &nd->path);
+ nd->inode = nd->path.dentry->d_inode;
+ }
+ } else {
+ /* Caller must check execute permissions on the starting path component */
+ struct fd f = fdget_raw(nd->dfd);
+ struct dentry *dentry;
+
+ if (!f.file)
+ return -EBADF;
+
+ dentry = f.file->f_path.dentry;
+
+ if (*nd->name->name && unlikely(!d_can_lookup(dentry))) {
+ fdput(f);
+ return -ENOTDIR;
+ }
+
+ nd->path = f.file->f_path;
+ if (nd->flags & LOOKUP_RCU) {
+ nd->inode = nd->path.dentry->d_inode;
+ nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
+ } else {
+ path_get(&nd->path);
+ nd->inode = nd->path.dentry->d_inode;
+ }
+ fdput(f);
+ }
+ return 0;
+}
+
/* must be paired with terminate_walk() */
static const char *path_init(struct nameidata *nd, unsigned flags)
{
+ int error;
const char *s = nd->name->name;
if (!*s)
@@ -2204,52 +2254,17 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
nd->m_seq = read_seqbegin(&mount_lock);
if (*s == '/') {
- set_root(nd);
- if (likely(!nd_jump_root(nd)))
- return s;
- return ERR_PTR(-ECHILD);
- } else if (nd->dfd == AT_FDCWD) {
- if (flags & LOOKUP_RCU) {
- struct fs_struct *fs = current->fs;
- unsigned seq;
-
- do {
- seq = read_seqcount_begin(&fs->seq);
- nd->path = fs->pwd;
- nd->inode = nd->path.dentry->d_inode;
- nd->seq = __read_seqcount_begin(&nd->path.dentry->d_seq);
- } while (read_seqcount_retry(&fs->seq, seq));
- } else {
- get_fs_pwd(current->fs, &nd->path);
- nd->inode = nd->path.dentry->d_inode;
- }
- return s;
- } else {
- /* Caller must check execute permissions on the starting path component */
- struct fd f = fdget_raw(nd->dfd);
- struct dentry *dentry;
-
- if (!f.file)
- return ERR_PTR(-EBADF);
-
- dentry = f.file->f_path.dentry;
-
- if (*s && unlikely(!d_can_lookup(dentry))) {
- fdput(f);
- return ERR_PTR(-ENOTDIR);
- }
-
- nd->path = f.file->f_path;
- if (flags & LOOKUP_RCU) {
- nd->inode = nd->path.dentry->d_inode;
- nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
- } else {
- path_get(&nd->path);
- nd->inode = nd->path.dentry->d_inode;
- }
- fdput(f);
+ if (likely(!nd->root.mnt))
+ set_root(nd);
+ error = nd_jump_root(nd);
+ if (unlikely(error))
+ s = ERR_PTR(error);
return s;
}
+ error = dirfd_path_init(nd);
+ if (unlikely(error))
+ return ERR_PTR(error);
+ return s;
}
static const char *trailing_symlink(struct nameidata *nd)
--
2.20.1
^ permalink raw reply related
* [PATCH v5 2/5] namei: O_BENEATH-style path resolution flags
From: Aleksa Sarai @ 2019-02-13 3:08 UTC (permalink / raw)
To: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells
Cc: Aleksa Sarai, Eric Biederman, Christian Brauner, Kees Cook,
David Drysdale, Andy Lutomirski, Linus Torvalds, Andrew Morton,
Alexei Starovoitov, Jann Horn, Chanho Min, Oleg Nesterov,
Aleksa Sarai, containers, linux-fsdevel, linux-api, linux-kernel,
linux-arch
In-Reply-To: <20190213030851.1881-1-cyphar@cyphar.com>
Add the following flags to allow various restrictions on path
resolution (these affect the *entire* resolution, rather than just the
final path component -- as is the case with most other AT_* flags).
The primary justification for these flags is to allow for programs to be
far more strict about how they want path resolution to handle symlinks,
mountpoint crossings, and paths that escape the dirfd (through an
absolute path or ".." shenanigans).
This is of particular concern to container runtimes that want to be very
careful about malicious root filesystems that a container's init might
have screwed around with (and there is no real way to protect against
this in userspace if you consider potential races against a malicious
container's init). More classical applications (which have their own
potentially buggy userspace path sanitisation code) include web
servers, archive extraction tools, network file servers, and so on.
* O_XDEV: Disallow mount-point crossing (both *down* into one, or *up*
from one). The primary "scoping" use is to blocking resolution that
crosses a bind-mount, which has a similar property to a symlink (in
the way that it allows for escape from the starting-point). Since it
is not possible to differentiate bind-mounts However since
bind-mounting requires privileges (in ways symlinks don't) this has
been split from LOOKUP_BENEATH. The naming is based on "find -xdev"
(though find(1) doesn't walk upwards, the semantics seem obvious).
* O_NOMAGICLINKS: Disallows ->get_link "symlink" jumping. This is a very
specific restriction, and it exists because /proc/$pid/fd/...
"symlinks" allow for access outside nd->root and pose risk to
container runtimes that don't want to be tricked into accessing a host
path (but do want to allow no-funny-business symlink resolution).
* O_NOSYMLINKS: Disallows symlink jumping *of any kind*. Implies
O_NOMAGICLINKS (obviously).
* O_BENEATH: Disallow "escapes" from the starting point of the
filesystem tree during resolution (you must stay "beneath" the
starting point at all times). Currently this is done by disallowing
".." and absolute paths (either in the given path or found during
symlink resolution) entirely, as well as all "magic link" jumping.
The wholesale banning of ".." is because it is currently not safe to
allow ".." resolution (races can cause the path to be moved outside of
the root -- this is conceptually similar to historical chroot(2)
escape attacks). Future patches in this series will address this, and
will re-enable ".." resolution once it is safe. With those patches,
".." resolution will only be allowed if it remains in the root
throughout resolution (such as "a/../b" not "a/../../outside/b").
The banning of "magic link" jumping is done because it is not clear
whether semantically they should be allowed -- while some "magic
links" are safe there are many that can cause escapes (and once a
resolution is outside of the root, O_BENEATH will no longer detect
it). Future patches may re-enable "magic link" jumping when such jumps
would remain inside the root.
The O_NO*LINK flags return -ELOOP if path resolution would violates
their requirement, while the others all return -EXDEV. Currently these
are only enabled for openat(2). In future it may be necessary to enable
these for *at(2) flags, as well as expanding support for AT_EMPTY_PATH.
This is a refresh of Al's AT_NO_JUMPS patchset[1] (which was a variation
on David Drysdale's O_BENEATH patchset[2], which in turn was based on
the Capsicum project[3]). Input from Linus and Andy in the AT_NO_JUMPS
thread[4] determined most of the API changes made in this refresh.
[1]: https://lwn.net/Articles/721443/
[2]: https://lwn.net/Articles/619151/
[3]: https://lwn.net/Articles/603929/
[4]: https://lwn.net/Articles/723057/
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: Kees Cook <keescook@chromium.org>
Suggested-by: David Drysdale <drysdale@google.com>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
fs/fcntl.c | 2 +-
fs/namei.c | 76 +++++++++++++++++++++++++++-----
fs/open.c | 11 ++++-
include/linux/fcntl.h | 3 +-
include/linux/namei.h | 7 +++
include/uapi/asm-generic/fcntl.h | 14 ++++++
6 files changed, 99 insertions(+), 14 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 083185174c6d..f782e99700f0 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1031,7 +1031,7 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(21 - 1 /* for O_RDONLY being 0 */ !=
+ BUILD_BUG_ON(25 - 1 /* for O_RDONLY being 0 */ !=
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC | __FMODE_NONOTIFY));
diff --git a/fs/namei.c b/fs/namei.c
index 4fdcb36f7c01..c9a07a8c005a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -845,6 +845,12 @@ static inline void path_to_nameidata(const struct path *path,
static int nd_jump_root(struct nameidata *nd)
{
+ if (unlikely(nd->flags & LOOKUP_BENEATH))
+ return -EXDEV;
+ if (unlikely(nd->flags & LOOKUP_XDEV)) {
+ if (nd->path.mnt != nd->root.mnt)
+ return -EXDEV;
+ }
if (nd->flags & LOOKUP_RCU) {
struct dentry *d;
nd->path = nd->root;
@@ -1053,6 +1059,9 @@ const char *get_link(struct nameidata *nd)
int error;
const char *res;
+ if (unlikely(nd->flags & LOOKUP_NO_SYMLINKS))
+ return ERR_PTR(-ELOOP);
+
if (!(nd->flags & LOOKUP_RCU)) {
touch_atime(&last->link);
cond_resched();
@@ -1083,14 +1092,23 @@ const char *get_link(struct nameidata *nd)
} else {
res = get(dentry, inode, &last->done);
}
+ /* If we just jumped it was because of a procfs-style link. */
+ if (unlikely(nd->flags & LOOKUP_JUMPED)) {
+ if (unlikely(nd->flags & LOOKUP_NO_MAGICLINKS))
+ return ERR_PTR(-ELOOP);
+ /* Not currently safe. */
+ if (unlikely(nd->flags & LOOKUP_BENEATH))
+ return ERR_PTR(-EXDEV);
+ }
if (IS_ERR_OR_NULL(res))
return res;
}
if (*res == '/') {
if (!nd->root.mnt)
set_root(nd);
- if (unlikely(nd_jump_root(nd)))
- return ERR_PTR(-ECHILD);
+ error = nd_jump_root(nd);
+ if (unlikely(error))
+ return ERR_PTR(error);
while (unlikely(*++res == '/'))
;
}
@@ -1271,12 +1289,16 @@ static int follow_managed(struct path *path, struct nameidata *nd)
break;
}
- if (need_mntput && path->mnt == mnt)
- mntput(path->mnt);
+ if (need_mntput) {
+ if (path->mnt == mnt)
+ mntput(path->mnt);
+ if (unlikely(nd->flags & LOOKUP_XDEV))
+ ret = -EXDEV;
+ else
+ nd->flags |= LOOKUP_JUMPED;
+ }
if (ret == -EISDIR || !ret)
ret = 1;
- if (need_mntput)
- nd->flags |= LOOKUP_JUMPED;
if (unlikely(ret < 0))
path_put_conditional(path, nd);
return ret;
@@ -1333,6 +1355,8 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path,
mounted = __lookup_mnt(path->mnt, path->dentry);
if (!mounted)
break;
+ if (unlikely(nd->flags & LOOKUP_XDEV))
+ return false;
path->mnt = &mounted->mnt;
path->dentry = mounted->mnt.mnt_root;
nd->flags |= LOOKUP_JUMPED;
@@ -1353,8 +1377,11 @@ static int follow_dotdot_rcu(struct nameidata *nd)
struct inode *inode = nd->inode;
while (1) {
- if (path_equal(&nd->path, &nd->root))
+ if (path_equal(&nd->path, &nd->root)) {
+ if (unlikely(nd->flags & LOOKUP_BENEATH))
+ return -EXDEV;
break;
+ }
if (nd->path.dentry != nd->path.mnt->mnt_root) {
struct dentry *old = nd->path.dentry;
struct dentry *parent = old->d_parent;
@@ -1379,6 +1406,8 @@ static int follow_dotdot_rcu(struct nameidata *nd)
return -ECHILD;
if (&mparent->mnt == nd->path.mnt)
break;
+ if (unlikely(nd->flags & LOOKUP_XDEV))
+ return -EXDEV;
/* we know that mountpoint was pinned */
nd->path.dentry = mountpoint;
nd->path.mnt = &mparent->mnt;
@@ -1393,6 +1422,8 @@ static int follow_dotdot_rcu(struct nameidata *nd)
return -ECHILD;
if (!mounted)
break;
+ if (unlikely(nd->flags & LOOKUP_XDEV))
+ return -EXDEV;
nd->path.mnt = &mounted->mnt;
nd->path.dentry = mounted->mnt.mnt_root;
inode = nd->path.dentry->d_inode;
@@ -1481,8 +1512,11 @@ static int path_parent_directory(struct path *path)
static int follow_dotdot(struct nameidata *nd)
{
while(1) {
- if (path_equal(&nd->path, &nd->root))
+ if (path_equal(&nd->path, &nd->root)) {
+ if (unlikely(nd->flags & LOOKUP_BENEATH))
+ return -EXDEV;
break;
+ }
if (nd->path.dentry != nd->path.mnt->mnt_root) {
int ret = path_parent_directory(&nd->path);
if (ret)
@@ -1491,6 +1525,8 @@ static int follow_dotdot(struct nameidata *nd)
}
if (!follow_up(&nd->path))
break;
+ if (unlikely(nd->flags & LOOKUP_XDEV))
+ return -EXDEV;
}
follow_mount(&nd->path);
nd->inode = nd->path.dentry->d_inode;
@@ -1705,6 +1741,13 @@ static inline int may_lookup(struct nameidata *nd)
static inline int handle_dots(struct nameidata *nd, int type)
{
if (type == LAST_DOTDOT) {
+ /*
+ * LOOKUP_BENEATH resolving ".." is not currently safe -- races can
+ * cause our parent to have moved outside of the root and us to skip
+ * over it.
+ */
+ if (unlikely(nd->flags & LOOKUP_BENEATH))
+ return -EXDEV;
if (!nd->root.mnt)
set_root(nd);
if (nd->flags & LOOKUP_RCU) {
@@ -2253,6 +2296,15 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
nd->path.dentry = NULL;
nd->m_seq = read_seqbegin(&mount_lock);
+
+ if (unlikely(nd->flags & LOOKUP_BENEATH)) {
+ error = dirfd_path_init(nd);
+ if (unlikely(error))
+ return ERR_PTR(error);
+ nd->root = nd->path;
+ if (!(nd->flags & LOOKUP_RCU))
+ path_get(&nd->root);
+ }
if (*s == '/') {
if (likely(!nd->root.mnt))
set_root(nd);
@@ -2261,9 +2313,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
s = ERR_PTR(error);
return s;
}
- error = dirfd_path_init(nd);
- if (unlikely(error))
- return ERR_PTR(error);
+ if (likely(!nd->path.mnt)) {
+ error = dirfd_path_init(nd);
+ if (unlikely(error))
+ return ERR_PTR(error);
+ }
return s;
}
diff --git a/fs/open.c b/fs/open.c
index 0285ce7dbd51..3e73f940f56e 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -959,7 +959,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
* If we have O_PATH in the open flag. Then we
* cannot have anything other than the below set of flags
*/
- flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH;
+ flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH | O_BENEATH |
+ O_XDEV | O_NOSYMLINKS | O_NOMAGICLINKS;
acc_mode = 0;
}
@@ -988,6 +989,14 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
lookup_flags |= LOOKUP_DIRECTORY;
if (!(flags & O_NOFOLLOW))
lookup_flags |= LOOKUP_FOLLOW;
+ if (flags & O_BENEATH)
+ lookup_flags |= LOOKUP_BENEATH;
+ if (flags & O_XDEV)
+ lookup_flags |= LOOKUP_XDEV;
+ if (flags & O_NOMAGICLINKS)
+ lookup_flags |= LOOKUP_NO_MAGICLINKS;
+ if (flags & O_NOSYMLINKS)
+ lookup_flags |= LOOKUP_NO_SYMLINKS;
op->lookup_flags = lookup_flags;
return 0;
}
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 27dc7a60693e..864399c2fdd2 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -9,7 +9,8 @@
(O_RDONLY | O_WRONLY | O_RDWR | O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC | \
O_APPEND | O_NDELAY | O_NONBLOCK | O_NDELAY | __O_SYNC | O_DSYNC | \
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
- O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE)
+ O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_BENEATH | O_XDEV | \
+ O_NOMAGICLINKS | O_NOSYMLINKS)
#ifndef force_o_largefile
#define force_o_largefile() (BITS_PER_LONG != 32)
diff --git a/include/linux/namei.h b/include/linux/namei.h
index a78606e8e3df..82b5039d27a6 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -47,6 +47,13 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
#define LOOKUP_EMPTY 0x4000
#define LOOKUP_DOWN 0x8000
+/* Scoping flags for lookup. */
+#define LOOKUP_BENEATH 0x010000 /* No escaping from starting point. */
+#define LOOKUP_XDEV 0x020000 /* No mountpoint crossing. */
+#define LOOKUP_NO_MAGICLINKS 0x040000 /* No /proc/$pid/fd/ "symlink" crossing. */
+#define LOOKUP_NO_SYMLINKS 0x080000 /* No symlink crossing *at all*.
+ Implies LOOKUP_NO_MAGICLINKS. */
+
extern int path_pts(struct path *path);
extern int user_path_at_empty(int, const char __user *, unsigned, struct path *, int *empty);
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 9dc0bf0c5a6e..ba53e0836cd4 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -97,6 +97,20 @@
#define O_NDELAY O_NONBLOCK
#endif
+/* Type of path-resolution scoping we are applying. */
+#ifndef O_BENEATH
+#define O_BENEATH 00040000000 /* - Block "lexical" trickery like "..", symlinks, absolute paths, etc. */
+#endif
+#ifndef O_XDEV
+#define O_XDEV 00100000000 /* - Block mount-point crossings (includes bind-mounts). */
+#endif
+#ifndef O_NOMAGICLINKS
+#define O_NOMAGICLINKS 00200000000 /* - Block procfs-style "magic" symlinks. */
+#endif
+#ifndef O_NOSYMLINKS
+#define O_NOSYMLINKS 01000000000 /* - Block all symlinks (implies AT_NO_MAGICLINKS). */
+#endif
+
#define F_DUPFD 0 /* dup */
#define F_GETFD 1 /* get close_on_exec */
#define F_SETFD 2 /* set/clear close_on_exec */
--
2.20.1
^ permalink raw reply related
* [PATCH v5 3/5] namei: O_THISROOT: chroot-like path resolution
From: Aleksa Sarai @ 2019-02-13 3:08 UTC (permalink / raw)
To: Al Viro, Jeff Layton, J. Bruce Fields, Arnd Bergmann,
David Howells
Cc: Aleksa Sarai, Eric Biederman, Christian Brauner, Kees Cook,
Andy Lutomirski, Andrew Morton, Alexei Starovoitov, Jann Horn,
David Drysdale, Chanho Min, Oleg Nesterov, Aleksa Sarai,
containers, linux-fsdevel, linux-api, linux-kernel, linux-arch
In-Reply-To: <20190213030851.1881-1-cyphar@cyphar.com>
The primary motivation for the need for this flag is container runtimes
which have to interact with malicious root filesystems in the host
namespaces. One of the first requirements for a container runtime to be
secure against a malicious rootfs is that they correctly scope symlinks
(that is, they should be scoped as though they are chroot(2)ed into the
container's rootfs) and ".."-style paths[*]. The already-existing O_XDEV
and O_NOMAGICLINKS[**] help defend against other potential attacks in a
malicious rootfs scenario.
Currently most container runtimes try to do this resolution in
userspace[1], causing many potential race conditions. In addition, the
"obvious" alternative (actually performing a {ch,pivot_}root(2))
requires a fork+exec (for some runtimes) which is *very* costly if
necessary for every filesystem operation involving a container.
[*] At the moment, ".." and "magic link" jumping are disallowed for the
same reason it is disabled for O_BENEATH -- currently it is not safe
to allow it. Future patches may enable it unconditionally once we
have resolved the possible races (for "..") and semantics (for
"magic link" jumping).
The most significant openat(2) semantic change with O_THISROOT is that
absolute pathnames no longer cause dirfd to be ignored completely. The
rationale is that O_THISROOT must necessarily chroot-scope symlinks with
absolute paths to dirfd, and so doing it for the base path seems to be
the most consistent behaviour (and also avoids foot-gunning users who
want to scope paths that are absolute).
Currently this is only enabled for openat(2), and similar to O_BENEATH
and family requires more discussion about extending it to more *at(2)
syscalls as well as extending AT_EMPTY_PATH support.
[1]: https://github.com/cyphar/filepath-securejoin
Cc: Eric Biederman <ebiederm@xmission.com>
Cc: Christian Brauner <christian@brauner.io>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
fs/fcntl.c | 2 +-
fs/namei.c | 6 +++---
fs/open.c | 4 +++-
include/linux/fcntl.h | 2 +-
include/linux/namei.h | 1 +
include/uapi/asm-generic/fcntl.h | 3 +++
6 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/fcntl.c b/fs/fcntl.c
index f782e99700f0..a6b4565a903d 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -1031,7 +1031,7 @@ static int __init fcntl_init(void)
* Exceptions: O_NONBLOCK is a two bit define on parisc; O_NDELAY
* is defined as O_NONBLOCK on some platforms and not on others.
*/
- BUILD_BUG_ON(25 - 1 /* for O_RDONLY being 0 */ !=
+ BUILD_BUG_ON(26 - 1 /* for O_RDONLY being 0 */ !=
HWEIGHT32(
(VALID_OPEN_FLAGS & ~(O_NONBLOCK | O_NDELAY)) |
__FMODE_EXEC | __FMODE_NONOTIFY));
diff --git a/fs/namei.c b/fs/namei.c
index c9a07a8c005a..798eb1702a0c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1097,7 +1097,7 @@ const char *get_link(struct nameidata *nd)
if (unlikely(nd->flags & LOOKUP_NO_MAGICLINKS))
return ERR_PTR(-ELOOP);
/* Not currently safe. */
- if (unlikely(nd->flags & LOOKUP_BENEATH))
+ if (unlikely(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT)))
return ERR_PTR(-EXDEV);
}
if (IS_ERR_OR_NULL(res))
@@ -1746,7 +1746,7 @@ static inline int handle_dots(struct nameidata *nd, int type)
* cause our parent to have moved outside of the root and us to skip
* over it.
*/
- if (unlikely(nd->flags & LOOKUP_BENEATH))
+ if (unlikely(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT)))
return -EXDEV;
if (!nd->root.mnt)
set_root(nd);
@@ -2297,7 +2297,7 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
nd->m_seq = read_seqbegin(&mount_lock);
- if (unlikely(nd->flags & LOOKUP_BENEATH)) {
+ if (unlikely(nd->flags & (LOOKUP_BENEATH | LOOKUP_IN_ROOT))) {
error = dirfd_path_init(nd);
if (unlikely(error))
return ERR_PTR(error);
diff --git a/fs/open.c b/fs/open.c
index 3e73f940f56e..5914aed2fac8 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -960,7 +960,7 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
* cannot have anything other than the below set of flags
*/
flags &= O_DIRECTORY | O_NOFOLLOW | O_PATH | O_BENEATH |
- O_XDEV | O_NOSYMLINKS | O_NOMAGICLINKS;
+ O_XDEV | O_NOSYMLINKS | O_NOMAGICLINKS | O_THISROOT;
acc_mode = 0;
}
@@ -997,6 +997,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
lookup_flags |= LOOKUP_NO_MAGICLINKS;
if (flags & O_NOSYMLINKS)
lookup_flags |= LOOKUP_NO_SYMLINKS;
+ if (flags & O_THISROOT)
+ lookup_flags |= LOOKUP_IN_ROOT;
op->lookup_flags = lookup_flags;
return 0;
}
diff --git a/include/linux/fcntl.h b/include/linux/fcntl.h
index 864399c2fdd2..46c92bbfce4a 100644
--- a/include/linux/fcntl.h
+++ b/include/linux/fcntl.h
@@ -10,7 +10,7 @@
O_APPEND | O_NDELAY | O_NONBLOCK | O_NDELAY | __O_SYNC | O_DSYNC | \
FASYNC | O_DIRECT | O_LARGEFILE | O_DIRECTORY | O_NOFOLLOW | \
O_NOATIME | O_CLOEXEC | O_PATH | __O_TMPFILE | O_BENEATH | O_XDEV | \
- O_NOMAGICLINKS | O_NOSYMLINKS)
+ O_NOMAGICLINKS | O_NOSYMLINKS | O_THISROOT)
#ifndef force_o_largefile
#define force_o_largefile() (BITS_PER_LONG != 32)
diff --git a/include/linux/namei.h b/include/linux/namei.h
index 82b5039d27a6..0969313b518f 100644
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -53,6 +53,7 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT, LAST_BIND};
#define LOOKUP_NO_MAGICLINKS 0x040000 /* No /proc/$pid/fd/ "symlink" crossing. */
#define LOOKUP_NO_SYMLINKS 0x080000 /* No symlink crossing *at all*.
Implies LOOKUP_NO_MAGICLINKS. */
+#define LOOKUP_IN_ROOT 0x100000 /* Treat dirfd as %current->fs->root. */
extern int path_pts(struct path *path);
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index ba53e0836cd4..3d0fe0de2ba2 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -110,6 +110,9 @@
#ifndef O_NOSYMLINKS
#define O_NOSYMLINKS 01000000000 /* - Block all symlinks (implies AT_NO_MAGICLINKS). */
#endif
+#ifndef O_THISROOT
+#define O_THISROOT 02000000000 /* - Scope ".." resolution to dirfd (like chroot(2)). */
+#endif
#define F_DUPFD 0 /* dup */
#define F_GETFD 1 /* get close_on_exec */
--
2.20.1
^ permalink raw reply related
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