* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Mathieu Desnoyers @ 2018-04-16 19:21 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon
In-Reply-To: <CA+55aFzZtY5ELKc+TGf9BA6px300BNzedadm_yNXzODTKN=HgQ@mail.gmail.com>
----- On Apr 16, 2018, at 2:39 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
> On Mon, Apr 16, 2018 at 11:35 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> Specifically for single-stepping, the __rseq_table section introduced
>> at user-level will allow newer debuggers and tools which do line and
>> instruction-level single-stepping to skip over rseq critical sections.
>> However, this breaks existing debuggers and tools.
>
> I really don't think single-stepping is a valid argument.
>
> Even if the cpu_opv() allows you to "single step", you're not actually
> single stepping the same thing that you're using. So you are literally
> debugging something else than the real code.
>
> At that point, you don't need "cpu_opv()", you need to just load
> /dev/urandom in a buffer, and single-step that. Ta-daa! No new kernel
> functionality needed.
>
> So if the main argument for cpu_opv is single-stepping, then just rip
> it out. It's not useful.
No, single-stepping is not the only use-case. Accessing remote cpu
data is another use-case fulfilled by cpu_opv, which I think is more
compelling.
>
> Anybody who cares deeply about single-stepping shouldn't be using
> optimistic algorithms, and they shouldn't be doing multi-threaded
> stuff either. They won't be able to use things like transactional
> memory either.
>
> You can't single-step into the kernel to see what the kernel does
> either when you're debugging something.
>
> News at 11: "single stepping isn't always viable".
I don't mind if people cannot stop the program with a debugger and
observe the state of registers manually at each step though a rseq
critical section.
I do mind breaking existing tools that rely on single-stepping
approaches to automatically analyze program behavior [1,2].
Introducing a rseq critical section into a library (e.g. glibc
memory allocator) would cause existing programs being analyzed
with existing tools to hang.
And I try very hard to avoid being told I'm the one breaking
user-space. ;-)
Thanks,
Mathieu
[1] http://rr-project.org/
[2] https://www.gnu.org/software/gdb/news/reversible.html
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Michal Hocko @ 2018-04-16 19:18 UTC (permalink / raw)
To: Jann Horn
Cc: Michael Kerrisk (man-pages), John Hubbard, linux-man,
Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <CAG48ez3DwRXMtiinUWKnan6hAppLYLdx-w+VzXG6ubioZUacQg@mail.gmail.com>
On Mon 16-04-18 15:55:36, Jann Horn wrote:
> On Mon, Apr 16, 2018 at 12:07 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Fri 13-04-18 18:17:36, Jann Horn wrote:
> >> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <jannh@google.com> wrote:
> >> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <mhocko@kernel.org> wrote:
> >> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
> >> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <mhocko@kernel.org> wrote:
> >> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> >> >>> > [...]
> >> >>> >> So, you mean remove this entire paragraph:
> >> >>> >>
> >> >>> >> For cases in which the specified memory region has not been
> >> >>> >> reserved using an existing mapping, newer kernels (Linux
> >> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> >> >>> >> should be used instead; older kernels require the caller to
> >> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
> >> >>> >> action if the kernel places the new mapping at a different
> >> >>> >> address.
> >> >>> >>
> >> >>> >> It seems like some version of the first half of the paragraph is worth
> >> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
> >> >>> >> How about replacing that text with the following:
> >> >>> >>
> >> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> >> >>> >> in a multithreaded program to avoid the hazard described
> >> >>> >> above.
> >> >>> >
> >> >>> > Yes, that sounds reasonable to me.
> >> >>>
> >> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
> >> >>> when actually, you just have to call mmap() with the address as hint,
> >> >>> and if mmap() returns a different address, munmap() it and go on your
> >> >>> normal error path.
> >> >>
> >> >> This is still racy in multithreaded application which is the main point
> >> >> of the whole section, no?
> >> >
> >> > No, it isn't.
> >
> > I could have been more specific, sorry.
> >
> >> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
> >> a memory region for you or return an error code. If it does allocate a
> >> memory region, it belongs to you until you deallocate it. It might be
> >> at a different address than you requested -
> >
> > Yes, this all is true. Except the atomicity is guaranteed only for the
> > syscall. Once you return to the userspace any error handling is error
> > prone and racy because your mapping might change under you feet. So...
>
> Can you please elaborate on why you think anything could change the
> mapping returned by mmap() under the caller's feet?
Because as soon as the mmap_sem is dropped then any other thread can
modify the shared address space.
> When mmap() returns a memory area to the caller, that memory area
> belongs to the caller. No unrelated code will touch it, unless that
> code is buggy.
Yes, reasonably well written application will not have this problem.
That, however, requires an external synchronization and that's why
called it error prone and racy. I guess that was the main motivation for
that part of the man page.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Linus Torvalds @ 2018-04-16 18:39 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andy Lutomirski, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon
In-Reply-To: <542721578.11358.1523903708510.JavaMail.zimbra@efficios.com>
On Mon, Apr 16, 2018 at 11:35 AM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> Specifically for single-stepping, the __rseq_table section introduced
> at user-level will allow newer debuggers and tools which do line and
> instruction-level single-stepping to skip over rseq critical sections.
> However, this breaks existing debuggers and tools.
I really don't think single-stepping is a valid argument.
Even if the cpu_opv() allows you to "single step", you're not actually
single stepping the same thing that you're using. So you are literally
debugging something else than the real code.
At that point, you don't need "cpu_opv()", you need to just load
/dev/urandom in a buffer, and single-step that. Ta-daa! No new kernel
functionality needed.
So if the main argument for cpu_opv is single-stepping, then just rip
it out. It's not useful.
Anybody who cares deeply about single-stepping shouldn't be using
optimistic algorithms, and they shouldn't be doing multi-threaded
stuff either. They won't be able to use things like transactional
memory either.
You can't single-step into the kernel to see what the kernel does
either when you're debugging something.
News at 11: "single stepping isn't always viable".
Linus
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Mathieu Desnoyers @ 2018-04-16 18:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Linus Torvalds, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon <will>
In-Reply-To: <CALCETrVvaMWUJKx+bxXdEm3oVBzn2r5RuJ4JcXNzOPSgJAC5PA@mail.gmail.com>
----- On Apr 14, 2018, at 6:44 PM, Andy Lutomirski luto@amacapital.net wrote:
> On Thu, Apr 12, 2018 at 12:43 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Thu, Apr 12, 2018 at 12:27 PM, Mathieu Desnoyers
>> <mathieu.desnoyers@efficios.com> wrote:
>>> The cpu_opv system call executes a vector of operations on behalf of
>>> user-space on a specific CPU with preemption disabled. It is inspired
>>> by readv() and writev() system calls which take a "struct iovec"
>>> array as argument.
>>
>> Do we really want the page pinning?
>>
>> This whole cpu_opv thing is the most questionable part of the series,
>> and the page pinning is the most questionable part of cpu_opv for me.
>>
>> Can we plan on merging just the plain rseq parts *without* this all
>> first, and then see the cpu_opv thing as a "maybe future expansion"
>> part.
>>
>> I think that would make Andy happier too.
>>
>
> It only makes me happier if the userspace code involved is actually
> going to work when single-stepped, which might actually be the case
> (fingers crossed).
Specifically for single-stepping, the __rseq_table section introduced
at user-level will allow newer debuggers and tools which do line and
instruction-level single-stepping to skip over rseq critical sections.
However, this breaks existing debuggers and tools.
For a userspace tracer tool such as LTTng-UST, requiring upgrade to newer
debugger versions would limit its adoption in the field. So if using rseq
breaks current debugger tools, lttng-ust won't use rseq until
single-stepping can be done in a non-breaking way, or will have to wait
until most end-user deployments (distributions used in the field) include
debugger versions that skip over the code identified by the __rseq_table
section, which will take many years.
> That being said, I'm not really convinced that
> cpu_opv() makes much difference here, since I'm not entirely convinced
> that user code will actually use it or that user code will actually be
> that well tested. C'est la vie.
For the use-case of cpu_opv invoked as single-stepping fall-back, this path
will indeed not be executed often enough to be well-tested. I'm considering
the following approach to allow user-space to test cpu_opv more thoroughly:
we can introduce an environment variable, e.g.:
- RSEQ_DISABLE=1: Disable rseq thread registration,
- RSEQ_DISABLE=random: Randomly disable rseq thread registration (some threads
use rseq, other threads end up using the cpu_opv fallback)
which would disable the rseq fast-path for all or some threads, and thus allow
thorough testing of cpu_opv used as single-stepping fallback.
Thoughts ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Andi Kleen @ 2018-04-16 17:02 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Andi Kleen, Linus Torvalds, Peter Zijlstra, Paul E. McKenney,
Boqun Feng, Andy Lutomirski, Dave Watson, linux-kernel, linux-api,
Paul Turner, Andrew Morton, Russell King, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, Andrew Hunter, Chris Lameter,
Ben Maurer, rostedt, Josh Triplett, Catalin
In-Reply-To: <751033048.11156.1523896094938.JavaMail.zimbra@efficios.com>
> Single-stepping is only a subset of the rseq limitations addressed
> by cpu_opv. Anoher major limitation is algorithms requiring data
> migration between per-cpu data structures safely against CPU hotplug,
> and without having to change the cpu affinity mask. This is the case
And how many people are going to implement such a complex separate
path just for CPU hotplug? And even if they implement it how long
before it bitrots? Seems more like a checkbox item than a realistic
approach.
> for memory allocators and userspace task schedulers which require
> cpu_opv for migration between per-cpu memory pools and scheduler
> runqueues.
Not sure about that. Is that common?
>
> About the vgettimeofday and general handling of vDSO by gdb, gdb's
> approach only takes care of line-by-line single-stepping by hiding
> Linux' vdso mapping so users cannot target source code lines within
> that shared object. However, it breaks instruction-level single-stepping.
> I reported this issue to you back in Nov. 2017:
> https://lkml.org/lkml/2017/11/20/803
It was known from day 1, but afaik never a problem.
-Andi
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Mathieu Desnoyers @ 2018-04-16 16:28 UTC (permalink / raw)
To: Andi Kleen
Cc: Linus Torvalds, Peter Zijlstra, Paul E. McKenney, Boqun Feng,
Andy Lutomirski, Dave Watson, linux-kernel, linux-api,
Paul Turner, Andrew Morton, Russell King, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, Andrew Hunter, Chris Lameter,
Ben Maurer, rostedt, Josh Triplett, Catalin Marinas, Will
In-Reply-To: <20180412202302.xnw5jdl3upl46ywt@two.firstfloor.org>
----- On Apr 12, 2018, at 4:23 PM, Andi Kleen andi@firstfloor.org wrote:
>> Can we plan on merging just the plain rseq parts *without* this all
>> first, and then see the cpu_opv thing as a "maybe future expansion"
>> part.
>
> That would be the right way to go. I doubt anybody really needs cpu_opv.
> We already have other code (e.g. vgettimeofday) which cannot
> be single stepped, and so far it never was a problem.
Single-stepping is only a subset of the rseq limitations addressed
by cpu_opv. Anoher major limitation is algorithms requiring data
migration between per-cpu data structures safely against CPU hotplug,
and without having to change the cpu affinity mask. This is the case
for memory allocators and userspace task schedulers which require
cpu_opv for migration between per-cpu memory pools and scheduler
runqueues.
About the vgettimeofday and general handling of vDSO by gdb, gdb's
approach only takes care of line-by-line single-stepping by hiding
Linux' vdso mapping so users cannot target source code lines within
that shared object. However, it breaks instruction-level single-stepping.
I reported this issue to you back in Nov. 2017:
https://lkml.org/lkml/2017/11/20/803
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Jann Horn @ 2018-04-16 13:55 UTC (permalink / raw)
To: Michal Hocko
Cc: Michael Kerrisk (man-pages), John Hubbard, linux-man,
Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <20180416100736.GG17484@dhcp22.suse.cz>
On Mon, Apr 16, 2018 at 12:07 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Fri 13-04-18 18:17:36, Jann Horn wrote:
>> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <jannh@google.com> wrote:
>> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <mhocko@kernel.org> wrote:
>> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>> >>> > [...]
>> >>> >> So, you mean remove this entire paragraph:
>> >>> >>
>> >>> >> For cases in which the specified memory region has not been
>> >>> >> reserved using an existing mapping, newer kernels (Linux
>> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>> >>> >> should be used instead; older kernels require the caller to
>> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>> >>> >> action if the kernel places the new mapping at a different
>> >>> >> address.
>> >>> >>
>> >>> >> It seems like some version of the first half of the paragraph is worth
>> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
>> >>> >> How about replacing that text with the following:
>> >>> >>
>> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>> >>> >> in a multithreaded program to avoid the hazard described
>> >>> >> above.
>> >>> >
>> >>> > Yes, that sounds reasonable to me.
>> >>>
>> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>> >>> when actually, you just have to call mmap() with the address as hint,
>> >>> and if mmap() returns a different address, munmap() it and go on your
>> >>> normal error path.
>> >>
>> >> This is still racy in multithreaded application which is the main point
>> >> of the whole section, no?
>> >
>> > No, it isn't.
>
> I could have been more specific, sorry.
>
>> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
>> a memory region for you or return an error code. If it does allocate a
>> memory region, it belongs to you until you deallocate it. It might be
>> at a different address than you requested -
>
> Yes, this all is true. Except the atomicity is guaranteed only for the
> syscall. Once you return to the userspace any error handling is error
> prone and racy because your mapping might change under you feet. So...
Can you please elaborate on why you think anything could change the
mapping returned by mmap() under the caller's feet?
When mmap() returns a memory area to the caller, that memory area
belongs to the caller. No unrelated code will touch it, unless that
code is buggy.
>> in that case you can
>> emulate MAP_FIXED_NOREPLACE by calling munmap() and treating it as an
>> error; or you can do something else with it.
>>
>> MAP_FIXED_NOREPLACE is just a performance optimization.
>
> This is not quite true because you get _your_ area or _an error_
> atomically which is not possible with 2 syscalls.
Why not?
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Michal Hocko @ 2018-04-16 10:07 UTC (permalink / raw)
To: Jann Horn
Cc: Michael Kerrisk (man-pages), John Hubbard, linux-man,
Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <CAG48ez1PdzMs8hkatbzSLBWYucjTc75o8ovSmeC66+e9mLvSfA@mail.gmail.com>
On Fri 13-04-18 18:17:36, Jann Horn wrote:
> On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <jannh@google.com> wrote:
> > On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <mhocko@kernel.org> wrote:
> >> On Fri 13-04-18 17:04:09, Jann Horn wrote:
> >>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <mhocko@kernel.org> wrote:
> >>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> >>> > [...]
> >>> >> So, you mean remove this entire paragraph:
> >>> >>
> >>> >> For cases in which the specified memory region has not been
> >>> >> reserved using an existing mapping, newer kernels (Linux
> >>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> >>> >> should be used instead; older kernels require the caller to
> >>> >> use addr as a hint (without MAP_FIXED) and take appropriate
> >>> >> action if the kernel places the new mapping at a different
> >>> >> address.
> >>> >>
> >>> >> It seems like some version of the first half of the paragraph is worth
> >>> >> keeping, though, so as to point the reader in the direction of a remedy.
> >>> >> How about replacing that text with the following:
> >>> >>
> >>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> >>> >> in a multithreaded program to avoid the hazard described
> >>> >> above.
> >>> >
> >>> > Yes, that sounds reasonable to me.
> >>>
> >>> But that kind of sounds as if you can't avoid it before Linux 4.17,
> >>> when actually, you just have to call mmap() with the address as hint,
> >>> and if mmap() returns a different address, munmap() it and go on your
> >>> normal error path.
> >>
> >> This is still racy in multithreaded application which is the main point
> >> of the whole section, no?
> >
> > No, it isn't.
I could have been more specific, sorry.
> mmap() with a hint (without MAP_FIXED) will always non-racily allocate
> a memory region for you or return an error code. If it does allocate a
> memory region, it belongs to you until you deallocate it. It might be
> at a different address than you requested -
Yes, this all is true. Except the atomicity is guaranteed only for the
syscall. Once you return to the userspace any error handling is error
prone and racy because your mapping might change under you feet. So...
> in that case you can
> emulate MAP_FIXED_NOREPLACE by calling munmap() and treating it as an
> error; or you can do something else with it.
>
> MAP_FIXED_NOREPLACE is just a performance optimization.
This is not quite true because you get _your_ area or _an error_
atomically which is not possible with 2 syscalls.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* [PATCH 7/7] aio: implement io_pgetevents
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
This is the io_getevents equivalent of ppoll/pselect and allows to
properly mix signals and aio completions (especially with IOCB_CMD_POLL)
and atomically executes the following sequence:
sigset_t origmask;
pthread_sigmask(SIG_SETMASK, &sigmask, &origmask);
ret = io_getevents(ctx, min_nr, nr, events, timeout);
pthread_sigmask(SIG_SETMASK, &origmask, NULL);
Note that unlike many other signal related calls we do not pass a sigmask
size, as that would get us to 7 arguments, which aren't easily supported
by the syscall infrastructure. It seems a lot less painful to just add a
new syscall variant in the unlikely case we're going to increase the
sigset size.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/aio.c | 114 ++++++++++++++++++++++---
include/linux/compat.h | 7 ++
include/linux/syscalls.h | 6 ++
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/aio_abi.h | 6 ++
kernel/sys_ni.c | 2 +
8 files changed, 130 insertions(+), 11 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index c58f75b088c5..99b6f7a36178 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -391,3 +391,4 @@
382 i386 pkey_free sys_pkey_free
383 i386 statx sys_statx
384 i386 arch_prctl sys_arch_prctl compat_sys_arch_prctl
+385 i386 io_pgetevents sys_io_pgetevents compat_sys_io_pgetevents
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 5aef183e2f85..e995cd2b4e65 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -339,6 +339,7 @@
330 common pkey_alloc sys_pkey_alloc
331 common pkey_free sys_pkey_free
332 common statx sys_statx
+333 common io_pgetevents sys_io_pgetevents
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/fs/aio.c b/fs/aio.c
index de56496ba86c..5cfeb5bc3a7d 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1303,10 +1303,6 @@ static long read_events(struct kioctx *ctx, long min_nr, long nr,
wait_event_interruptible_hrtimeout(ctx->wait,
aio_read_events(ctx, min_nr, nr, event, &ret),
until);
-
- if (!ret && signal_pending(current))
- ret = -EINTR;
-
return ret;
}
@@ -1913,13 +1909,60 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
struct timespec __user *, timeout)
{
struct timespec64 ts;
+ int ret;
+
+ if (timeout && unlikely(get_timespec64(&ts, timeout)))
+ return -EFAULT;
+
+ ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
+ if (!ret && signal_pending(current))
+ ret = -EINTR;
+ return ret;
+}
- if (timeout) {
- if (unlikely(get_timespec64(&ts, timeout)))
+SYSCALL_DEFINE6(io_pgetevents,
+ aio_context_t, ctx_id,
+ long, min_nr,
+ long, nr,
+ struct io_event __user *, events,
+ struct timespec __user *, timeout,
+ const struct __aio_sigset __user *, usig)
+{
+ struct __aio_sigset ksig = { NULL, };
+ sigset_t ksigmask, sigsaved;
+ struct timespec64 ts;
+ int ret;
+
+ if (timeout && unlikely(get_timespec64(&ts, timeout)))
+ return -EFAULT;
+
+ if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
+ return -EFAULT;
+
+ if (ksig.sigmask) {
+ if (ksig.sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+ if (copy_from_user(&ksigmask, ksig.sigmask, sizeof(ksigmask)))
return -EFAULT;
+ sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+ sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
+ }
+
+ ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
+ if (signal_pending(current)) {
+ if (ksig.sigmask) {
+ current->saved_sigmask = sigsaved;
+ set_restore_sigmask();
+ }
+
+ if (!ret)
+ ret = -ERESTARTNOHAND;
+ } else {
+ if (ksig.sigmask)
+ sigprocmask(SIG_SETMASK, &sigsaved, NULL);
}
- return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
+ return ret;
}
#ifdef CONFIG_COMPAT
@@ -1930,13 +1973,64 @@ COMPAT_SYSCALL_DEFINE5(io_getevents, compat_aio_context_t, ctx_id,
struct compat_timespec __user *, timeout)
{
struct timespec64 t;
+ int ret;
+
+ if (timeout && compat_get_timespec64(&t, timeout))
+ return -EFAULT;
+
+ ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
+ if (!ret && signal_pending(current))
+ ret = -EINTR;
+ return ret;
+}
+
- if (timeout) {
- if (compat_get_timespec64(&t, timeout))
+struct __compat_aio_sigset {
+ compat_sigset_t __user *sigmask;
+ compat_size_t sigsetsize;
+};
+
+COMPAT_SYSCALL_DEFINE6(io_pgetevents,
+ compat_aio_context_t, ctx_id,
+ compat_long_t, min_nr,
+ compat_long_t, nr,
+ struct io_event __user *, events,
+ struct compat_timespec __user *, timeout,
+ const struct __compat_aio_sigset __user *, usig)
+{
+ struct __compat_aio_sigset ksig = { NULL, };
+ sigset_t ksigmask, sigsaved;
+ struct timespec64 t;
+ int ret;
+
+ if (timeout && compat_get_timespec64(&t, timeout))
+ return -EFAULT;
+
+ if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
+ return -EFAULT;
+
+ if (ksig.sigmask) {
+ if (ksig.sigsetsize != sizeof(compat_sigset_t))
+ return -EINVAL;
+ if (get_compat_sigset(&ksigmask, ksig.sigmask))
return -EFAULT;
+ sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+ sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
+ }
+ ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
+ if (signal_pending(current)) {
+ if (ksig.sigmask) {
+ current->saved_sigmask = sigsaved;
+ set_restore_sigmask();
+ }
+ if (!ret)
+ ret = -ERESTARTNOHAND;
+ } else {
+ if (ksig.sigmask)
+ sigprocmask(SIG_SETMASK, &sigsaved, NULL);
}
- return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
+ return ret;
}
#endif
diff --git a/include/linux/compat.h b/include/linux/compat.h
index f188eab10570..fdd068f192c1 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -310,6 +310,7 @@ extern int put_compat_rusage(const struct rusage *,
struct compat_rusage __user *);
struct compat_siginfo;
+struct __compat_aio_sigset;
struct compat_dirent {
u32 d_ino;
@@ -528,6 +529,12 @@ asmlinkage long compat_sys_io_getevents(compat_aio_context_t ctx_id,
compat_long_t nr,
struct io_event __user *events,
struct compat_timespec __user *timeout);
+asmlinkage long compat_sys_io_pgetevents(compat_aio_context_t ctx_id,
+ compat_long_t min_nr,
+ compat_long_t nr,
+ struct io_event __user *events,
+ struct compat_timespec __user *timeout,
+ const struct __compat_aio_sigset __user *usig);
/* fs/cookies.c */
asmlinkage long compat_sys_lookup_dcookie(u32, u32, char __user *, compat_size_t);
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index b961184f597a..c40cf5ec24c9 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -264,6 +264,12 @@ asmlinkage long sys_io_getevents(aio_context_t ctx_id,
long nr,
struct io_event __user *events,
struct timespec __user *timeout);
+asmlinkage long sys_io_pgetevents(aio_context_t ctx_id,
+ long min_nr,
+ long nr,
+ struct io_event __user *events,
+ struct timespec __user *timeout,
+ const struct __aio_sigset *sig);
/* fs/xattr.c */
asmlinkage long sys_setxattr(const char __user *path, const char __user *name,
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 8bcb186c6f67..42990676a55e 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -732,9 +732,11 @@ __SYSCALL(__NR_pkey_alloc, sys_pkey_alloc)
__SYSCALL(__NR_pkey_free, sys_pkey_free)
#define __NR_statx 291
__SYSCALL(__NR_statx, sys_statx)
+#define __NR_io_pgetevents 292
+__SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
#undef __NR_syscalls
-#define __NR_syscalls 292
+#define __NR_syscalls 293
/*
* 32 bit systems traditionally used different
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index a04adbc70ddf..2c0a3415beee 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -29,6 +29,7 @@
#include <linux/types.h>
#include <linux/fs.h>
+#include <linux/signal.h>
#include <asm/byteorder.h>
typedef __kernel_ulong_t aio_context_t;
@@ -108,5 +109,10 @@ struct iocb {
#undef IFBIG
#undef IFLITTLE
+struct __aio_sigset {
+ sigset_t __user *sigmask;
+ size_t sigsetsize;
+};
+
#endif /* __LINUX__AIO_ABI_H */
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 6cafc008f6db..7571cf9d7424 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -33,7 +33,9 @@ COND_SYSCALL(io_submit);
COND_SYSCALL_COMPAT(io_submit);
COND_SYSCALL(io_cancel);
COND_SYSCALL(io_getevents);
+COND_SYSCALL(io_pgetevents);
COND_SYSCALL_COMPAT(io_getevents);
+COND_SYSCALL_COMPAT(io_pgetevents);
/* fs/xattr.c */
--
2.17.0
^ permalink raw reply related
* [PATCH 6/7] aio: implement IOCB_CMD_FSYNC and IOCB_CMD_FDSYNC
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
Simple workqueue offload for now, but prepared for adding a real aio_fsync
method if the need arises. Based on an earlier patch from Dave Chinner.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/aio.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/fs/aio.c b/fs/aio.c
index d4dd002712c8..de56496ba86c 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -156,6 +156,12 @@ struct kioctx {
unsigned id;
};
+struct fsync_iocb {
+ struct work_struct work;
+ struct file *file;
+ bool datasync;
+};
+
/*
* We use ki_cancel == KIOCB_CANCELLED to indicate that a kiocb has been either
* cancelled or completed (this makes a certain amount of sense because
@@ -172,6 +178,7 @@ struct kioctx {
struct aio_kiocb {
union {
struct kiocb rw;
+ struct fsync_iocb fsync;
};
struct kioctx *ki_ctx;
@@ -1571,6 +1578,36 @@ static ssize_t aio_write(struct kiocb *req, struct iocb *iocb, bool vectored,
return ret;
}
+static void aio_fsync_work(struct work_struct *work)
+{
+ struct fsync_iocb *req = container_of(work, struct fsync_iocb, work);
+ int ret;
+
+ ret = vfs_fsync(req->file, req->datasync);
+ fput(req->file);
+ aio_complete(container_of(req, struct aio_kiocb, fsync), ret, 0);
+}
+
+static int aio_fsync(struct fsync_iocb *req, struct iocb *iocb, bool datasync)
+{
+ if (unlikely(iocb->aio_buf || iocb->aio_offset || iocb->aio_nbytes ||
+ iocb->aio_rw_flags))
+ return -EINVAL;
+
+ req->file = fget(iocb->aio_fildes);
+ if (unlikely(!req->file))
+ return -EBADF;
+ if (unlikely(!req->file->f_op->fsync)) {
+ fput(req->file);
+ return -EINVAL;
+ }
+
+ req->datasync = datasync;
+ INIT_WORK(&req->work, aio_fsync_work);
+ schedule_work(&req->work);
+ return -EIOCBQUEUED;
+}
+
static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
struct iocb *iocb, bool compat)
{
@@ -1634,6 +1671,12 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
case IOCB_CMD_PWRITEV:
ret = aio_write(&req->rw, iocb, true, compat);
break;
+ case IOCB_CMD_FSYNC:
+ ret = aio_fsync(&req->fsync, iocb, false);
+ break;
+ case IOCB_CMD_FDSYNC:
+ ret = aio_fsync(&req->fsync, iocb, true);
+ break;
default:
pr_debug("invalid aio operation %d\n", iocb->aio_lio_opcode);
ret = -EINVAL;
--
2.17.0
--
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
* [PATCH 5/7] aio: refactor read/write iocb setup
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
Don't reference the kiocb structure from the common aio code, and move
any use of it into helper specific to the read/write path. This is in
preparation for aio_poll support that wants to use the space for different
fields.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/aio.c | 159 +++++++++++++++++++++++++++++++------------------------
1 file changed, 91 insertions(+), 68 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index d7be32cdd1db..d4dd002712c8 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -170,7 +170,9 @@ struct kioctx {
#define KIOCB_CANCELLED ((void *) (~0ULL))
struct aio_kiocb {
- struct kiocb common;
+ union {
+ struct kiocb rw;
+ };
struct kioctx *ki_ctx;
kiocb_cancel_fn *ki_cancel;
@@ -549,7 +551,7 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int nr_events)
void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
{
- struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, common);
+ struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
struct kioctx *ctx = req->ki_ctx;
unsigned long flags;
@@ -581,7 +583,7 @@ static int kiocb_cancel(struct aio_kiocb *kiocb)
cancel = cmpxchg(&kiocb->ki_cancel, old, KIOCB_CANCELLED);
} while (cancel != old);
- return cancel(&kiocb->common);
+ return cancel(&kiocb->rw);
}
/*
@@ -1046,15 +1048,6 @@ static inline struct aio_kiocb *aio_get_req(struct kioctx *ctx)
return NULL;
}
-static void kiocb_free(struct aio_kiocb *req)
-{
- if (req->common.ki_filp)
- fput(req->common.ki_filp);
- if (req->ki_eventfd != NULL)
- eventfd_ctx_put(req->ki_eventfd);
- kmem_cache_free(kiocb_cachep, req);
-}
-
static struct kioctx *lookup_ioctx(unsigned long ctx_id)
{
struct aio_ring __user *ring = (void __user *)ctx_id;
@@ -1085,27 +1078,14 @@ static struct kioctx *lookup_ioctx(unsigned long ctx_id)
/* aio_complete
* Called when the io request on the given iocb is complete.
*/
-static void aio_complete(struct kiocb *kiocb, long res, long res2)
+static void aio_complete(struct aio_kiocb *iocb, long res, long res2)
{
- struct aio_kiocb *iocb = container_of(kiocb, struct aio_kiocb, common);
struct kioctx *ctx = iocb->ki_ctx;
struct aio_ring *ring;
struct io_event *ev_page, *event;
unsigned tail, pos, head;
unsigned long flags;
- if (kiocb->ki_flags & IOCB_WRITE) {
- struct file *file = kiocb->ki_filp;
-
- /*
- * Tell lockdep we inherited freeze protection from submission
- * thread.
- */
- if (S_ISREG(file_inode(file)->i_mode))
- __sb_writers_acquired(file_inode(file)->i_sb, SB_FREEZE_WRITE);
- file_end_write(file);
- }
-
if (!list_empty_careful(&iocb->ki_list)) {
unsigned long flags;
@@ -1167,11 +1147,12 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
* eventfd. The eventfd_signal() function is safe to be called
* from IRQ context.
*/
- if (iocb->ki_eventfd != NULL)
+ if (iocb->ki_eventfd) {
eventfd_signal(iocb->ki_eventfd, 1);
+ eventfd_ctx_put(iocb->ki_eventfd);
+ }
- /* everything turned out well, dispose of the aiocb. */
- kiocb_free(iocb);
+ kmem_cache_free(kiocb_cachep, iocb);
/*
* We have to order our ring_info tail store above and test
@@ -1434,6 +1415,45 @@ SYSCALL_DEFINE1(io_destroy, aio_context_t, ctx)
return -EINVAL;
}
+static void aio_complete_rw(struct kiocb *kiocb, long res, long res2)
+{
+ struct aio_kiocb *iocb = container_of(kiocb, struct aio_kiocb, rw);
+
+ if (kiocb->ki_flags & IOCB_WRITE) {
+ struct inode *inode = file_inode(kiocb->ki_filp);
+
+ /*
+ * Tell lockdep we inherited freeze protection from submission
+ * thread.
+ */
+ if (S_ISREG(inode->i_mode))
+ __sb_writers_acquired(inode->i_sb, SB_FREEZE_WRITE);
+ file_end_write(kiocb->ki_filp);
+ }
+
+ fput(kiocb->ki_filp);
+ aio_complete(iocb, res, res2);
+}
+
+static int aio_prep_rw(struct kiocb *req, struct iocb *iocb)
+{
+ int ret;
+
+ req->ki_filp = fget(iocb->aio_fildes);
+ if (unlikely(!req->ki_filp))
+ return -EBADF;
+ req->ki_complete = aio_complete_rw;
+ req->ki_pos = iocb->aio_offset;
+ req->ki_flags = iocb_flags(req->ki_filp);
+ if (iocb->aio_flags & IOCB_FLAG_RESFD)
+ req->ki_flags |= IOCB_EVENTFD;
+ req->ki_hint = file_write_hint(req->ki_filp);
+ ret = kiocb_set_rw_flags(req, iocb->aio_rw_flags);
+ if (unlikely(ret))
+ fput(req->ki_filp);
+ return ret;
+}
+
static int aio_setup_rw(int rw, struct iocb *iocb, struct iovec **iovec,
bool vectored, bool compat, struct iov_iter *iter)
{
@@ -1453,7 +1473,7 @@ static int aio_setup_rw(int rw, struct iocb *iocb, struct iovec **iovec,
return import_iovec(rw, buf, len, UIO_FASTIOV, iovec, iter);
}
-static inline ssize_t aio_ret(struct kiocb *req, ssize_t ret)
+static inline ssize_t aio_rw_ret(struct kiocb *req, ssize_t ret)
{
switch (ret) {
case -EIOCBQUEUED:
@@ -1469,7 +1489,7 @@ static inline ssize_t aio_ret(struct kiocb *req, ssize_t ret)
ret = -EINTR;
/*FALLTHRU*/
default:
- aio_complete(req, ret, 0);
+ aio_complete_rw(req, ret, 0);
return 0;
}
}
@@ -1477,57 +1497,77 @@ static inline ssize_t aio_ret(struct kiocb *req, ssize_t ret)
static ssize_t aio_read(struct kiocb *req, struct iocb *iocb, bool vectored,
bool compat)
{
- struct file *file = req->ki_filp;
struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
struct iov_iter iter;
+ struct file *file;
ssize_t ret;
+ ret = aio_prep_rw(req, iocb);
+ if (ret)
+ return ret;
+ file = req->ki_filp;
+
+ ret = -EBADF;
if (unlikely(!(file->f_mode & FMODE_READ)))
- return -EBADF;
+ goto out_fput;
+ ret = -EINVAL;
if (unlikely(!file->f_op->read_iter))
- return -EINVAL;
+ goto out_fput;
ret = aio_setup_rw(READ, iocb, &iovec, vectored, compat, &iter);
if (ret)
- return ret;
+ goto out_fput;
ret = rw_verify_area(READ, file, &req->ki_pos, iov_iter_count(&iter));
if (!ret)
- ret = aio_ret(req, call_read_iter(file, req, &iter));
+ ret = aio_rw_ret(req, call_read_iter(file, req, &iter));
kfree(iovec);
+out_fput:
+ if (unlikely(ret && ret != -EIOCBQUEUED))
+ fput(file);
return ret;
}
static ssize_t aio_write(struct kiocb *req, struct iocb *iocb, bool vectored,
bool compat)
{
- struct file *file = req->ki_filp;
struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs;
struct iov_iter iter;
+ struct file *file;
ssize_t ret;
+ ret = aio_prep_rw(req, iocb);
+ if (ret)
+ return ret;
+ file = req->ki_filp;
+
+ ret = -EBADF;
if (unlikely(!(file->f_mode & FMODE_WRITE)))
- return -EBADF;
+ goto out_fput;
+ ret = -EINVAL;
if (unlikely(!file->f_op->write_iter))
- return -EINVAL;
+ goto out_fput;
ret = aio_setup_rw(WRITE, iocb, &iovec, vectored, compat, &iter);
if (ret)
- return ret;
+ goto out_fput;
ret = rw_verify_area(WRITE, file, &req->ki_pos, iov_iter_count(&iter));
if (!ret) {
/*
- * We release freeze protection in aio_complete(). Fool lockdep
- * by telling it the lock got released so that it doesn't
- * complain about held lock when we return to userspace.
+ * We release freeze protection in aio_complete_rw(). Fool
+ * lockdep by telling it the lock got released so that it
+ * doesn't complain about held lock when we return to userspace.
*/
if (S_ISREG(file_inode(file)->i_mode)) {
__sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, true);
__sb_writers_release(file_inode(file)->i_sb, SB_FREEZE_WRITE);
}
req->ki_flags |= IOCB_WRITE;
- ret = aio_ret(req, call_write_iter(file, req, &iter));
+ ret = aio_rw_ret(req, call_write_iter(file, req, &iter));
}
kfree(iovec);
+out_fput:
+ if (unlikely(ret && ret != -EIOCBQUEUED))
+ fput(file);
return ret;
}
@@ -1535,7 +1575,6 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
struct iocb *iocb, bool compat)
{
struct aio_kiocb *req;
- struct file *file;
ssize_t ret;
/* enforce forwards compatibility on users */
@@ -1558,16 +1597,6 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
if (unlikely(!req))
return -EAGAIN;
- req->common.ki_filp = file = fget(iocb->aio_fildes);
- if (unlikely(!req->common.ki_filp)) {
- ret = -EBADF;
- goto out_put_req;
- }
- req->common.ki_pos = iocb->aio_offset;
- req->common.ki_complete = aio_complete;
- req->common.ki_flags = iocb_flags(req->common.ki_filp);
- req->common.ki_hint = file_write_hint(file);
-
if (iocb->aio_flags & IOCB_FLAG_RESFD) {
/*
* If the IOCB_FLAG_RESFD flag of aio_flags is set, get an
@@ -1581,14 +1610,6 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
req->ki_eventfd = NULL;
goto out_put_req;
}
-
- req->common.ki_flags |= IOCB_EVENTFD;
- }
-
- ret = kiocb_set_rw_flags(&req->common, iocb->aio_rw_flags);
- if (unlikely(ret)) {
- pr_debug("EINVAL: aio_rw_flags\n");
- goto out_put_req;
}
ret = put_user(KIOCB_KEY, &user_iocb->aio_key);
@@ -1602,16 +1623,16 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
switch (iocb->aio_lio_opcode) {
case IOCB_CMD_PREAD:
- ret = aio_read(&req->common, iocb, false, compat);
+ ret = aio_read(&req->rw, iocb, false, compat);
break;
case IOCB_CMD_PWRITE:
- ret = aio_write(&req->common, iocb, false, compat);
+ ret = aio_write(&req->rw, iocb, false, compat);
break;
case IOCB_CMD_PREADV:
- ret = aio_read(&req->common, iocb, true, compat);
+ ret = aio_read(&req->rw, iocb, true, compat);
break;
case IOCB_CMD_PWRITEV:
- ret = aio_write(&req->common, iocb, true, compat);
+ ret = aio_write(&req->rw, iocb, true, compat);
break;
default:
pr_debug("invalid aio operation %d\n", iocb->aio_lio_opcode);
@@ -1625,7 +1646,9 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
out_put_req:
put_reqs_available(ctx, 1);
percpu_ref_put(&ctx->reqs);
- kiocb_free(req);
+ if (req->ki_eventfd)
+ eventfd_ctx_put(req->ki_eventfd);
+ kmem_cache_free(kiocb_cachep, req);
return ret;
}
--
2.17.0
--
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
* [PATCH 4/7] aio: remove the extra get_file/fput pair in io_submit_one
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
If we release the lockdep write protection token before calling into
->write_iter and thus never access the file pointer after an -EIOCBQUEUED
return from ->write_iter or ->read_iter we don't need this extra
reference.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 18507743757a..d7be32cdd1db 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1515,16 +1515,17 @@ static ssize_t aio_write(struct kiocb *req, struct iocb *iocb, bool vectored,
return ret;
ret = rw_verify_area(WRITE, file, &req->ki_pos, iov_iter_count(&iter));
if (!ret) {
- req->ki_flags |= IOCB_WRITE;
- file_start_write(file);
- ret = aio_ret(req, call_write_iter(file, req, &iter));
/*
* We release freeze protection in aio_complete(). Fool lockdep
* by telling it the lock got released so that it doesn't
* complain about held lock when we return to userspace.
*/
- if (S_ISREG(file_inode(file)->i_mode))
+ if (S_ISREG(file_inode(file)->i_mode)) {
+ __sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, true);
__sb_writers_release(file_inode(file)->i_sb, SB_FREEZE_WRITE);
+ }
+ req->ki_flags |= IOCB_WRITE;
+ ret = aio_ret(req, call_write_iter(file, req, &iter));
}
kfree(iovec);
return ret;
@@ -1599,7 +1600,6 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
req->ki_user_iocb = user_iocb;
req->ki_user_data = iocb->aio_data;
- get_file(file);
switch (iocb->aio_lio_opcode) {
case IOCB_CMD_PREAD:
ret = aio_read(&req->common, iocb, false, compat);
@@ -1618,7 +1618,6 @@ static int io_submit_one(struct kioctx *ctx, struct iocb __user *user_iocb,
ret = -EINVAL;
break;
}
- fput(file);
if (ret && ret != -EIOCBQUEUED)
goto out_put_req;
--
2.17.0
^ permalink raw reply related
* [PATCH 3/7] aio: sanitize ki_list handling
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
Instead of handcoded non-null checks always initialize ki_list to an
empty list and use list_empty / list_empty_careful on it. While we're
at it also error out on a double call to kiocb_set_cancel_fn instead
of ignoring it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/aio.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 7c1855afd723..18507743757a 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -553,13 +553,12 @@ void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
struct kioctx *ctx = req->ki_ctx;
unsigned long flags;
- spin_lock_irqsave(&ctx->ctx_lock, flags);
-
- if (!req->ki_list.next)
- list_add(&req->ki_list, &ctx->active_reqs);
+ if (WARN_ON_ONCE(!list_empty(&req->ki_list)))
+ return;
+ spin_lock_irqsave(&ctx->ctx_lock, flags);
+ list_add_tail(&req->ki_list, &ctx->active_reqs);
req->ki_cancel = cancel;
-
spin_unlock_irqrestore(&ctx->ctx_lock, flags);
}
EXPORT_SYMBOL(kiocb_set_cancel_fn);
@@ -1039,7 +1038,7 @@ static inline struct aio_kiocb *aio_get_req(struct kioctx *ctx)
goto out_put;
percpu_ref_get(&ctx->reqs);
-
+ INIT_LIST_HEAD(&req->ki_list);
req->ki_ctx = ctx;
return req;
out_put:
@@ -1107,7 +1106,7 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
file_end_write(file);
}
- if (iocb->ki_list.next) {
+ if (!list_empty_careful(&iocb->ki_list)) {
unsigned long flags;
spin_lock_irqsave(&ctx->ctx_lock, flags);
--
2.17.0
--
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
* [PATCH 2/7] aio: remove an outdated BUG_ON and comment in aio_complete
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
These days we don't treat sync iocbs special in the aio completion code as
they never use it. Remove the old comment and BUG_ON given that the
current definition of is_sync_kiocb makes it impossible to hit.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/aio.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index add46b06be86..7c1855afd723 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1107,15 +1107,6 @@ static void aio_complete(struct kiocb *kiocb, long res, long res2)
file_end_write(file);
}
- /*
- * Special case handling for sync iocbs:
- * - events go directly into the iocb for fast handling
- * - the sync task with the iocb in its stack holds the single iocb
- * ref, no other paths have a way to get another ref
- * - the sync task helpfully left a reference to itself in the iocb
- */
- BUG_ON(is_sync_kiocb(kiocb));
-
if (iocb->ki_list.next) {
unsigned long flags;
--
2.17.0
--
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
* [PATCH 1/7] aio: don't print the page size at boot time
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
In-Reply-To: <20180415150108.1341-1-hch@lst.de>
The page size is in no way related to the aio code, and printing it in
the (debug) dmesg at every boot serves no purpose.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Jeff Moyer <jmoyer@redhat.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/aio.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/aio.c b/fs/aio.c
index 88d7927ffbc6..add46b06be86 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -264,9 +264,6 @@ static int __init aio_setup(void)
kiocb_cachep = KMEM_CACHE(aio_kiocb, SLAB_HWCACHE_ALIGN|SLAB_PANIC);
kioctx_cachep = KMEM_CACHE(kioctx,SLAB_HWCACHE_ALIGN|SLAB_PANIC);
-
- pr_debug("sizeof(struct page) = %zu\n", sizeof(struct page));
-
return 0;
}
__initcall(aio_setup);
--
2.17.0
^ permalink raw reply related
* io_pgetevents & aio fsync V3
From: Christoph Hellwig @ 2018-04-15 15:01 UTC (permalink / raw)
To: viro; +Cc: Avi Kivity, linux-aio, linux-fsdevel, linux-api, linux-kernel
Hi all,
this patch adds workqueue based fsync offload. Version of this
patch have been floating around for a couple years, but we now
have a user with seastar used by ScyllaDB (who sponsored this
work) that really wants this in addition to the aio poll support.
More details are in the patch itself.
Because the iocb types have been defined sine day one (and probably
were supported by RHEL3) libaio already supports these calls as-is.
This also pulls in the aio cleanups and io_pgetevents support previously
submitted and review as part of the aio poll series. The aio poll
series will be resubmitted on top of this series
A git tree is available here:
git://git.infradead.org/users/hch/vfs.git aio-fsync.3
Gitweb:
http://git.infradead.org/users/hch/vfs.git/shortlog/refs/heads/aio-fsync.3
Changes since V2:
- don't introduce a use after free for lockdep sb release tracking
- set up list for cancellation before I/O submission
Changes since V1:
- remove a BUG_ON_ONE(is_sync_kiocb(kiocb));
- moved cancellation patches to the poll series
- improve a list_empty check
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Andy Lutomirski @ 2018-04-14 22:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Mathieu Desnoyers, Peter Zijlstra, Paul E . McKenney, Boqun Feng,
Dave Watson, Linux Kernel Mailing List, Linux API, Paul Turner,
Andrew Morton, Russell King, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, Andrew Hunter, Andi Kleen, Chris Lameter,
Ben Maurer, Steven Rostedt, Josh Triplett, Catalin Marinas
In-Reply-To: <CA+55aFxcJ2V_1BoW0Cct=Kkp7REkgiU_BTimGdmoFmWHaWaZ-g@mail.gmail.com>
On Thu, Apr 12, 2018 at 12:43 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Apr 12, 2018 at 12:27 PM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> The cpu_opv system call executes a vector of operations on behalf of
>> user-space on a specific CPU with preemption disabled. It is inspired
>> by readv() and writev() system calls which take a "struct iovec"
>> array as argument.
>
> Do we really want the page pinning?
>
> This whole cpu_opv thing is the most questionable part of the series,
> and the page pinning is the most questionable part of cpu_opv for me.
>
> Can we plan on merging just the plain rseq parts *without* this all
> first, and then see the cpu_opv thing as a "maybe future expansion"
> part.
>
> I think that would make Andy happier too.
>
It only makes me happier if the userspace code involved is actually
going to work when single-stepped, which might actually be the case
(fingers crossed). That being said, I'm not really convinced that
cpu_opv() makes much difference here, since I'm not entirely convinced
that user code will actually use it or that user code will actually be
that well tested. C'est la vie.
^ permalink raw reply
* Re: [PATCH 24/24] debugfs: Restrict debugfs when the kernel is locked down
From: Pavel Machek @ 2018-04-13 20:22 UTC (permalink / raw)
To: David Howells
Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
linux-security-module
In-Reply-To: <152346403637.4030.15247096217928429102.stgit@warthog.procyon.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]
On Wed 2018-04-11 17:27:16, David Howells wrote:
> Disallow opening of debugfs files that might be used to muck around when
> the kernel is locked down as various drivers give raw access to hardware
> through debugfs. Given the effort of auditing all 2000 or so files and
> manually fixing each one as necessary, I've chosen to apply a heuristic
> instead. The following changes are made:
>
> (1) chmod and chown are disallowed on debugfs objects (though the root dir
> can be modified by mount and remount, but I'm not worried about that).
This has nothing to do with the lockdown goals, right? I find chown of
such files quite nice, to allow debugging without doing sudo all the time.
> (2) When the kernel is locked down, only files with the following criteria
> are permitted to be opened:
>
> - The file must have mode 00444
> - The file must not have ioctl methods
> - The file must not have mmap
Dunno. Would not it be nicer to go through the debugfs files and split
them into safe/unsafe varieties?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 07/24] hibernate: Disable when the kernel is locked down
From: Pavel Machek @ 2018-04-13 20:22 UTC (permalink / raw)
To: David Howells
Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
linux-security-module
In-Reply-To: <152346392521.4030.5108539377959227838.stgit@warthog.procyon.org.uk>
[-- Attachment #1: Type: text/plain, Size: 624 bytes --]
On Wed 2018-04-11 17:25:25, David Howells wrote:
> From: Josh Boyer <jwboyer@fedoraproject.org>
>
> There is currently no way to verify the resume image when returning
> from hibernate. This might compromise the signed modules trust model,
> so until we can work with signed hibernate images we disable it when the
> kernel is locked down.
I'd rather see hibernation fixed than disabled like this.
I believe Jiri Kosina may have some patches for that?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH 02/24] Add a SysRq option to lift kernel lockdown
From: Pavel Machek @ 2018-04-13 20:22 UTC (permalink / raw)
To: David Howells
Cc: torvalds, linux-man, linux-api, jmorris, linux-kernel,
linux-security-module
In-Reply-To: <152346389240.4030.11187964053014260180.stgit@warthog.procyon.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1790 bytes --]
On Wed 2018-04-11 17:24:52, David Howells wrote:
> From: Kyle McMartin <kyle@redhat.com>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
>
> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.
>
> Signed-off-by: Kyle McMartin <kyle@redhat.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: x86@kernel.org
Is that good idea? Magic sysrq was meant for debugging, not for
toggling options like that. Distros are expected to turn it off.
It also works over serial consoles etc, being able to toggle security
options from serial is surprising...
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -487,6 +487,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
> /* x: May be registered on mips for TLB dump */
> /* x: May be registered on ppc/powerpc for xmon */
> /* x: May be registered on sparc64 for global PMU dump */
> + /* x: May be registered on x86_64 for disabling secure boot */
> NULL, /* x */
What about x86-32?
> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> + .handler = sysrq_handle_lockdown_lift,
> + .help_msg = "unSB(x)",
> + .action_msg = "Disabling Secure Boot restrictions",
> + .enable_mask = SYSRQ_DISABLE_USERSPACE,
> +};
I'd remove secure boot mentions here.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Mathieu Desnoyers @ 2018-04-13 18:06 UTC (permalink / raw)
To: Linus Torvalds
Cc: Peter Zijlstra, Paul E. McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon
In-Reply-To: <CA+55aFwXCnnfrOjPj5SB=G34xgveo1jqNyf23YNt6jOKX_VdGw@mail.gmail.com>
----- On Apr 13, 2018, at 12:37 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
> On Fri, Apr 13, 2018 at 5:16 AM, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>> The vmalloc space needed by cpu_opv is bound by the number of pages
>> a cpu_opv call can touch.
>
> No it's not.
>
> You can have a thousand different processes doing cpu_opv at the same time.
>
> A *single* cpu_opv may me limited toi "only" a megabyte, but I'm not
> seeing any global limit anywhere.
>
> In short, this looks like a guaranteed DoS approach to me.
Right, so one simple approach to solve this is to limit to the number
of concurrent cpu_opv executed at any given time.
Considering that cpu_opv is a slow path, we can limit the number
of concurrent cpu_opv executions by protecting this with a global
mutex, or a semaphore if we want the number of concurrent executions
to be greater than 1.
Another approach if we want to be fancier is to keep track of the
amount of vma address space currently used by all in-flight cpu_opv.
Beyond a given threshold, further execution of additional cpu_opv
instances would block, awaiting to be woken up when vmalloc address
space is freed when in-flight cpu_opv complete.
What global vmalloc address-space budget should we aim for ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [RFC PATCH for 4.18 12/23] cpu_opv: Provide cpu_opv system call (v7)
From: Linus Torvalds @ 2018-04-13 16:37 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Peter Zijlstra, Paul E. McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson, linux-kernel, linux-api, Paul Turner, Andrew Morton,
Russell King, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Andrew Hunter, Andi Kleen, Chris Lameter, Ben Maurer, rostedt,
Josh Triplett, Catalin Marinas, Will Deacon
In-Reply-To: <625160026.9658.1523621809662.JavaMail.zimbra@efficios.com>
On Fri, Apr 13, 2018 at 5:16 AM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> The vmalloc space needed by cpu_opv is bound by the number of pages
> a cpu_opv call can touch.
No it's not.
You can have a thousand different processes doing cpu_opv at the same time.
A *single* cpu_opv may me limited toi "only" a megabyte, but I'm not
seeing any global limit anywhere.
In short, this looks like a guaranteed DoS approach to me.
Linus
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Jann Horn @ 2018-04-13 16:17 UTC (permalink / raw)
To: Michal Hocko
Cc: Michael Kerrisk (man-pages), John Hubbard, linux-man,
Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <CAG48ez3-xtmAt2EpRFR8GNKKPcsDsyg7XdwQ=D5w3Ym6w4Krjw@mail.gmail.com>
On Fri, Apr 13, 2018 at 6:05 PM, Jann Horn <jannh@google.com> wrote:
> On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <mhocko@kernel.org> wrote:
>> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <mhocko@kernel.org> wrote:
>>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>>> > [...]
>>> >> So, you mean remove this entire paragraph:
>>> >>
>>> >> For cases in which the specified memory region has not been
>>> >> reserved using an existing mapping, newer kernels (Linux
>>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>>> >> should be used instead; older kernels require the caller to
>>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>>> >> action if the kernel places the new mapping at a different
>>> >> address.
>>> >>
>>> >> It seems like some version of the first half of the paragraph is worth
>>> >> keeping, though, so as to point the reader in the direction of a remedy.
>>> >> How about replacing that text with the following:
>>> >>
>>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>>> >> in a multithreaded program to avoid the hazard described
>>> >> above.
>>> >
>>> > Yes, that sounds reasonable to me.
>>>
>>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>>> when actually, you just have to call mmap() with the address as hint,
>>> and if mmap() returns a different address, munmap() it and go on your
>>> normal error path.
>>
>> This is still racy in multithreaded application which is the main point
>> of the whole section, no?
>
> No, it isn't.
mmap() with a hint (without MAP_FIXED) will always non-racily allocate
a memory region for you or return an error code. If it does allocate a
memory region, it belongs to you until you deallocate it. It might be
at a different address than you requested - in that case you can
emulate MAP_FIXED_NOREPLACE by calling munmap() and treating it as an
error; or you can do something else with it.
MAP_FIXED_NOREPLACE is just a performance optimization.
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Jann Horn @ 2018-04-13 16:05 UTC (permalink / raw)
To: Michal Hocko
Cc: Michael Kerrisk (man-pages), John Hubbard, linux-man,
Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <20180413160435.GA17484@dhcp22.suse.cz>
On Fri, Apr 13, 2018 at 6:04 PM, Michal Hocko <mhocko@kernel.org> wrote:
> On Fri 13-04-18 17:04:09, Jann Horn wrote:
>> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <mhocko@kernel.org> wrote:
>> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
>> > [...]
>> >> So, you mean remove this entire paragraph:
>> >>
>> >> For cases in which the specified memory region has not been
>> >> reserved using an existing mapping, newer kernels (Linux
>> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
>> >> should be used instead; older kernels require the caller to
>> >> use addr as a hint (without MAP_FIXED) and take appropriate
>> >> action if the kernel places the new mapping at a different
>> >> address.
>> >>
>> >> It seems like some version of the first half of the paragraph is worth
>> >> keeping, though, so as to point the reader in the direction of a remedy.
>> >> How about replacing that text with the following:
>> >>
>> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
>> >> in a multithreaded program to avoid the hazard described
>> >> above.
>> >
>> > Yes, that sounds reasonable to me.
>>
>> But that kind of sounds as if you can't avoid it before Linux 4.17,
>> when actually, you just have to call mmap() with the address as hint,
>> and if mmap() returns a different address, munmap() it and go on your
>> normal error path.
>
> This is still racy in multithreaded application which is the main point
> of the whole section, no?
No, it isn't.
^ permalink raw reply
* Re: [PATCH] mmap.2: MAP_FIXED is okay if the address range has been reserved
From: Michal Hocko @ 2018-04-13 16:04 UTC (permalink / raw)
To: Jann Horn
Cc: Michael Kerrisk (man-pages), John Hubbard, linux-man,
Andrew Morton, Linux-MM, lkml, Linux API
In-Reply-To: <CAG48ez2w+3FDh9LM3+P2EHowicjM2Xw6giR6uq=26JfWHYsTAQ@mail.gmail.com>
On Fri 13-04-18 17:04:09, Jann Horn wrote:
> On Fri, Apr 13, 2018 at 8:49 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > On Fri 13-04-18 08:43:27, Michael Kerrisk wrote:
> > [...]
> >> So, you mean remove this entire paragraph:
> >>
> >> For cases in which the specified memory region has not been
> >> reserved using an existing mapping, newer kernels (Linux
> >> 4.17 and later) provide an option MAP_FIXED_NOREPLACE that
> >> should be used instead; older kernels require the caller to
> >> use addr as a hint (without MAP_FIXED) and take appropriate
> >> action if the kernel places the new mapping at a different
> >> address.
> >>
> >> It seems like some version of the first half of the paragraph is worth
> >> keeping, though, so as to point the reader in the direction of a remedy.
> >> How about replacing that text with the following:
> >>
> >> Since Linux 4.17, the MAP_FIXED_NOREPLACE flag can be used
> >> in a multithreaded program to avoid the hazard described
> >> above.
> >
> > Yes, that sounds reasonable to me.
>
> But that kind of sounds as if you can't avoid it before Linux 4.17,
> when actually, you just have to call mmap() with the address as hint,
> and if mmap() returns a different address, munmap() it and go on your
> normal error path.
This is still racy in multithreaded application which is the main point
of the whole section, no?
--
Michal Hocko
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox