* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Stefan Wahren @ 2015-08-10 11:24 UTC (permalink / raw)
To: Srinivas Kandagatla, Andrew Lunn
Cc: wxt, linux-api, Rob Herring, sboyd, arnd, linux-arm-kernel,
khilman, s.hauer, Greg Kroah-Hartman, linux-kernel, linux-arm-msm,
mporter, pantelis.antoniou, devicetree, Mark Brown,
maitysanchayan, p.zabel
In-Reply-To: <55C87EEF.6020609@linaro.org>
Hi Srinivas,
hi Andrew,
[add Sanchayan and Philipp]
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> hat am 10. August 2015 um
> 12:37 geschrieben:
>
>
> Hi Andrew,
>
> On 09/08/15 16:19, Andrew Lunn wrote:
> > Hi Srinivas
> >
> > The AT24 eeprom driver contains the comment:
> >
> > /*
> > * Export the EEPROM bytes through sysfs, since that's convenient.
> > * By default, only root should see the data (maybe passwords etc)
> > */
> >
> > and as you would expect from this:
> >
> > # ls -l
> > ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/eeprom
> > -rw------- 1 root root 512 Aug 9 10:16
> > ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/eeprom
> >
> > The AT25 and the MAX6875 driver are the same.
> >
> > However nvmem has different defaults:
> >
> > # ls -l
> > ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/0-00500/nvmem
> > -rw-r--r-- 1 root root 0 Aug 9 10:16
> > ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/0-00500/nvmem
> >
> Yes, nvmem by default sets the read permission for everyone S_IRUGO.
> Which was the most common use case.
i can only speak for mxs-ocotp, but root-only access is sufficient.
>
> > Has this been considered and discussed?
> Thanks for bringing this up, No, we did not discuss this explicitly, but
> it should not be an issue to accommodate this. Provider still have
> flexibility to configure such things.
IMHO it would better the default settings of the framework would be compatible
and same restrictive as the eeprom drivers.
In case a provider needs more privileges, it should be his job.
>
> As a first patchset we wanted to keep the framework simple and add
> features as we move on.
That's true, but it's an ABI too. So changing default settings soon is better.
Regards
Stefan
>
> Thanks,
> srini
> >
> > Thanks
> > Andrew
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Srinivas Kandagatla @ 2015-08-10 10:37 UTC (permalink / raw)
To: Andrew Lunn
Cc: stefan.wahren-eS4NqCHxEME, devicetree-u79uwXL29TY76Z2rM5mHXA,
arnd-r2nGTMty4D4, khilman-DgEjT+Ai2ygdnm+yROfE0A,
Greg Kroah-Hartman, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
mporter-OWPKS81ov/FWk0Htik3J/w,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Rob Herring, Mark Brown,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wxt-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <20150809151916.GF14271-g2DYL2Zd6BY@public.gmane.org>
Hi Andrew,
On 09/08/15 16:19, Andrew Lunn wrote:
> Hi Srinivas
>
> The AT24 eeprom driver contains the comment:
>
> /*
> * Export the EEPROM bytes through sysfs, since that's convenient.
> * By default, only root should see the data (maybe passwords etc)
> */
>
> and as you would expect from this:
>
> # ls -l ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/eeprom
> -rw------- 1 root root 512 Aug 9 10:16 ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/eeprom
>
> The AT25 and the MAX6875 driver are the same.
>
> However nvmem has different defaults:
>
> # ls -l ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/0-00500/nvmem
> -rw-r--r-- 1 root root 0 Aug 9 10:16 ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/0-00500/nvmem
>
Yes, nvmem by default sets the read permission for everyone S_IRUGO.
Which was the most common use case.
> Has this been considered and discussed?
Thanks for bringing this up, No, we did not discuss this explicitly, but
it should not be an issue to accommodate this. Provider still have
flexibility to configure such things.
As a first patchset we wanted to keep the framework simple and add
features as we move on.
Thanks,
srini
>
> Thanks
> Andrew
>
^ permalink raw reply
* Re: [PATCH] eventfd: implementation of EFD_MASK flag
From: Damian Hobson-Garcia @ 2015-08-10 8:57 UTC (permalink / raw)
To: Martin Sustrik
Cc: viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <45138eb2cdd715f14fbd01fb5c8d3655-NG+5sAMe/NuLHBE00WrnfQ@public.gmane.org>
Hi Martin,
Thanks for your comments.
On 2015-08-10 3:39 PM, Martin Sustrik wrote:
> On 2015-08-10 08:23, Damian Hobson-Garcia wrote:
>> Replying to my own post, but I had the following comments/questions.
>> Martin, if you have any response to my comments I would be very happy to
>> hear them.
>>
>> On 2015-08-10 2:51 PM, Damian Hobson-Garcia wrote:
>>> From: Martin Sustrik <sustrik-8XN41Hm7QBXQT0dZR+AlfA@public.gmane.org>
>>>
>> [snip]
>>>
>>> write(2):
>>>
>>> User is allowed to write only buffers containing the following
>>> structure:
>>>
>>> struct efd_mask {
>>> __u32 events;
>>> __u64 data;
>>> };
>>>
>>> The value of 'events' should be any combination of event flags as
>>> defined by
>>> poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified
>>> events will
>>> be signaled when polling (select, poll, epoll) on the eventfd is done
>>> later on.
>>> 'data' is opaque data that are not interpreted by eventfd object.
>>>
>> I'm not fully clear on the purpose that the 'data' member serves. Does
>> this opaque handle need to be tied together with this event
>> synchronization construct?
>
> It's a convenience thing. Imagine you are implementing your own file
> descriptor type in user space. You create an EFD_MASK socket and a
> structure that will hold any state that you need for the socket (tx/rx
> buffers and such).
>
> Now you have two things to pass around. If you want to pass the fd to a
> function, it must have two parameters (fd and pointer to the structure).
>
> To fix it you can put the fd into the structure. That way there's only
> one thing to pass around (the structure).
>
> The problem with that approach is when you have generic code that deals
> with file descriptors. For example, a simple poller which accepts a list
> of (fd, callback) pairs and invokes the callback when one of the fds
> signals POLLIN. You can't send a pointer to a structure to such
> function. All you can send is the fd, but then, when the callback is
> invoked, fd is all you have. You have no idea where your state is.
>
> 'data' member allows you to put the pointer to the state to the socket
> itself. Thus, if you have a fd, you can always find out where the
> associated data is by reading the mask structure from the fd.
>
Ok, I see what you're saying. I guess that keeping track of the mapping
between the fd and the struct in user space could be non-trivial if
there are a large number of active fds that are polling very frequently.
Wouldn't it be sufficient to just use epoll() in this case though? It
already seems to support this kind of thing.
>>
>> [snip]
>>
>>> @@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
>>> {
>>> + /* This function should never be used with eventfd in the mask
>>> mode. */
>>> + BUG_ON(ctx->flags & EFD_MASK);
>>> +
>> ...
>>> @@ -158,6 +180,9 @@ int eventfd_ctx_remove_wait_queue(struct
>>> eventfd_ctx *ctx, wait_queue_t *wait,
>>> {
>>> + /* This function should never be used with eventfd in the mask
>>> mode. */
>>> + BUG_ON(ctx->flags & EFD_MASK);
>>> +
>> ...
>>> @@ -188,6 +213,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx,
>>> int no_wait, __u64 *cnt)
>>> + /* This function should never be used with eventfd in the mask
>>> mode. */
>>> + BUG_ON(ctx->flags & EFD_MASK);
>>> +
>>
>> If eventfd_ctx_fileget() returns EINVAL when EFD_MASK is set, I don't
>> think that there will be a way to call these functions in the mask mode,
>> so it should be possible to get rid of the BUG_ON checks.
>
> Sure. Feel free to do so.
>
>>
>> [snip]
>>> @@ -230,6 +258,19 @@ static ssize_t eventfd_read(struct file *file,
>>> char __user *buf, size_t count,
>>> ssize_t res;
>>> __u64 cnt;
>>>
>>> + if (ctx->flags & EFD_MASK) {
>>> + struct efd_mask mask;
>>> +
>>> + if (count < sizeof(mask))
>>> + return -EINVAL;
>>> + spin_lock_irq(&ctx->wqh.lock);
>>> + mask = ctx->mask;
>>> + spin_unlock_irq(&ctx->wqh.lock);
>>> + if (copy_to_user(buf, &mask, sizeof(mask)))
>>> + return -EFAULT;
>>> + return sizeof(mask);
>>> + }
>>> +
>>
>> For the other eventfd modes, reading the value will update the internal
>> state of the eventfd (either clearing or decrementing the counter).
>> Should something similar be done here? I'm thinking of a case where a
>> process is polling on this fd in a loop. Clearing the efd_mask data on
>> read should provide an easy way for the polling process to know if it is
>> seeing new poll events.
>
> No. In this case reading the value has no effect on the state of the fd.
> How it should work is rather:
>
> // fd is in POLLIN state
> poll(fd);
> // function exits with POLLIN but fd remains in POLLIN state
> my_recv(fd, buf, size);
> // my_recv function have found out that there's no more data to recv and
> switched off the POLLIN flag
> poll(fd); // we block here waiting for more data to arrive from the network
>
How exactly doe the receiver switch off the POLLIN flag? Does the
receiver also write to the eventfd? or do you mean that it just doesn't
set POLLIN in the events mask? It seems cleaner to have the sender only
write the eventfd and the receiver only read it. Your example would be
exactly the same, except that my_recv(fd, buf, size) would read to clear
instead of write.
>>
>>> @@ -292,8 +351,13 @@ static void eventfd_show_fdinfo(struct seq_file
>>> *m, struct file *f)
>>> struct eventfd_ctx *ctx = f->private_data;
>>>
>>> spin_lock_irq(&ctx->wqh.lock);
>>> - seq_printf(m, "eventfd-count: %16llx\n",
>>> - (unsigned long long)ctx->count);
>>> + if (ctx->flags & EFD_MASK) {
>>> + seq_printf(m, "eventfd-mask: %x\n",
>>> + (unsigned)ctx->mask.events);
>>> + } else {
>>> + seq_printf(m, "eventfd-count: %16llx\n",
>>> + (unsigned long long)ctx->count);
>>> + }
>>> spin_unlock_irq(&ctx->wqh.lock);
>>> }
>> I think that putting the EFD_MASK functionality into a different fops
>> structure might be useful for reducing the number of if statements.
>
> Sure. No objections.
>
> Thanks for re-submitting the patch!
My pleasure.
> Martin
>
Damian
^ permalink raw reply
* [PATCHv4 1/1] Documentation: describe how to add a system call
From: David Drysdale @ 2015-08-10 8:00 UTC (permalink / raw)
To: linux-api, Michael Kerrisk, Andrew Morton, Arnd Bergmann,
Shuah Khan, Jonathan Corbet, Eric B Munson, Randy Dunlap,
Cyril Hrubis, Josh Triplett, Pavel Machek
Cc: Andrea Arcangeli, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Peter Zijlstra,
Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Milosz Tanski, Fam Zheng,
Mathieu Desnoyers, linux-doc, linux-kernel, David Drysdale
In-Reply-To: <1439193644-4773-1-git-send-email-drysdale@google.com>
Add a document describing the process of adding a new system call,
including the need for a flags argument for future compatibility, and
covering 32-bit/64-bit concerns (albeit in an x86-centric way).
Signed-off-by: David Drysdale <drysdale@google.com>
Reviewed-by: Michael Kerrisk <mtk.manpages@gmail.com>
Reviewed-by: Eric B Munson <emunson@akamai.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Reviewed-by: Josh Triplett <josh@joshtriplett.org>
---
Documentation/adding-syscalls.txt | 527 ++++++++++++++++++++++++++++++++++++++
1 file changed, 527 insertions(+)
create mode 100644 Documentation/adding-syscalls.txt
diff --git a/Documentation/adding-syscalls.txt b/Documentation/adding-syscalls.txt
new file mode 100644
index 000000000000..cc2d4ac4f404
--- /dev/null
+++ b/Documentation/adding-syscalls.txt
@@ -0,0 +1,527 @@
+Adding a New System Call
+========================
+
+This document describes what's involved in adding a new system call to the
+Linux kernel, over and above the normal submission advice in
+Documentation/SubmittingPatches.
+
+
+System Call Alternatives
+------------------------
+
+The first thing to consider when adding a new system call is whether one of
+the alternatives might be suitable instead. Although system calls are the
+most traditional and most obvious interaction points between userspace and the
+kernel, there are other possibilities -- choose what fits best for your
+interface.
+
+ - If the operations involved can be made to look like a filesystem-like
+ object, it may make more sense to create a new filesystem or device. This
+ also makes it easier to encapsulate the new functionality in a kernel module
+ rather than requiring it to be built into the main kernel.
+ - If the new functionality involves operations where the kernel notifies
+ userspace that something has happened, then returning a new file
+ descriptor for the relevant object allows userspace to use
+ poll/select/epoll to receive that notification.
+ - However, operations that don't map to read(2)/write(2)-like operations
+ have to be implemented as ioctl(2) requests, which can lead to a
+ somewhat opaque API.
+ - If you're just exposing runtime system information, a new node in sysfs
+ (see Documentation/filesystems/sysfs.txt) or the /proc filesystem may be
+ more appropriate. However, access to these mechanisms requires that the
+ relevant filesystem is mounted, which might not always be the case (e.g.
+ in a namespaced/sandboxed/chrooted environment). Avoid adding any API to
+ debugfs, as this is not considered a 'production' interface to userspace.
+ - If the operation is specific to a particular file or file descriptor, then
+ an additional fcntl(2) command option may be more appropriate. However,
+ fcntl(2) is a multiplexing system call that hides a lot of complexity, so
+ this option is best for when the new function is closely analogous to
+ existing fcntl(2) functionality, or the new functionality is very simple
+ (for example, getting/setting a simple flag related to a file descriptor).
+ - If the operation is specific to a particular task or process, then an
+ additional prctl(2) command option may be more appropriate. As with
+ fcntl(2), this system call is a complicated multiplexor so is best reserved
+ for near-analogs of existing prctl() commands or getting/setting a simple
+ flag related to a process.
+
+
+Designing the API: Planning for Extension
+-----------------------------------------
+
+A new system call forms part of the API of the kernel, and has to be supported
+indefinitely. As such, it's a very good idea to explicitly discuss the
+interface on the kernel mailing list, and it's important to plan for future
+extensions of the interface.
+
+(The syscall table is littered with historical examples where this wasn't done,
+together with the corresponding follow-up system calls -- eventfd/eventfd2,
+dup2/dup3, inotify_init/inotify_init1, pipe/pipe2, renameat/renameat2 -- so
+learn from the history of the kernel and plan for extensions from the start.)
+
+For simpler system calls that only take a couple of arguments, the preferred
+way to allow for future extensibility is to include a flags argument to the
+system call. To make sure that userspace programs can safely use flags
+between kernel versions, check whether the flags value holds any unknown
+flags, and reject the system call (with EINVAL) if it does:
+
+ if (flags & ~(THING_FLAG1 | THING_FLAG2 | THING_FLAG3))
+ return -EINVAL;
+
+(If no flags values are used yet, check that the flags argument is zero.)
+
+For more sophisticated system calls that involve a larger number of arguments,
+it's preferred to encapsulate the majority of the arguments into a structure
+that is passed in by pointer. Such a structure can cope with future extension
+by including a size argument in the structure:
+
+ struct xyzzy_params {
+ u32 size; /* userspace sets p->size = sizeof(struct xyzzy_params) */
+ u32 param_1;
+ u64 param_2;
+ u64 param_3;
+ };
+
+As long as any subsequently added field, say param_4, is designed so that a
+zero value gives the previous behaviour, then this allows both directions of
+version mismatch:
+
+ - To cope with a later userspace program calling an older kernel, the kernel
+ code should check that any memory beyond the size of the structure that it
+ expects is zero (effectively checking that param_4 == 0).
+ - To cope with an older userspace program calling a newer kernel, the kernel
+ code can zero-extend a smaller instance of the structure (effectively
+ setting param_4 = 0).
+
+See perf_event_open(2) and the perf_copy_attr() function (in
+kernel/events/core.c) for an example of this approach.
+
+
+Designing the API: Other Considerations
+---------------------------------------
+
+If your new system call allows userspace to refer to a kernel object, it
+should use a file descriptor as the handle for that object -- don't invent a
+new type of userspace object handle when the kernel already has mechanisms and
+well-defined semantics for using file descriptors.
+
+If your new xyzzy(2) system call does return a new file descriptor, then the
+flags argument should include a value that is equivalent to setting O_CLOEXEC
+on the new FD. This makes it possible for userspace to close the timing
+window between xyzzy() and calling fcntl(fd, F_SETFD, FD_CLOEXEC), where an
+unexpected fork() and execve() in another thread could leak a descriptor to
+the exec'ed program. (However, resist the temptation to re-use the actual value
+of the O_CLOEXEC constant, as it is architecture-specific and is part of a
+numbering space of O_* flags that is fairly full.)
+
+If your system call returns a new file descriptor, you should also consider
+what it means to use the poll(2) family of system calls on that file
+descriptor. Making a file descriptor ready for reading or writing is the
+normal way for the kernel to indicate to userspace that an event has
+occurred on the corresponding kernel object.
+
+If your new xyzzy(2) system call involves a filename argument:
+
+ int sys_xyzzy(const char __user *path, ..., unsigned int flags);
+
+you should also consider whether an xyzzyat(2) version is more appropriate:
+
+ int sys_xyzzyat(int dfd, const char __user *path, ..., unsigned int flags);
+
+This allows more flexibility for how userspace specifies the file in question;
+in particular it allows userspace to request the functionality for an
+already-opened file descriptor using the AT_EMPTY_PATH flag, effectively giving
+an fxyzzy(3) operation for free:
+
+ - xyzzyat(AT_FDCWD, path, ..., 0) is equivalent to xyzzy(path,...)
+ - xyzzyat(fd, "", ..., AT_EMPTY_PATH) is equivalent to fxyzzy(fd, ...)
+
+(For more details on the rationale of the *at() calls, see the openat(2) man
+page; for an example of AT_EMPTY_PATH, see the statat(2) man page.)
+
+If your new xyzzy(2) system call involves a parameter describing an offset
+within a file, make its type loff_t so that 64-bit offsets can be supported
+even on 32-bit architectures.
+
+If your new xyzzy(2) system call involves privileged functionality, it needs
+to be governed by the appropriate Linux capability bit (checked with a call to
+capable()), as described in the capabilities(7) man page. Choose an existing
+capability bit that governs related functionality, but try to avoid combining
+lots of only vaguely related functions together under the same bit, as this
+goes against capabilities' purpose of splitting the power of root. In
+particular, avoid adding new uses of the already overly-general CAP_SYS_ADMIN
+capability.
+
+If your new xyzzy(2) system call manipulates a process other than the calling
+process, it should be restricted (using a call to ptrace_may_access()) so that
+only a calling process with the same permissions as the target process, or
+with the necessary capabilities, can manipulate the target process.
+
+Finally, be aware that some non-x86 architectures have an easier time if
+system call parameters that are explicitly 64-bit fall on odd-numbered
+arguments (i.e. parameter 1, 3, 5), to allow use of contiguous pairs of 32-bit
+registers. (This concern does not apply if the arguments are part of a
+structure that's passed in by pointer.)
+
+
+Proposing the API
+-----------------
+
+To make new system calls easy to review, it's best to divide up the patchset
+into separate chunks. These should include at least the following items as
+distinct commits (each of which is described further below):
+
+ - The core implementation of the system call, together with prototypes,
+ generic numbering, Kconfig changes and fallback stub implementation.
+ - Wiring up of the new system call for one particular architecture, usually
+ x86 (including all of x86_64, x86_32 and x32).
+ - A demonstration of the use of the new system call in userspace via a
+ selftest in tools/testing/selftests/.
+ - A draft man-page for the new system call, either as plain text in the
+ cover letter, or as a patch to the (separate) man-pages repository.
+
+New system call proposals, like any change to the kernel's API, should always
+be cc'ed to linux-api@vger.kernel.org.
+
+
+Generic System Call Implementation
+----------------------------------
+
+The main entry point for your new xyzzy(2) system call will be called
+sys_xyzzy(), but you add this entry point with the appropriate
+SYSCALL_DEFINEn() macro rather than explicitly. The 'n' indicates the number
+of arguments to the system call, and the macro takes the system call name
+followed by the (type, name) pairs for the parameters as arguments. Using
+this macro allows metadata about the new system call to be made available for
+other tools.
+
+The new entry point also needs a corresponding function prototype, in
+include/linux/syscalls.h, marked as asmlinkage to match the way that system
+calls are invoked:
+
+ asmlinkage long sys_xyzzy(...);
+
+Some architectures (e.g. x86) have their own architecture-specific syscall
+tables, but several other architectures share a generic syscall table. Add your
+new system call to the generic list by adding an entry to the list in
+include/uapi/asm-generic/unistd.h:
+
+ #define __NR_xyzzy 292
+ __SYSCALL(__NR_xyzzy, sys_xyzzy)
+
+Also update the __NR_syscalls count to reflect the additional system call, and
+note that if multiple new system calls are added in the same merge window,
+your new syscall number may get adjusted to resolve conflicts.
+
+The file kernel/sys_ni.c provides a fallback stub implementation of each system
+call, returning -ENOSYS. Add your new system call here too:
+
+ cond_syscall(sys_xyzzy);
+
+Your new kernel functionality, and the system call that controls it, should
+normally be optional, so add a CONFIG option (typically to init/Kconfig) for
+it. As usual for new CONFIG options:
+
+ - Include a description of the new functionality and system call controlled
+ by the option.
+ - Make the option depend on EXPERT if it should be hidden from normal users.
+ - Make any new source files implementing the function dependent on the CONFIG
+ option in the Makefile (e.g. "obj-$(CONFIG_XYZZY_SYSCALL) += xyzzy.c").
+ - Double check that the kernel still builds with the new CONFIG option turned
+ off.
+
+To summarize, you need a commit that includes:
+
+ - CONFIG option for the new function, normally in init/Kconfig
+ - SYSCALL_DEFINEn(xyzzy, ...) for the entry point
+ - corresponding prototype in include/linux/syscalls.h
+ - generic table entry in include/uapi/asm-generic/unistd.h
+ - fallback stub in kernel/sys_ni.c
+
+
+x86 System Call Implementation
+------------------------------
+
+To wire up your new system call for x86 platforms, you need to update the
+master syscall tables. Assuming your new system call isn't special in some
+way (see below), this involves a "common" entry (for x86_64 and x32) in
+arch/x86/entry/syscalls/syscall_64.tbl:
+
+ 333 common xyzzy sys_xyzzy
+
+and an "i386" entry in arch/x86/entry/syscalls/syscall_32.tbl:
+
+ 380 i386 xyzzy sys_xyzzy
+
+Again, these numbers are liable to be changed if there are conflicts in the
+relevant merge window.
+
+
+Compatibility System Calls (Generic)
+------------------------------------
+
+For most system calls the same 64-bit implementation can be invoked even when
+the userspace program is itself 32-bit; even if the system call's parameters
+include an explicit pointer, this is handled transparently.
+
+However, there are a couple of situations where a compatibility layer is
+needed to cope with size differences between 32-bit and 64-bit.
+
+The first is if the 64-bit kernel also supports 32-bit userspace programs, and
+so needs to parse areas of (__user) memory that could hold either 32-bit or
+64-bit values. In particular, this is needed whenever a system call argument
+is:
+
+ - a pointer to a pointer
+ - a pointer to a struct containing a pointer (e.g. struct iovec __user *)
+ - a pointer to a varying sized integral type (time_t, off_t, long, ...)
+ - a pointer to a struct containing a varying sized integral type.
+
+The second situation that requires a compatibility layer is if one of the
+system call's arguments has a type that is explicitly 64-bit even on a 32-bit
+architecture, for example loff_t or __u64. In this case, a value that arrives
+at a 64-bit kernel from a 32-bit application will be split into two 32-bit
+values, which then need to be re-assembled in the compatibility layer.
+
+(Note that a system call argument that's a pointer to an explicit 64-bit type
+does *not* need a compatibility layer; for example, splice(2)'s arguments of
+type loff_t __user * do not trigger the need for a compat_ system call.)
+
+The compatibility version of the system call is called compat_sys_xyzzy(), and
+is added with the COMPAT_SYSCALL_DEFINEn() macro, analogously to
+SYSCALL_DEFINEn. This version of the implementation runs as part of a 64-bit
+kernel, but expects to receive 32-bit parameter values and does whatever is
+needed to deal with them. (Typically, the compat_sys_ version converts the
+values to 64-bit versions and either calls on to the sys_ version, or both of
+them call a common inner implementation function.)
+
+The compat entry point also needs a corresponding function prototype, in
+include/linux/compat.h, marked as asmlinkage to match the way that system
+calls are invoked:
+
+ asmlinkage long compat_sys_xyzzy(...);
+
+If the system call involves a structure that is laid out differently on 32-bit
+and 64-bit systems, say struct xyzzy_args, then the include/linux/compat.h
+header file should also include a compat version of the structure (struct
+compat_xyzzy_args) where each variable-size field has the appropriate compat_
+type that corresponds to the type in struct xyzzy_args. The
+compat_sys_xyzzy() routine can then use this compat_ structure to parse the
+arguments from a 32-bit invocation.
+
+For example, if there are fields:
+
+ struct xyzzy_args {
+ const char __user *ptr;
+ __kernel_long_t varying_val;
+ u64 fixed_val;
+ /* ... */
+ };
+
+in struct xyzzy_args, then struct compat_xyzzy_args would have:
+
+ struct compat_xyzzy_args {
+ compat_uptr_t ptr;
+ compat_long_t varying_val;
+ u64 fixed_val;
+ /* ... */
+ };
+
+The generic system call list also needs adjusting to allow for the compat
+version; the entry in include/uapi/asm-generic/unistd.h should use
+__SC_COMP rather than __SYSCALL:
+
+ #define __NR_xyzzy 292
+ __SC_COMP(__NR_xyzzy, sys_xyzzy, compat_sys_xyzzy)
+
+To summarize, you need:
+
+ - a COMPAT_SYSCALL_DEFINEn(xyzzy, ...) for the compat entry point
+ - corresponding prototype in include/linux/compat.h
+ - (if needed) 32-bit mapping struct in include/linux/compat.h
+ - instance of __SC_COMP not __SYSCALL in include/uapi/asm-generic/unistd.h
+
+
+Compatibility System Calls (x86)
+--------------------------------
+
+To wire up the x86 architecture of a system call with a compatibility version,
+the entries in the syscall tables need to be adjusted.
+
+First, the entry in arch/x86/entry/syscalls/syscall_32.tbl gets an extra
+column to indicate that a 32-bit userspace program running on a 64-bit kernel
+should hit the compat entry point:
+
+ 380 i386 xyzzy sys_xyzzy compat_sys_xyzzy
+
+Second, you need to figure out what should happen for the x32 ABI version of
+the new system call. There's a choice here: the layout of the arguments
+should either match the 64-bit version or the 32-bit version.
+
+If there's a pointer-to-a-pointer involved, the decision is easy: x32 is
+ILP32, so the layout should match the 32-bit version, and the entry in
+arch/x86/entry/syscalls/syscall_64.tbl is split so that x32 programs hit the
+compatibility wrapper:
+
+ 333 64 xyzzy sys_xyzzy
+ ...
+ 555 x32 xyzzy compat_sys_xyzzy
+
+If no pointers are involved, then it is preferable to re-use the 64-bit system
+call for the x32 ABI (and consequently the entry in
+arch/x86/entry/syscalls/syscall_64.tbl is unchanged).
+
+In either case, you should check that the types involved in your argument
+layout do indeed map exactly from x32 (-mx32) to either the 32-bit (-m32) or
+64-bit (-m64) equivalents.
+
+
+System Calls Returning Elsewhere
+--------------------------------
+
+For most system calls, once the system call is complete the user program
+continues exactly where it left off -- at the next instruction, with the
+stack the same and most of the registers the same as before the system call,
+and with the same virtual memory space.
+
+However, a few system calls do things differently. They might return to a
+different location (rt_sigreturn) or change the memory space (fork/vfork/clone)
+or even architecture (execve/execveat) of the program.
+
+To allow for this, the kernel implementation of the system call may need to
+save and restore additional registers to the kernel stack, allowing complete
+control of where and how execution continues after the system call.
+
+This is arch-specific, but typically involves defining assembly entry points
+that save/restore additional registers and invoke the real system call entry
+point.
+
+For x86_64, this is implemented as a stub_xyzzy entry point in
+arch/x86/entry/entry_64.S, and the entry in the syscall table
+(arch/x86/entry/syscalls/syscall_64.tbl) is adjusted to match:
+
+ 333 common xyzzy stub_xyzzy
+
+The equivalent for 32-bit programs running on a 64-bit kernel is normally
+called stub32_xyzzy and implemented in arch/x86/entry/entry_64_compat.S,
+with the corresponding syscall table adjustment in
+arch/x86/entry/syscalls/syscall_32.tbl:
+
+ 380 i386 xyzzy sys_xyzzy stub32_xyzzy
+
+If the system call needs a compatibility layer (as in the previous section)
+then the stub32_ version needs to call on to the compat_sys_ version of the
+system call rather than the native 64-bit version. Also, if the x32 ABI
+implementation is not common with the x86_64 version, then its syscall
+table will also need to invoke a stub that calls on to the compat_sys_
+version.
+
+For completeness, it's also nice to set up a mapping so that user-mode Linux
+still works -- its syscall table will reference stub_xyzzy, but the UML build
+doesn't include arch/x86/entry/entry_64.S implementation (because UML
+simulates registers etc). Fixing this is as simple as adding a #define to
+arch/x86/um/sys_call_table_64.c:
+
+ #define stub_xyzzy sys_xyzzy
+
+
+Other Details
+-------------
+
+Most of the kernel treats system calls in a generic way, but there is the
+occasional exception that may need updating for your particular system call.
+
+The audit subsystem is one such special case; it includes (arch-specific)
+functions that classify some special types of system call -- specifically
+file open (open/openat), program execution (execve/exeveat) or socket
+multiplexor (socketcall) operations. If your new system call is analogous to
+one of these, then the audit system should be updated.
+
+More generally, if there is an existing system call that is analogous to your
+new system call, it's worth doing a kernel-wide grep for the existing system
+call to check there are no other special cases.
+
+
+Testing
+-------
+
+A new system call should obviously be tested; it is also useful to provide
+reviewers with a demonstration of how user space programs will use the system
+call. A good way to combine these aims is to include a simple self-test
+program in a new directory under tools/testing/selftests/.
+
+For a new system call, there will obviously be no libc wrapper function and so
+the test will need to invoke it using syscall(); also, if the system call
+involves a new userspace-visible structure, the corresponding header will need
+to be installed to compile the test.
+
+Make sure the selftest runs successfully on all supported architectures. For
+example, check that it works when compiled as an x86_64 (-m64), x86_32 (-m32)
+and x32 (-mx32) ABI program.
+
+For more extensive and thorough testing of new functionality, you should also
+consider adding tests to the Linux Test Project, or to the xfstests project
+for filesystem-related changes.
+ - https://linux-test-project.github.io/
+ - git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
+
+
+Man Page
+--------
+
+All new system calls should come with a complete man page, ideally using groff
+markup, but plain text will do. If groff is used, it's helpful to include a
+pre-rendered ASCII version of the man page in the cover email for the
+patchset, for the convenience of reviewers.
+
+The man page should be cc'ed to linux-man@vger.kernel.org
+For more details, see https://www.kernel.org/doc/man-pages/patches.html
+
+References and Sources
+----------------------
+
+ - LWN article from Michael Kerrisk on use of flags argument in system calls:
+ https://lwn.net/Articles/585415/
+ - LWN article from Michael Kerrisk on how to handle unknown flags in a system
+ call: https://lwn.net/Articles/588444/
+ - LWN article from Jake Edge describing constraints on 64-bit system call
+ arguments: https://lwn.net/Articles/311630/
+ - Pair of LWN articles from David Drysdale that describe the system call
+ implementation paths in detail for v3.14:
+ - https://lwn.net/Articles/604287/
+ - https://lwn.net/Articles/604515/
+ - Architecture-specific requirements for system calls are discussed in the
+ syscall(2) man-page:
+ http://man7.org/linux/man-pages/man2/syscall.2.html#NOTES
+ - Collated emails from Linus Torvalds discussing the problems with ioctl():
+ http://yarchive.net/comp/linux/ioctl.html
+ - "How to not invent kernel interfaces", Arnd Bergmann,
+ http://www.ukuug.org/events/linux2007/2007/papers/Bergmann.pdf
+ - LWN article from Michael Kerrisk on avoiding new uses of CAP_SYS_ADMIN:
+ https://lwn.net/Articles/486306/
+ - Recommendation from Andrew Morton that all related information for a new
+ system call should come in the same email thread:
+ https://lkml.org/lkml/2014/7/24/641
+ - Recommendation from Michael Kerrisk that a new system call should come with
+ a man page: https://lkml.org/lkml/2014/6/13/309
+ - Suggestion from Thomas Gleixner that x86 wire-up should be in a separate
+ commit: https://lkml.org/lkml/2014/11/19/254
+ - Suggestion from Greg Kroah-Hartman that it's good for new system calls to
+ come with a man-page & selftest: https://lkml.org/lkml/2014/3/19/710
+ - Discussion from Michael Kerrisk of new system call vs. prctl(2) extension:
+ https://lkml.org/lkml/2014/6/3/411
+ - Suggestion from Ingo Molnar that system calls that involve multiple
+ arguments should encapsulate those arguments in a struct, which includes a
+ size field for future extensibility: https://lkml.org/lkml/2015/7/30/117
+ - Numbering oddities arising from (re-)use of O_* numbering space flags:
+ - commit 75069f2b5bfb ("vfs: renumber FMODE_NONOTIFY and add to uniqueness
+ check")
+ - commit 12ed2e36c98a ("fanotify: FMODE_NONOTIFY and __O_SYNC in sparc
+ conflict")
+ - commit bb458c644a59 ("Safer ABI for O_TMPFILE")
+ - Discussion from Matthew Wilcox about restrictions on 64-bit arguments:
+ https://lkml.org/lkml/2008/12/12/187
+ - Recommendation from Greg Kroah-Hartman that unknown flags should be
+ policed: https://lkml.org/lkml/2014/7/17/577
+ - Recommendation from Linus Torvalds that x32 system calls should prefer
+ compatibility with 64-bit versions rather than 32-bit versions:
+ https://lkml.org/lkml/2011/8/31/244
--
1.9.1
^ permalink raw reply related
* [PATCHv4 0/1] Document how to add a new syscall
From: David Drysdale @ 2015-08-10 8:00 UTC (permalink / raw)
To: linux-api, Michael Kerrisk, Andrew Morton, Arnd Bergmann,
Shuah Khan, Jonathan Corbet, Eric B Munson, Randy Dunlap,
Cyril Hrubis, Josh Triplett, Pavel Machek
Cc: Andrea Arcangeli, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Peter Zijlstra,
Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Milosz Tanski, Fam Zheng,
Mathieu Desnoyers, linux-doc, linux-kernel, David Drysdale
Given that I've gotten some of the details wrong in the past (and I've
seen others do likewise), I thought it might be helpful to collate the
best practices for adding a new system call to the kernel.
Apologies for the wide circulation -- I've tried to include folk who've
recently added or proposed a system call, as they're most likely to
have opinions on:
- whether this a useful addition to Documentation/
- whether the details of the advice are correct and complete.
Thanks,
David
(With thanks to Andrew Morton for looking over an initial draft, and to
Michael Kerrisk for suggesting several clarifications and additions.)
Changes since v3:
- remove suggestion of adding new capability bit [Josh Triplett]
- clarifications & typos [Pavel Machek]
Changes since v2:
- add struct-with-size as option for extensibility [Ingo Molnar]
- mention LTP and xfstests [Cyril Hrubis]
- always use file descriptors as handles for kernel objects [Josh Triplett]
- consider poll(2) semantics for new file descriptors [Josh Triplett]
- use a CONFIG option to control new functionality [Josh Triplett]
- check ptrace_may_access() for process manipulation [Josh Triplett]
- various minor markups [Josh Triplett]
Changes since v1:
- added paragraph on build requirements to Testing section [Shuah Khan,
Peter Zijlstra]
- various text clarifications [Kees Cook]
- added Reviewed-by markers
David Drysdale (1):
Documentation: describe how to add a system call
Documentation/adding-syscalls.txt | 527 ++++++++++++++++++++++++++++++++++++++
1 file changed, 527 insertions(+)
create mode 100644 Documentation/adding-syscalls.txt
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply
* Re: [PATCHv3 1/1] Documentation: describe how to add a system call
From: David Drysdale @ 2015-08-10 7:43 UTC (permalink / raw)
To: Pavel Machek
Cc: Linux API, Michael Kerrisk, Andrew Morton, Arnd Bergmann,
Shuah Khan, Jonathan Corbet, Eric B Munson, Randy Dunlap,
Cyril Hrubis, Josh Triplett, Andrea Arcangeli, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, Oleg Nesterov, Linus Torvalds,
Greg Kroah-Hartman, Andy Lutomirski, Al Viro, Rusty Russell,
Peter Zijlstra, Vivek Goyal, Alexei Starovoitov, Davi
In-Reply-To: <20150805162131.GB16341@amd>
On Wed, Aug 5, 2015 at 5:21 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> Add a document describing the process of adding a new system call,
>> including the need for a flags argument for future compatibility, and
>> covering 32-bit/64-bit concerns (albeit in an x86-centric way).
>>
>> Signed-off-by: David Drysdale <drysdale@google.com>
>> Reviewed-by: Michael Kerrisk <mtk.manpages@gmail.com>
>> Reviewed-by: Eric B Munson <emunson@akamai.com>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>> Documentation/adding-syscalls.txt | 531 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 531 insertions(+)
>> create mode 100644 Documentation/adding-syscalls.txt
>
> We usually align documentation to less than 80 columns, afaict.
Ah, a few re-worded paragraphs crept over the limit. Reflowed.
>> +call. To make sure that userspace programs can safely use flags between kernel
>> +versions, check whether the flags value holds any unknown flags, and reject the
>> +sycall (with EINVAL) if it does:
>
> syscall?
Fixed, thanks.
>> +New system call proposals, like any change to the kernel's API, should always
>> +be cc'ed to linux-api@vger.kernel.org
>
> . at and of sentence?
Fixed.
>> +System Calls Returning Elsewhere
>> +--------------------------------
>> +
>> +For most system calls, once the system call is complete the user program
>> +continues exactly where it left off -- at the next instruction, with the same
>> +stack and registers as before the system call, and with the same virtual
>> +memory space.
>
> Umm. Normally we place return value in register. And I'm not sure.. do
> syscalls preserve registers that are normally caller-clobbered in the ABI?
Reworded to:
For most system calls, once the system call is complete the user program
continues exactly where it left off -- at the next instruction, with the
stack the same and most of the registers the same as before the system call,
and with the same virtual memory space.
> Thanks,
> Pavel
Thanks for the updates,
David
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* Re: [PATCH 2/4] mm/compaction: enable mobile-page migration
From: Minchan Kim @ 2015-08-10 7:19 UTC (permalink / raw)
To: Gioh Kim
Cc: jlayton-vpEMnDpepFuMZCB2o+C8xQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
vbabka-AlSwsSmVLrQ, iamjoonsoo.kim-Hm3cg6mZ9cc,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, mst-H+wXaHxf7aLQT0dZR+AlfA,
koct9i-Re5JQEeQqe8AvxtiuMwx3w, aquini-H+wXaHxf7aLQT0dZR+AlfA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Gioh Kim
In-Reply-To: <20150731104309.GA22158@bbox>
On Fri, Jul 31, 2015 at 07:43:09PM +0900, Minchan Kim wrote:
> Hi Gioh,
>
> On Mon, Jul 13, 2015 at 05:35:17PM +0900, Gioh Kim wrote:
> > From: Gioh Kim <gurugio-A7HHaXTFBl7R7s880joybQ@public.gmane.org>
> >
> > Add framework to register callback functions and check page mobility.
> > There are some modes for page isolation so that isolate interface
> > has arguments of page address and isolation mode while putback
> > interface has only page address as argument.
> >
> > Signed-off-by: Gioh Kim <gioh.kim-Hm3cg6mZ9cc@public.gmane.org>
> > Acked-by: Rafael Aquini <aquini-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> > fs/proc/page.c | 3 ++
> > include/linux/compaction.h | 80 ++++++++++++++++++++++++++++++++++
> > include/linux/fs.h | 2 +
> > include/linux/page-flags.h | 19 ++++++++
> > include/uapi/linux/kernel-page-flags.h | 1 +
> > 5 files changed, 105 insertions(+)
> >
> > diff --git a/fs/proc/page.c b/fs/proc/page.c
> > index 7eee2d8..a4f5a00 100644
> > --- a/fs/proc/page.c
> > +++ b/fs/proc/page.c
> > @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page)
> > if (PageBalloon(page))
> > u |= 1 << KPF_BALLOON;
> >
> > + if (PageMobile(page))
> > + u |= 1 << KPF_MOBILE;
> > +
>
> Need a new page flag for non-LRU page migration?
> I am worry that we don't have empty slot for page flag on 32-bit.
>
> Aha, see SetPageMobile below. You use _mapcount.
> It seems to be work for non-LRU pages but unfortunately, zsmalloc
> already have used that field as own purpose so there is no room
> for it.
Gioh, I just sent a patch which zsmalloc doesn't use page->mapping
and _mapcount so I think zsmalloc could support compaction with your
patchset. Although zsmalloc can support compaction with it, I still
don't think it's a good that driver have to mark pages mobile via
_mapcount.
I hope you can find another way.
Thanks.
>
>
> > u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked);
> >
> > u |= kpf_copy_bit(k, KPF_SLAB, PG_slab);
> > diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> > index aa8f61c..f693072 100644
> > --- a/include/linux/compaction.h
> > +++ b/include/linux/compaction.h
> > @@ -1,6 +1,9 @@
> > #ifndef _LINUX_COMPACTION_H
> > #define _LINUX_COMPACTION_H
> >
> > +#include <linux/page-flags.h>
> > +#include <linux/pagemap.h>
> > +
> > /* Return values for compact_zone() and try_to_compact_pages() */
> > /* compaction didn't start as it was deferred due to past failures */
> > #define COMPACT_DEFERRED 0
> > @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone, int order,
> > bool alloc_success);
> > extern bool compaction_restarting(struct zone *zone, int order);
> >
> > +static inline bool mobile_page(struct page *page)
> > +{
> > + return page->mapping && (PageMobile(page) || PageBalloon(page));
>
>
> What's the locking rule to test mobile_page?
> Why we need such many checking?
>
> Why we need PageBalloon check?
> You are trying to make generic abstraction for non-LRU page to migrate
> but PageBalloon check in here breaks your goal.
>
> > +}
> > +
> > +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t mode)
> > +{
> > + bool ret = false;
> > +
> > + /*
> > + * Avoid burning cycles with pages that are yet under __free_pages(),
> > + * or just got freed under us.
> > + *
> > + * In case we 'win' a race for a mobile page being freed under us and
> > + * raise its refcount preventing __free_pages() from doing its job
> > + * the put_page() at the end of this block will take care of
> > + * release this page, thus avoiding a nasty leakage.
> > + */
>
> Good.
>
> > + if (unlikely(!get_page_unless_zero(page)))
> > + goto out;
> > +
> > + /*
> > + * As mobile pages are not isolated from LRU lists, concurrent
> > + * compaction threads can race against page migration functions
> > + * as well as race against the releasing a page.
>
> How can it race with releasing a page?
> We already get refcount above.
>
> Do you mean page->mapping tearing race?
> If so, zsmalloc should be chaned to hold a lock.
>
>
> > + *
> > + * In order to avoid having an already isolated mobile page
> > + * being (wrongly) re-isolated while it is under migration,
> > + * or to avoid attempting to isolate pages being released,
> > + * lets be sure we have the page lock
> > + * before proceeding with the mobile page isolation steps.
> > + */
> > + if (unlikely(!trylock_page(page)))
> > + goto out_putpage;
> > +
> > + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage))
> > + goto out_not_isolated;
>
> I cannot know how isolate_mobilepage is called by upper layer
> because this patch doesn't include it.
> Anyway, why we need such many checks to prove it's mobile page?
>
> mobile_page() {
> page->mapping && (PageMobile(page) || PageBalloon(page));
> }
>
> Additionally, added page->mapping->a_ops->isolatepage check
>
> Is it possible that a non-LRU page's address space provides
> only page->maping->migratepage without isolate/putback?
>
> Couldn't we make it simple like this?
>
> page->mapping && page->mapping->migratepage
>
> So, couldn't we make mobile_page like this
>
> PageMobile(struct page *page)
> {
> VM_BUG_ON_PAGE(!PageLocked(page), page);
> VM_BUG_ON_PAGE(PageLRU(page), page);
>
> return page->mapping && page->mapping->migratepage;
> }
>
> It will save _mapcount of struct page.
>
> > + ret = page->mapping->a_ops->isolatepage(page, mode);
> > + if (!ret)
> > + goto out_not_isolated;
> > + unlock_page(page);
> > + return ret;
> > +
> > +out_not_isolated:
> > + unlock_page(page);
> > +out_putpage:
> > + put_page(page);
> > +out:
> > + return ret;
> > +}
> > +
> > +static inline void putback_mobilepage(struct page *page)
> > +{
> > + /*
> > + * 'lock_page()' stabilizes the page and prevents races against
>
> What does it mean 'stabilize'?
> Clear comment is always welcome rather than vague word.
>
> > + * concurrent isolation threads attempting to re-isolate it.
> > + */
> > + lock_page(page);
> > + if (page->mapping && page->mapping->a_ops->putbackpage)
> > + page->mapping->a_ops->putbackpage(page);
> > + unlock_page(page);
> > + /* drop the extra ref count taken for mobile page isolation */
> > + put_page(page);
> > +}
> > #else
> > static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
> > unsigned int order, int alloc_flags,
> > @@ -83,6 +150,19 @@ static inline bool compaction_deferred(struct zone *zone, int order)
> > return true;
> > }
> >
> > +static inline bool mobile_page(struct page *page)
> > +{
> > + return false;
> > +}
> > +
> > +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t mode)
> > +{
> > + return false;
> > +}
> > +
> > +static inline void putback_mobilepage(struct page *page)
> > +{
> > +}
> > #endif /* CONFIG_COMPACTION */
> >
> > #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index a0653e5..2cc4b24 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -396,6 +396,8 @@ struct address_space_operations {
> > */
> > int (*migratepage) (struct address_space *,
> > struct page *, struct page *, enum migrate_mode);
> > + bool (*isolatepage) (struct page *, isolate_mode_t);
> > + void (*putbackpage) (struct page *);
> > int (*launder_page) (struct page *);
> > int (*is_partially_uptodate) (struct page *, unsigned long,
> > unsigned long);
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index f34e040..abef145 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -582,6 +582,25 @@ static inline void __ClearPageBalloon(struct page *page)
> > atomic_set(&page->_mapcount, -1);
> > }
> >
> > +#define PAGE_MOBILE_MAPCOUNT_VALUE (-255)
> > +
> > +static inline int PageMobile(struct page *page)
> > +{
> > + return atomic_read(&page->_mapcount) == PAGE_MOBILE_MAPCOUNT_VALUE;
> > +}
> > +
> > +static inline void __SetPageMobile(struct page *page)
> > +{
> > + VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
> > + atomic_set(&page->_mapcount, PAGE_MOBILE_MAPCOUNT_VALUE);
> > +}
> > +
> > +static inline void __ClearPageMobile(struct page *page)
> > +{
> > + VM_BUG_ON_PAGE(!PageMobile(page), page);
> > + atomic_set(&page->_mapcount, -1);
> > +}
> > +
> > /*
> > * If network-based swap is enabled, sl*b must keep track of whether pages
> > * were allocated from pfmemalloc reserves.
> > diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
> > index a6c4962..d50d9e8 100644
> > --- a/include/uapi/linux/kernel-page-flags.h
> > +++ b/include/uapi/linux/kernel-page-flags.h
> > @@ -33,6 +33,7 @@
> > #define KPF_THP 22
> > #define KPF_BALLOON 23
> > #define KPF_ZERO_PAGE 24
> > +#define KPF_MOBILE 25
> >
> >
> > #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
> > --
> > 2.1.4
> >
^ permalink raw reply
* Re: [PATCH] eventfd: implementation of EFD_MASK flag
From: Martin Sustrik @ 2015-08-10 6:39 UTC (permalink / raw)
To: Damian Hobson-Garcia; +Cc: viro, linux-fsdevel, linux-kernel, netdev, linux-api
In-Reply-To: <55C8436C.1000806@igel.co.jp>
On 2015-08-10 08:23, Damian Hobson-Garcia wrote:
> Replying to my own post, but I had the following comments/questions.
> Martin, if you have any response to my comments I would be very happy
> to
> hear them.
>
> On 2015-08-10 2:51 PM, Damian Hobson-Garcia wrote:
>> From: Martin Sustrik <sustrik@250bpm.com>
>>
> [snip]
>>
>> write(2):
>>
>> User is allowed to write only buffers containing the following
>> structure:
>>
>> struct efd_mask {
>> __u32 events;
>> __u64 data;
>> };
>>
>> The value of 'events' should be any combination of event flags as
>> defined by
>> poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified
>> events will
>> be signaled when polling (select, poll, epoll) on the eventfd is done
>> later on.
>> 'data' is opaque data that are not interpreted by eventfd object.
>>
> I'm not fully clear on the purpose that the 'data' member serves. Does
> this opaque handle need to be tied together with this event
> synchronization construct?
It's a convenience thing. Imagine you are implementing your own file
descriptor type in user space. You create an EFD_MASK socket and a
structure that will hold any state that you need for the socket (tx/rx
buffers and such).
Now you have two things to pass around. If you want to pass the fd to a
function, it must have two parameters (fd and pointer to the structure).
To fix it you can put the fd into the structure. That way there's only
one thing to pass around (the structure).
The problem with that approach is when you have generic code that deals
with file descriptors. For example, a simple poller which accepts a list
of (fd, callback) pairs and invokes the callback when one of the fds
signals POLLIN. You can't send a pointer to a structure to such
function. All you can send is the fd, but then, when the callback is
invoked, fd is all you have. You have no idea where your state is.
'data' member allows you to put the pointer to the state to the socket
itself. Thus, if you have a fd, you can always find out where the
associated data is by reading the mask structure from the fd.
>
> [snip]
>
>> @@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64
>> n)
>> {
>> + /* This function should never be used with eventfd in the mask mode.
>> */
>> + BUG_ON(ctx->flags & EFD_MASK);
>> +
> ...
>> @@ -158,6 +180,9 @@ int eventfd_ctx_remove_wait_queue(struct
>> eventfd_ctx *ctx, wait_queue_t *wait,
>> {
>> + /* This function should never be used with eventfd in the mask mode.
>> */
>> + BUG_ON(ctx->flags & EFD_MASK);
>> +
> ...
>> @@ -188,6 +213,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx,
>> int no_wait, __u64 *cnt)
>> + /* This function should never be used with eventfd in the mask mode.
>> */
>> + BUG_ON(ctx->flags & EFD_MASK);
>> +
>
> If eventfd_ctx_fileget() returns EINVAL when EFD_MASK is set, I don't
> think that there will be a way to call these functions in the mask
> mode,
> so it should be possible to get rid of the BUG_ON checks.
Sure. Feel free to do so.
>
> [snip]
>> @@ -230,6 +258,19 @@ static ssize_t eventfd_read(struct file *file,
>> char __user *buf, size_t count,
>> ssize_t res;
>> __u64 cnt;
>>
>> + if (ctx->flags & EFD_MASK) {
>> + struct efd_mask mask;
>> +
>> + if (count < sizeof(mask))
>> + return -EINVAL;
>> + spin_lock_irq(&ctx->wqh.lock);
>> + mask = ctx->mask;
>> + spin_unlock_irq(&ctx->wqh.lock);
>> + if (copy_to_user(buf, &mask, sizeof(mask)))
>> + return -EFAULT;
>> + return sizeof(mask);
>> + }
>> +
>
> For the other eventfd modes, reading the value will update the internal
> state of the eventfd (either clearing or decrementing the counter).
> Should something similar be done here? I'm thinking of a case where a
> process is polling on this fd in a loop. Clearing the efd_mask data on
> read should provide an easy way for the polling process to know if it
> is
> seeing new poll events.
No. In this case reading the value has no effect on the state of the fd.
How it should work is rather:
// fd is in POLLIN state
poll(fd);
// function exits with POLLIN but fd remains in POLLIN state
my_recv(fd, buf, size);
// my_recv function have found out that there's no more data to recv and
switched off the POLLIN flag
poll(fd); // we block here waiting for more data to arrive from the
network
>
>> @@ -292,8 +351,13 @@ static void eventfd_show_fdinfo(struct seq_file
>> *m, struct file *f)
>> struct eventfd_ctx *ctx = f->private_data;
>>
>> spin_lock_irq(&ctx->wqh.lock);
>> - seq_printf(m, "eventfd-count: %16llx\n",
>> - (unsigned long long)ctx->count);
>> + if (ctx->flags & EFD_MASK) {
>> + seq_printf(m, "eventfd-mask: %x\n",
>> + (unsigned)ctx->mask.events);
>> + } else {
>> + seq_printf(m, "eventfd-count: %16llx\n",
>> + (unsigned long long)ctx->count);
>> + }
>> spin_unlock_irq(&ctx->wqh.lock);
>> }
> I think that putting the EFD_MASK functionality into a different fops
> structure might be useful for reducing the number of if statements.
Sure. No objections.
Thanks for re-submitting the patch!
Martin
^ permalink raw reply
* Re: [PATCH] eventfd: implementation of EFD_MASK flag
From: Damian Hobson-Garcia @ 2015-08-10 6:23 UTC (permalink / raw)
To: sustrik-8XN41Hm7QBXQT0dZR+AlfA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1439185906-28180-2-git-send-email-dhobsong-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
Replying to my own post, but I had the following comments/questions.
Martin, if you have any response to my comments I would be very happy to
hear them.
On 2015-08-10 2:51 PM, Damian Hobson-Garcia wrote:
> From: Martin Sustrik <sustrik-8XN41Hm7QBXQT0dZR+AlfA@public.gmane.org>
>
[snip]
>
> write(2):
>
> User is allowed to write only buffers containing the following structure:
>
> struct efd_mask {
> __u32 events;
> __u64 data;
> };
>
> The value of 'events' should be any combination of event flags as defined by
> poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified events will
> be signaled when polling (select, poll, epoll) on the eventfd is done later on.
> 'data' is opaque data that are not interpreted by eventfd object.
>
I'm not fully clear on the purpose that the 'data' member serves. Does
this opaque handle need to be tied together with this event
synchronization construct?
[snip]
> @@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
> {
> + /* This function should never be used with eventfd in the mask mode. */
> + BUG_ON(ctx->flags & EFD_MASK);
> +
...
> @@ -158,6 +180,9 @@ int eventfd_ctx_remove_wait_queue(struct eventfd_ctx *ctx, wait_queue_t *wait,
> {
> + /* This function should never be used with eventfd in the mask mode. */
> + BUG_ON(ctx->flags & EFD_MASK);
> +
...
> @@ -188,6 +213,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx, int no_wait, __u64 *cnt)
> + /* This function should never be used with eventfd in the mask mode. */
> + BUG_ON(ctx->flags & EFD_MASK);
> +
If eventfd_ctx_fileget() returns EINVAL when EFD_MASK is set, I don't
think that there will be a way to call these functions in the mask mode,
so it should be possible to get rid of the BUG_ON checks.
[snip]
> @@ -230,6 +258,19 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
> ssize_t res;
> __u64 cnt;
>
> + if (ctx->flags & EFD_MASK) {
> + struct efd_mask mask;
> +
> + if (count < sizeof(mask))
> + return -EINVAL;
> + spin_lock_irq(&ctx->wqh.lock);
> + mask = ctx->mask;
> + spin_unlock_irq(&ctx->wqh.lock);
> + if (copy_to_user(buf, &mask, sizeof(mask)))
> + return -EFAULT;
> + return sizeof(mask);
> + }
> +
For the other eventfd modes, reading the value will update the internal
state of the eventfd (either clearing or decrementing the counter).
Should something similar be done here? I'm thinking of a case where a
process is polling on this fd in a loop. Clearing the efd_mask data on
read should provide an easy way for the polling process to know if it is
seeing new poll events.
> @@ -292,8 +351,13 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
> struct eventfd_ctx *ctx = f->private_data;
>
> spin_lock_irq(&ctx->wqh.lock);
> - seq_printf(m, "eventfd-count: %16llx\n",
> - (unsigned long long)ctx->count);
> + if (ctx->flags & EFD_MASK) {
> + seq_printf(m, "eventfd-mask: %x\n",
> + (unsigned)ctx->mask.events);
> + } else {
> + seq_printf(m, "eventfd-count: %16llx\n",
> + (unsigned long long)ctx->count);
> + }
> spin_unlock_irq(&ctx->wqh.lock);
> }
I think that putting the EFD_MASK functionality into a different fops
structure might be useful for reducing the number of if statements.
Thank you,
Damian
^ permalink raw reply
* [PATCH] eventfd: implementation of EFD_MASK flag
From: Damian Hobson-Garcia @ 2015-08-10 5:51 UTC (permalink / raw)
To: sustrik, viro; +Cc: linux-fsdevel, linux-kernel, netdev, linux-api
In-Reply-To: <1439185906-28180-1-git-send-email-dhobsong@igel.co.jp>
From: Martin Sustrik <sustrik@250bpm.com>
When implementing network protocols in user space, one has to implement
fake file descriptors to represent the sockets for the protocol.
Polling on such fake file descriptors is a problem (poll/select/epoll accept
only true file descriptors) and forces protocol implementers to use various
workarounds resulting in complex, non-standard and convoluted APIs.
More generally, ability to create full-blown file descriptors for
userspace-to-userspace signalling is missing. While eventfd(2) goes half the way
towards this goal it has follwoing shorcomings:
I. There's no way to signal POLLPRI, POLLHUP etc.
II. There's no way to signal arbitrary combination of POLL* flags. Most notably,
simultaneous !POLLIN and !POLLOUT, which is a perfectly valid combination
for a network protocol (rx buffer is empty and tx buffer is full), cannot be
signaled using eventfd.
This patch implements new EFD_MASK flag which solves the above problems.
Additionally, to provide a way to associate user-space state with eventfd
object, it allows to attach user-space data to the file descriptor.
The semantics of EFD_MASK are as follows:
eventfd(2):
If eventfd is created with EFD_MASK flag set, it is initialised in such a way
as to signal no events on the file descriptor when it is polled on. 'initval'
argument is ignored.
write(2):
User is allowed to write only buffers containing the following structure:
struct efd_mask {
__u32 events;
__u64 data;
};
The value of 'events' should be any combination of event flags as defined by
poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified events will
be signaled when polling (select, poll, epoll) on the eventfd is done later on.
'data' is opaque data that are not interpreted by eventfd object.
read(2):
User is allowed to read an efd_mask structure from the eventfd marked by
EFD_MASK. Returned value shall be the last one written to the eventfd.
select(2), poll(2) and similar:
When polling on the eventfd marked by EFD_MASK flag, all the events specified
in last written 'events' field shall be signaled.
Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
---
fs/eventfd.c | 93 ++++++++++++++++++++++++++++++++++++++------
include/linux/eventfd.h | 16 +-------
include/uapi/linux/eventfd.h | 40 +++++++++++++++++++
3 files changed, 123 insertions(+), 26 deletions(-)
create mode 100644 include/uapi/linux/eventfd.h
diff --git a/fs/eventfd.c b/fs/eventfd.c
index 8d0c0df..11fb92a 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -2,6 +2,7 @@
* fs/eventfd.c
*
* Copyright (C) 2007 Davide Libenzi <davidel@xmailserver.org>
+ * Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
*
*/
@@ -22,18 +23,31 @@
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
+#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
+#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
+
struct eventfd_ctx {
struct kref kref;
wait_queue_head_t wqh;
- /*
- * Every time that a write(2) is performed on an eventfd, the
- * value of the __u64 being written is added to "count" and a
- * wakeup is performed on "wqh". A read(2) will return the "count"
- * value to userspace, and will reset "count" to zero. The kernel
- * side eventfd_signal() also, adds to the "count" counter and
- * issue a wakeup.
- */
- __u64 count;
+ union {
+ /*
+ * Every time that a write(2) is performed on an eventfd, the
+ * value of the __u64 being written is added to "count" and a
+ * wakeup is performed on "wqh". A read(2) will return the
+ * "count" value to userspace, and will reset "count" to zero.
+ * The kernel side eventfd_signal() also, adds to the "count"
+ * counter and issue a wakeup.
+ */
+ __u64 count;
+
+ /*
+ * When using eventfd in EFD_MASK mode this stracture stores the
+ * current events to be signaled on the eventfd (events member)
+ * along with opaque user-defined data (data member).
+ */
+ struct efd_mask mask;
+ };
unsigned int flags;
};
@@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
{
unsigned long flags;
+ /* This function should never be used with eventfd in the mask mode. */
+ BUG_ON(ctx->flags & EFD_MASK);
+
spin_lock_irqsave(&ctx->wqh.lock, flags);
if (ULLONG_MAX - ctx->count < n)
n = ULLONG_MAX - ctx->count;
@@ -124,6 +141,11 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
smp_rmb();
count = ctx->count;
+ if (ctx->flags & EFD_MASK) {
+ events = ctx->mask.events;
+ return events;
+ }
+
if (count > 0)
events |= POLLIN;
if (count == ULLONG_MAX)
@@ -158,6 +180,9 @@ int eventfd_ctx_remove_wait_queue(struct eventfd_ctx *ctx, wait_queue_t *wait,
{
unsigned long flags;
+ /* This function should never be used with eventfd in the mask mode. */
+ BUG_ON(ctx->flags & EFD_MASK);
+
spin_lock_irqsave(&ctx->wqh.lock, flags);
eventfd_ctx_do_read(ctx, cnt);
__remove_wait_queue(&ctx->wqh, wait);
@@ -188,6 +213,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx, int no_wait, __u64 *cnt)
ssize_t res;
DECLARE_WAITQUEUE(wait, current);
+ /* This function should never be used with eventfd in the mask mode. */
+ BUG_ON(ctx->flags & EFD_MASK);
+
spin_lock_irq(&ctx->wqh.lock);
*cnt = 0;
res = -EAGAIN;
@@ -230,6 +258,19 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
ssize_t res;
__u64 cnt;
+ if (ctx->flags & EFD_MASK) {
+ struct efd_mask mask;
+
+ if (count < sizeof(mask))
+ return -EINVAL;
+ spin_lock_irq(&ctx->wqh.lock);
+ mask = ctx->mask;
+ spin_unlock_irq(&ctx->wqh.lock);
+ if (copy_to_user(buf, &mask, sizeof(mask)))
+ return -EFAULT;
+ return sizeof(mask);
+ }
+
if (count < sizeof(cnt))
return -EINVAL;
res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
@@ -247,6 +288,24 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
__u64 ucnt;
DECLARE_WAITQUEUE(wait, current);
+ if (ctx->flags & EFD_MASK) {
+ struct efd_mask mask;
+
+ if (count < sizeof(mask))
+ return -EINVAL;
+ if (copy_from_user(&mask, buf, sizeof(mask)))
+ return -EFAULT;
+ if (mask.events & ~EFD_MASK_VALID_EVENTS)
+ return -EINVAL;
+ spin_lock_irq(&ctx->wqh.lock);
+ memcpy(&ctx->mask, &mask, sizeof(ctx->mask));
+ if (waitqueue_active(&ctx->wqh))
+ wake_up_locked_poll(&ctx->wqh,
+ (unsigned long)ctx->mask.events);
+ spin_unlock_irq(&ctx->wqh.lock);
+ return sizeof(ctx->mask);
+ }
+
if (count < sizeof(ucnt))
return -EINVAL;
if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
@@ -292,8 +351,13 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
struct eventfd_ctx *ctx = f->private_data;
spin_lock_irq(&ctx->wqh.lock);
- seq_printf(m, "eventfd-count: %16llx\n",
- (unsigned long long)ctx->count);
+ if (ctx->flags & EFD_MASK) {
+ seq_printf(m, "eventfd-mask: %x\n",
+ (unsigned)ctx->mask.events);
+ } else {
+ seq_printf(m, "eventfd-count: %16llx\n",
+ (unsigned long long)ctx->count);
+ }
spin_unlock_irq(&ctx->wqh.lock);
}
#endif
@@ -406,7 +470,12 @@ struct file *eventfd_file_create(unsigned int count, int flags)
kref_init(&ctx->kref);
init_waitqueue_head(&ctx->wqh);
- ctx->count = count;
+ if (flags & EFD_MASK) {
+ ctx->mask.events = 0;
+ ctx->mask.data = 0;
+ } else {
+ ctx->count = count;
+ }
ctx->flags = flags;
file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
index ff0b981..87de343 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -8,23 +8,11 @@
#ifndef _LINUX_EVENTFD_H
#define _LINUX_EVENTFD_H
+#include <uapi/linux/eventfd.h>
+
#include <linux/fcntl.h>
#include <linux/wait.h>
-/*
- * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
- * new flags, since they might collide with O_* ones. We want
- * to re-use O_* flags that couldn't possibly have a meaning
- * from eventfd, in order to leave a free define-space for
- * shared O_* flags.
- */
-#define EFD_SEMAPHORE (1 << 0)
-#define EFD_CLOEXEC O_CLOEXEC
-#define EFD_NONBLOCK O_NONBLOCK
-
-#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
-#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
-
struct file;
#ifdef CONFIG_EVENTFD
diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
new file mode 100644
index 0000000..03057a5
--- /dev/null
+++ b/include/uapi/linux/eventfd.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _UAPI_LINUX_EVENTFD_H
+#define _UAPI_LINUX_EVENTFD_H
+
+/* For O_CLOEXEC */
+#include <linux/fcntl.h>
+#include <linux/types.h>
+
+/*
+ * CAREFUL: Check include/asm-generic/fcntl.h when defining
+ * new flags, since they might collide with O_* ones. We want
+ * to re-use O_* flags that couldn't possibly have a meaning
+ * from eventfd, in order to leave a free define-space for
+ * shared O_* flags.
+ */
+
+/* Provide semaphore-like semantics for reads from the eventfd. */
+#define EFD_SEMAPHORE (1 << 0)
+/* Provide event mask semantics for the eventfd. */
+#define EFD_MASK (1 << 1)
+/* Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
+#define EFD_CLOEXEC O_CLOEXEC
+/* Create the eventfd in non-blocking mode. */
+#define EFD_NONBLOCK O_NONBLOCK
+
+/* Structure to read/write to eventfd in EFD_MASK mode. */
+struct efd_mask {
+ __u32 events;
+ __u64 data;
+} __packed;
+
+#endif /* _UAPI_LINUX_EVENTFD_H */
--
1.9.1
^ permalink raw reply related
* [RFC/PATCH] Generalize poll events from eventfd
From: Damian Hobson-Garcia @ 2015-08-10 5:51 UTC (permalink / raw)
To: sustrik, viro
Cc: linux-fsdevel, linux-kernel, netdev, linux-api,
Damian Hobson-Garcia
Hello all,
eventfd is very useful for generating POLLIN/POLLOUT poll events from user
space but some applications may want to generate POLLPRI/POLLERR events as well.
This patch submission aims to generalize the events generated by an
eventfd. This is a resubmission of a patch from Feb 2013[1]. The original
discussion trailed off without any conclusion, but the original author
has recently confirmed[2] that this functionality is still useful, so I
volunteered to rebase and resubmit the patch for discussion.
I have rebased this patch onto v4.2-rc5 and modified the original patch to
not add an indentation level to the implementations of eventfd_{read,write,
poll}. I hope these changes will make it easier to review. The description of
the efd_mask structure was also updated.
I have a few comments and questions, but I will add those separately as a
reply for open discussion.
[1] https://lkml.org/lkml/2013/2/18/147
[2] https://lkml.org/lkml/2015/7/9/153
Thank you,
Damian
Martin Sustrik (1):
eventfd: implementation of EFD_MASK flag
fs/eventfd.c | 91 ++++++++++++++++++++++++++++++++++++++------
include/linux/eventfd.h | 16 +-------
include/uapi/linux/eventfd.h | 40 +++++++++++++++++++
3 files changed, 121 insertions(+), 26 deletions(-)
create mode 100644 include/uapi/linux/eventfd.h
--
1.9.1
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Andrew Lunn @ 2015-08-09 15:19 UTC (permalink / raw)
Cc: Srinivas Kandagatla, stefan.wahren-eS4NqCHxEME,
devicetree-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
khilman-DgEjT+Ai2ygdnm+yROfE0A, Greg Kroah-Hartman,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
mporter-OWPKS81ov/FWk0Htik3J/w,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Rob Herring, Mark Brown,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wxt-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <1438693367.3793.29.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Srinivas
The AT24 eeprom driver contains the comment:
/*
* Export the EEPROM bytes through sysfs, since that's convenient.
* By default, only root should see the data (maybe passwords etc)
*/
and as you would expect from this:
# ls -l ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/eeprom
-rw------- 1 root root 512 Aug 9 10:16 ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/eeprom
The AT25 and the MAX6875 driver are the same.
However nvmem has different defaults:
# ls -l ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/0-00500/nvmem
-rw-r--r-- 1 root root 0 Aug 9 10:16 ./devices/platform/soc/soc:aips-bus@40000000/40066000.i2c/i2c-0/0-0050/0-00500/nvmem
Has this been considered and discussed?
Thanks
Andrew
^ permalink raw reply
* [PATCH v7 6/6] mips: Add entry for new mlock2 syscall
From: Eric B Munson @ 2015-08-09 5:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Ralf Baechle, linux-mips, linux-api, linux-arch,
linux-mm, linux-kernel
In-Reply-To: <1439097776-27695-1-git-send-email-emunson@akamai.com>
A previous commit introduced the new mlock2 syscall, add entries for the
MIPS architecture.
Signed-off-by: Eric B Munson <emunson@akamai.com>
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
---
arch/mips/include/uapi/asm/unistd.h | 15 +++++++++------
arch/mips/kernel/scall32-o32.S | 1 +
arch/mips/kernel/scall64-64.S | 1 +
arch/mips/kernel/scall64-n32.S | 1 +
arch/mips/kernel/scall64-o32.S | 1 +
5 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/arch/mips/include/uapi/asm/unistd.h b/arch/mips/include/uapi/asm/unistd.h
index c03088f..d0bdfaa 100644
--- a/arch/mips/include/uapi/asm/unistd.h
+++ b/arch/mips/include/uapi/asm/unistd.h
@@ -377,16 +377,17 @@
#define __NR_memfd_create (__NR_Linux + 354)
#define __NR_bpf (__NR_Linux + 355)
#define __NR_execveat (__NR_Linux + 356)
+#define __NR_mlock2 (__NR_Linux + 357)
/*
* Offset of the last Linux o32 flavoured syscall
*/
-#define __NR_Linux_syscalls 356
+#define __NR_Linux_syscalls 357
#endif /* _MIPS_SIM == _MIPS_SIM_ABI32 */
#define __NR_O32_Linux 4000
-#define __NR_O32_Linux_syscalls 356
+#define __NR_O32_Linux_syscalls 357
#if _MIPS_SIM == _MIPS_SIM_ABI64
@@ -711,16 +712,17 @@
#define __NR_memfd_create (__NR_Linux + 314)
#define __NR_bpf (__NR_Linux + 315)
#define __NR_execveat (__NR_Linux + 316)
+#define __NR_mlock2 (__NR_Linux + 317)
/*
* Offset of the last Linux 64-bit flavoured syscall
*/
-#define __NR_Linux_syscalls 316
+#define __NR_Linux_syscalls 317
#endif /* _MIPS_SIM == _MIPS_SIM_ABI64 */
#define __NR_64_Linux 5000
-#define __NR_64_Linux_syscalls 316
+#define __NR_64_Linux_syscalls 317
#if _MIPS_SIM == _MIPS_SIM_NABI32
@@ -1049,15 +1051,16 @@
#define __NR_memfd_create (__NR_Linux + 318)
#define __NR_bpf (__NR_Linux + 319)
#define __NR_execveat (__NR_Linux + 320)
+#define __NR_mlock2 (__NR_Linux + 321)
/*
* Offset of the last N32 flavoured syscall
*/
-#define __NR_Linux_syscalls 320
+#define __NR_Linux_syscalls 321
#endif /* _MIPS_SIM == _MIPS_SIM_NABI32 */
#define __NR_N32_Linux 6000
-#define __NR_N32_Linux_syscalls 320
+#define __NR_N32_Linux_syscalls 321
#endif /* _UAPI_ASM_UNISTD_H */
diff --git a/arch/mips/kernel/scall32-o32.S b/arch/mips/kernel/scall32-o32.S
index 4cc1350..b0b377a 100644
--- a/arch/mips/kernel/scall32-o32.S
+++ b/arch/mips/kernel/scall32-o32.S
@@ -599,3 +599,4 @@ EXPORT(sys_call_table)
PTR sys_memfd_create
PTR sys_bpf /* 4355 */
PTR sys_execveat
+ PTR sys_mlock2
diff --git a/arch/mips/kernel/scall64-64.S b/arch/mips/kernel/scall64-64.S
index ad4d4463..97aaf51 100644
--- a/arch/mips/kernel/scall64-64.S
+++ b/arch/mips/kernel/scall64-64.S
@@ -436,4 +436,5 @@ EXPORT(sys_call_table)
PTR sys_memfd_create
PTR sys_bpf /* 5315 */
PTR sys_execveat
+ PTR sys_mlock2
.size sys_call_table,.-sys_call_table
diff --git a/arch/mips/kernel/scall64-n32.S b/arch/mips/kernel/scall64-n32.S
index 446cc65..e36f21e 100644
--- a/arch/mips/kernel/scall64-n32.S
+++ b/arch/mips/kernel/scall64-n32.S
@@ -429,4 +429,5 @@ EXPORT(sysn32_call_table)
PTR sys_memfd_create
PTR sys_bpf
PTR compat_sys_execveat /* 6320 */
+ PTR sys_mlock2
.size sysn32_call_table,.-sysn32_call_table
diff --git a/arch/mips/kernel/scall64-o32.S b/arch/mips/kernel/scall64-o32.S
index f543ff4..7a8b2df 100644
--- a/arch/mips/kernel/scall64-o32.S
+++ b/arch/mips/kernel/scall64-o32.S
@@ -584,4 +584,5 @@ EXPORT(sys32_call_table)
PTR sys_memfd_create
PTR sys_bpf /* 4355 */
PTR compat_sys_execveat
+ PTR sys_mlock2
.size sys32_call_table,.-sys32_call_table
--
1.9.1
^ permalink raw reply related
* [PATCH v7 5/6] selftests: vm: Add tests for lock on fault
From: Eric B Munson @ 2015-08-09 5:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Shuah Khan, Michal Hocko, Vlastimil Babka,
Jonathan Corbet, linux-mm, linux-kernel, linux-api
In-Reply-To: <1439097776-27695-1-git-send-email-emunson@akamai.com>
Test the mmap() flag, and the mlockall() flag. These tests ensure that
pages are not faulted in until they are accessed, that the pages are
unevictable once faulted in, and that VMA splitting and merging works
with the new VM flag. The second test ensures that mlock limits are
respected. Note that the limit test needs to be run a normal user.
Also add tests to use the new mlock2 family of system calls.
Signed-off-by: Eric B Munson <emunson@akamai.com>
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-api@vger.kernel.org
---
tools/testing/selftests/vm/Makefile | 2 +
tools/testing/selftests/vm/mlock2-tests.c | 661 ++++++++++++++++++++++++++++
tools/testing/selftests/vm/on-fault-limit.c | 47 ++
tools/testing/selftests/vm/run_vmtests | 22 +
4 files changed, 732 insertions(+)
create mode 100644 tools/testing/selftests/vm/mlock2-tests.c
create mode 100644 tools/testing/selftests/vm/on-fault-limit.c
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 231b9a0..71a4e9f 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -6,6 +6,8 @@ BINARIES += hugepage-mmap
BINARIES += hugepage-shm
BINARIES += hugetlbfstest
BINARIES += map_hugetlb
+BINARIES += mlock2-tests
+BINARIES += on-fault-limit
BINARIES += thuge-gen
BINARIES += transhuge-stress
diff --git a/tools/testing/selftests/vm/mlock2-tests.c b/tools/testing/selftests/vm/mlock2-tests.c
new file mode 100644
index 0000000..c49122b
--- /dev/null
+++ b/tools/testing/selftests/vm/mlock2-tests.c
@@ -0,0 +1,661 @@
+#include <sys/mman.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+#include <errno.h>
+#include <stdbool.h>
+
+#ifndef MLOCK_ONFAULT
+#define MLOCK_ONFAULT 1
+#endif
+
+#ifndef MCL_ONFAULT
+#define MCL_ONFAULT (MCL_FUTURE << 1)
+#endif
+
+static int mlock2_(void *start, size_t len, int flags)
+{
+#ifdef __NR_mlock2
+ return syscall(__NR_mlock2, start, len, flags);
+#else
+ errno = ENOSYS;
+ return -1;
+#endif
+}
+
+struct vm_boundaries {
+ unsigned long start;
+ unsigned long end;
+};
+
+static int get_vm_area(unsigned long addr, struct vm_boundaries *area)
+{
+ FILE *file;
+ int ret = 1;
+ char line[1024] = {0};
+ char *end_addr;
+ char *stop;
+ unsigned long start;
+ unsigned long end;
+
+ if (!area)
+ return ret;
+
+ file = fopen("/proc/self/maps", "r");
+ if (!file) {
+ perror("fopen");
+ return ret;
+ }
+
+ memset(area, 0, sizeof(struct vm_boundaries));
+
+ while(fgets(line, 1024, file)) {
+ end_addr = strchr(line, '-');
+ if (!end_addr) {
+ printf("cannot parse /proc/self/maps\n");
+ goto out;
+ }
+ *end_addr = '\0';
+ end_addr++;
+ stop = strchr(end_addr, ' ');
+ if (!stop) {
+ printf("cannot parse /proc/self/maps\n");
+ goto out;
+ }
+ stop = '\0';
+
+ sscanf(line, "%lx", &start);
+ sscanf(end_addr, "%lx", &end);
+
+ if (start <= addr && end > addr) {
+ area->start = start;
+ area->end = end;
+ ret = 0;
+ goto out;
+ }
+ }
+out:
+ fclose(file);
+ return ret;
+}
+
+static unsigned long get_pageflags(unsigned long addr)
+{
+ FILE *file;
+ unsigned long pfn;
+ unsigned long offset;
+
+ file = fopen("/proc/self/pagemap", "r");
+ if (!file) {
+ perror("fopen pagemap");
+ _exit(1);
+ }
+
+ offset = addr / getpagesize() * sizeof(unsigned long);
+ if (fseek(file, offset, SEEK_SET)) {
+ perror("fseek pagemap");
+ _exit(1);
+ }
+
+ if (fread(&pfn, sizeof(unsigned long), 1, file) != 1) {
+ perror("fread pagemap");
+ _exit(1);
+ }
+
+ fclose(file);
+ return pfn;
+}
+
+static unsigned long get_kpageflags(unsigned long pfn)
+{
+ unsigned long flags;
+ FILE *file;
+
+ file = fopen("/proc/kpageflags", "r");
+ if (!file) {
+ perror("fopen kpageflags");
+ _exit(1);
+ }
+
+ if (fseek(file, pfn * sizeof(unsigned long), SEEK_SET)) {
+ perror("fseek kpageflags");
+ _exit(1);
+ }
+
+ if (fread(&flags, sizeof(unsigned long), 1, file) != 1) {
+ perror("fread kpageflags");
+ _exit(1);
+ }
+
+ fclose(file);
+ return flags;
+}
+
+#define VMFLAGS "VmFlags:"
+
+static bool find_flag(FILE *file, const char *vmflag)
+{
+ char *line = NULL;
+ char *flags;
+ size_t size = 0;
+ bool ret = false;
+
+ while (getline(&line, &size, file) > 0) {
+ if (!strstr(line, VMFLAGS)) {
+ free(line);
+ line = NULL;
+ size = 0;
+ continue;
+ }
+
+ flags = line + strlen(VMFLAGS);
+ ret = (strstr(flags, vmflag) != NULL);
+ goto out;
+ }
+
+out:
+ free(line);
+ return ret;
+}
+
+static bool is_vmflag_set(unsigned long addr, const char *vmflag)
+{
+ FILE *file;
+ char *line = NULL;
+ size_t size = 0;
+ bool ret = false;
+ unsigned long start, end;
+ char perms[5];
+ unsigned long offset;
+ char dev[32];
+ unsigned long inode;
+ char path[BUFSIZ];
+
+ file = fopen("/proc/self/smaps", "r");
+ if (!file) {
+ perror("fopen smaps");
+ _exit(1);
+ }
+
+ while (getline(&line, &size, file) > 0) {
+ if (sscanf(line, "%lx-%lx %s %lx %s %lu %s\n",
+ &start, &end, perms, &offset, dev, &inode, path) < 6)
+ goto next;
+
+ if (start <= addr && addr < end) {
+ ret = find_flag(file, vmflag);
+ goto out;
+ }
+
+next:
+ free(line);
+ line = NULL;
+ size = 0;
+ }
+
+out:
+ free(line);
+ fclose(file);
+ return ret;
+}
+
+#define PRESENT_BIT 0x8000000000000000
+#define PFN_MASK 0x007FFFFFFFFFFFFF
+#define UNEVICTABLE_BIT (1UL << 18)
+
+#define LOCKED "lo"
+#define LOCKEDONFAULT "lf"
+
+static int lock_check(char *map)
+{
+ unsigned long page1_flags;
+ unsigned long page2_flags;
+ unsigned long page_size = getpagesize();
+
+ page1_flags = get_pageflags((unsigned long)map);
+ page2_flags = get_pageflags((unsigned long)map + page_size);
+
+ /* Both pages should be present */
+ if (((page1_flags & PRESENT_BIT) == 0) ||
+ ((page2_flags & PRESENT_BIT) == 0)) {
+ printf("Failed to make both pages present\n");
+ return 1;
+ }
+
+ page1_flags = get_kpageflags(page1_flags & PFN_MASK);
+ page2_flags = get_kpageflags(page2_flags & PFN_MASK);
+
+ /* Both pages should be unevictable */
+ if (((page1_flags & UNEVICTABLE_BIT) == 0) ||
+ ((page2_flags & UNEVICTABLE_BIT) == 0)) {
+ printf("Failed to make both pages unevictable\n");
+ return 1;
+ }
+
+ if (!is_vmflag_set((unsigned long)map, LOCKED) ||
+ !is_vmflag_set((unsigned long)map + page_size, LOCKED)) {
+ printf("VMA flag %s is missing\n", LOCKED);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int unlock_lock_check(char *map)
+{
+ unsigned long page1_flags;
+ unsigned long page2_flags;
+ unsigned long page_size = getpagesize();
+
+ page1_flags = get_pageflags((unsigned long)map);
+ page2_flags = get_pageflags((unsigned long)map + page_size);
+ page1_flags = get_kpageflags(page1_flags & PFN_MASK);
+ page2_flags = get_kpageflags(page2_flags & PFN_MASK);
+
+ if ((page1_flags & UNEVICTABLE_BIT) || (page2_flags & UNEVICTABLE_BIT)) {
+ printf("A page is still marked unevictable after unlock\n");
+ return 1;
+ }
+
+ if (is_vmflag_set((unsigned long)map, LOCKED) ||
+ is_vmflag_set((unsigned long)map + page_size, LOCKED)) {
+ printf("VMA flag %s is still set after unlock\n", LOCKED);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int test_mlock_lock()
+{
+ char *map;
+ int ret = 1;
+ unsigned long page_size = getpagesize();
+
+ map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+ if (map == MAP_FAILED) {
+ perror("test_mlock_locked mmap");
+ goto out;
+ }
+
+ if (mlock2_(map, 2 * page_size, 0)) {
+ if (errno == ENOSYS) {
+ printf("Cannot call new mlock family, skipping test\n");
+ _exit(0);
+ }
+ perror("mlock2(0)");
+ goto unmap;
+ }
+
+ if (lock_check(map))
+ goto unmap;
+
+ /* Now unlock and recheck attributes */
+ if (munlock(map, 2 * page_size)) {
+ perror("munlock()");
+ goto unmap;
+ }
+
+ ret = unlock_lock_check(map);
+
+unmap:
+ munmap(map, 2 * page_size);
+out:
+ return ret;
+}
+
+static int onfault_check(char *map)
+{
+ unsigned long page1_flags;
+ unsigned long page2_flags;
+ unsigned long page_size = getpagesize();
+
+ page1_flags = get_pageflags((unsigned long)map);
+ page2_flags = get_pageflags((unsigned long)map + page_size);
+
+ /* Neither page should be present */
+ if ((page1_flags & PRESENT_BIT) || (page2_flags & PRESENT_BIT)) {
+ printf("Pages were made present by MLOCK_ONFAULT\n");
+ return 1;
+ }
+
+ *map = 'a';
+ page1_flags = get_pageflags((unsigned long)map);
+ page2_flags = get_pageflags((unsigned long)map + page_size);
+
+ /* Only page 1 should be present */
+ if ((page1_flags & PRESENT_BIT) == 0) {
+ printf("Page 1 is not present after fault\n");
+ return 1;
+ } else if (page2_flags & PRESENT_BIT) {
+ printf("Page 2 was made present\n");
+ return 1;
+ }
+
+ page1_flags = get_kpageflags(page1_flags & PFN_MASK);
+
+ /* Page 1 should be unevictable */
+ if ((page1_flags & UNEVICTABLE_BIT) == 0) {
+ printf("Failed to make faulted page unevictable\n");
+ return 1;
+ }
+
+ if (!is_vmflag_set((unsigned long)map, LOCKEDONFAULT) ||
+ !is_vmflag_set((unsigned long)map + page_size, LOCKEDONFAULT)) {
+ printf("VMA flag %s is missing\n", LOCKEDONFAULT);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int unlock_onfault_check(char *map)
+{
+ unsigned long page1_flags;
+ unsigned long page2_flags;
+ unsigned long page_size = getpagesize();
+
+ page1_flags = get_pageflags((unsigned long)map);
+ page1_flags = get_kpageflags(page1_flags & PFN_MASK);
+
+ if (page1_flags & UNEVICTABLE_BIT) {
+ printf("Page 1 is still marked unevictable after unlock\n");
+ return 1;
+ }
+
+ if (is_vmflag_set((unsigned long)map, LOCKEDONFAULT) ||
+ is_vmflag_set((unsigned long)map + page_size, LOCKEDONFAULT)) {
+ printf("VMA flag %s is still set after unlock\n", LOCKEDONFAULT);
+ return 1;
+ }
+
+ return 0;
+}
+
+static int test_mlock_onfault()
+{
+ char *map;
+ int ret = 1;
+ unsigned long page_size = getpagesize();
+
+ map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+ if (map == MAP_FAILED) {
+ perror("test_mlock_locked mmap");
+ goto out;
+ }
+
+ if (mlock2_(map, 2 * page_size, MLOCK_ONFAULT)) {
+ if (errno == ENOSYS) {
+ printf("Cannot call new mlock family, skipping test\n");
+ _exit(0);
+ }
+ perror("mlock2(MLOCK_ONFAULT)");
+ goto unmap;
+ }
+
+ if (onfault_check(map))
+ goto unmap;
+
+ /* Now unlock and recheck attributes */
+ if (munlock(map, 2 * page_size)) {
+ if (errno == ENOSYS) {
+ printf("Cannot call new mlock family, skipping test\n");
+ _exit(0);
+ }
+ perror("munlock()");
+ goto unmap;
+ }
+
+ ret = unlock_onfault_check(map);
+unmap:
+ munmap(map, 2 * page_size);
+out:
+ return ret;
+}
+
+static int test_lock_onfault_of_present()
+{
+ char *map;
+ int ret = 1;
+ unsigned long page1_flags;
+ unsigned long page2_flags;
+ unsigned long page_size = getpagesize();
+
+ map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+ if (map == MAP_FAILED) {
+ perror("test_mlock_locked mmap");
+ goto out;
+ }
+
+ *map = 'a';
+
+ if (mlock2_(map, 2 * page_size, MLOCK_ONFAULT)) {
+ if (errno == ENOSYS) {
+ printf("Cannot call new mlock family, skipping test\n");
+ _exit(0);
+ }
+ perror("mlock2(MLOCK_ONFAULT)");
+ goto unmap;
+ }
+
+ page1_flags = get_pageflags((unsigned long)map);
+ page2_flags = get_pageflags((unsigned long)map + page_size);
+ page1_flags = get_kpageflags(page1_flags & PFN_MASK);
+ page2_flags = get_kpageflags(page2_flags & PFN_MASK);
+
+ /* Page 1 should be unevictable */
+ if ((page1_flags & UNEVICTABLE_BIT) == 0) {
+ printf("Failed to make present page unevictable\n");
+ goto unmap;
+ }
+
+ if (!is_vmflag_set((unsigned long)map, LOCKEDONFAULT) ||
+ !is_vmflag_set((unsigned long)map + page_size, LOCKEDONFAULT)) {
+ printf("VMA flag %s is missing for one of the pages\n", LOCKEDONFAULT);
+ goto unmap;
+ }
+ ret = 0;
+unmap:
+ munmap(map, 2 * page_size);
+out:
+ return ret;
+}
+
+static int test_munlockall()
+{
+ char *map;
+ int ret = 1;
+ unsigned long page1_flags;
+ unsigned long page2_flags;
+ unsigned long page_size = getpagesize();
+
+ map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+
+ if (map == MAP_FAILED) {
+ perror("test_munlockall mmap");
+ goto out;
+ }
+
+ if (mlockall(MCL_CURRENT)) {
+ perror("mlockall(MCL_CURRENT)");
+ goto out;
+ }
+
+ if (lock_check(map))
+ goto unmap;
+
+ if (munlockall()) {
+ perror("munlockall()");
+ goto unmap;
+ }
+
+ if (unlock_lock_check(map))
+ goto unmap;
+
+ munmap(map, 2 * page_size);
+
+ map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+
+ if (map == MAP_FAILED) {
+ perror("test_munlockall second mmap");
+ goto out;
+ }
+
+ if (mlockall(MCL_CURRENT | MCL_ONFAULT)) {
+ perror("mlockall(MCL_CURRENT | MCL_ONFAULT)");
+ goto unmap;
+ }
+
+ if (onfault_check(map))
+ goto unmap;
+
+ if (munlockall()) {
+ perror("munlockall()");
+ goto unmap;
+ }
+
+ if (unlock_onfault_check(map))
+ goto unmap;
+
+ if (mlockall(MCL_CURRENT | MCL_FUTURE)) {
+ perror("mlockall(MCL_CURRENT | MCL_FUTURE)");
+ goto out;
+ }
+
+ if (lock_check(map))
+ goto unmap;
+
+ if (munlockall()) {
+ perror("munlockall()");
+ goto unmap;
+ }
+
+ ret = unlock_lock_check(map);
+
+unmap:
+ munmap(map, 2 * page_size);
+out:
+ munlockall();
+ return ret;
+}
+
+static int test_vma_management(bool call_mlock)
+{
+ int ret = 1;
+ void *map;
+ unsigned long page_size = getpagesize();
+ struct vm_boundaries page1;
+ struct vm_boundaries page2;
+ struct vm_boundaries page3;
+
+ map = mmap(NULL, 3 * page_size, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
+ if (map == MAP_FAILED) {
+ perror("mmap()");
+ return ret;
+ }
+
+ if (call_mlock && mlock2_(map, 3 * page_size, MLOCK_ONFAULT)) {
+ if (errno == ENOSYS) {
+ printf("Cannot call new mlock family, skipping test\n");
+ _exit(0);
+ }
+ perror("mlock(ONFAULT)\n");
+ goto out;
+ }
+
+ if (get_vm_area((unsigned long)map, &page1) ||
+ get_vm_area((unsigned long)map + page_size, &page2) ||
+ get_vm_area((unsigned long)map + page_size * 2, &page3)) {
+ printf("couldn't find mapping in /proc/self/maps\n");
+ goto out;
+ }
+
+ /*
+ * Before we unlock a portion, we need to that all three pages are in
+ * the same VMA. If they are not we abort this test (Note that this is
+ * not a failure)
+ */
+ if (page1.start != page2.start || page2.start != page3.start) {
+ printf("VMAs are not merged to start, aborting test\n");
+ ret = 0;
+ goto out;
+ }
+
+ if (munlock(map + page_size, page_size)) {
+ perror("munlock()");
+ goto out;
+ }
+
+ if (get_vm_area((unsigned long)map, &page1) ||
+ get_vm_area((unsigned long)map + page_size, &page2) ||
+ get_vm_area((unsigned long)map + page_size * 2, &page3)) {
+ printf("couldn't find mapping in /proc/self/maps\n");
+ goto out;
+ }
+
+ /* All three VMAs should be different */
+ if (page1.start == page2.start || page2.start == page3.start) {
+ printf("failed to split VMA for munlock\n");
+ goto out;
+ }
+
+ /* Now unlock the first and third page and check the VMAs again */
+ if (munlock(map, page_size * 3)) {
+ perror("munlock()");
+ goto out;
+ }
+
+ if (get_vm_area((unsigned long)map, &page1) ||
+ get_vm_area((unsigned long)map + page_size, &page2) ||
+ get_vm_area((unsigned long)map + page_size * 2, &page3)) {
+ printf("couldn't find mapping in /proc/self/maps\n");
+ goto out;
+ }
+
+ /* Now all three VMAs should be the same */
+ if (page1.start != page2.start || page2.start != page3.start) {
+ printf("failed to merge VMAs after munlock\n");
+ goto out;
+ }
+
+ ret = 0;
+out:
+ munmap(map, 3 * page_size);
+ return ret;
+}
+
+static int test_mlockall(int (test_function)(bool call_mlock))
+{
+ int ret = 1;
+
+ if (mlockall(MCL_CURRENT | MCL_ONFAULT | MCL_FUTURE)) {
+ perror("mlockall");
+ return ret;
+ }
+
+ ret = test_function(false);
+ munlockall();
+ return ret;
+}
+
+int main(char **argv, int argc)
+{
+ int ret = 0;
+ ret += test_mlock_lock();
+ ret += test_mlock_onfault();
+ ret += test_munlockall();
+ ret += test_lock_onfault_of_present();
+ ret += test_vma_management(true);
+ ret += test_mlockall(test_vma_management);
+ return ret;
+}
+
diff --git a/tools/testing/selftests/vm/on-fault-limit.c b/tools/testing/selftests/vm/on-fault-limit.c
new file mode 100644
index 0000000..245accc
--- /dev/null
+++ b/tools/testing/selftests/vm/on-fault-limit.c
@@ -0,0 +1,47 @@
+#include <sys/mman.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/time.h>
+#include <sys/resource.h>
+
+#ifndef MCL_ONFAULT
+#define MCL_ONFAULT (MCL_FUTURE << 1)
+#endif
+
+static int test_limit(void)
+{
+ int ret = 1;
+ struct rlimit lims;
+ void *map;
+
+ if (getrlimit(RLIMIT_MEMLOCK, &lims)) {
+ perror("getrlimit");
+ return ret;
+ }
+
+ if (mlockall(MCL_CURRENT | MCL_ONFAULT | MCL_FUTURE)) {
+ perror("mlockall");
+ return ret;
+ }
+
+ map = mmap(NULL, 2 * lims.rlim_max, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_POPULATE, 0, 0);
+ if (map != MAP_FAILED)
+ printf("mmap should have failed, but didn't\n");
+ else {
+ ret = 0;
+ munmap(map, 2 * lims.rlim_max);
+ }
+
+ munlockall();
+ return ret;
+}
+
+int main(int argc, char **argv)
+{
+ int ret = 0;
+
+ ret += test_limit();
+ return ret;
+}
diff --git a/tools/testing/selftests/vm/run_vmtests b/tools/testing/selftests/vm/run_vmtests
index 49ece11..877ca04a 100755
--- a/tools/testing/selftests/vm/run_vmtests
+++ b/tools/testing/selftests/vm/run_vmtests
@@ -102,4 +102,26 @@ else
echo "[PASS]"
fi
+echo "--------------------"
+echo "running on-fault-limit"
+echo "--------------------"
+sudo -u nobody ./on-fault-limit
+if [ $? -ne 0 ]; then
+ echo "[FAIL]"
+ exitcode=1
+else
+ echo "[PASS]"
+fi
+
+echo "--------------------"
+echo "running mlock2-tests"
+echo "--------------------"
+./mlock2-tests
+if [ $? -ne 0 ]; then
+ echo "[FAIL]"
+ exitcode=1
+else
+ echo "[PASS]"
+fi
+
exit $exitcode
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v7 4/6] mm: mlock: Add mlock flags to enable VM_LOCKONFAULT usage
From: Eric B Munson @ 2015-08-09 5:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Michal Hocko, Vlastimil Babka, Jonathan Corbet,
Kirill A. Shutemov, linux-alpha, linux-kernel, linux-mips,
linux-parisc, linuxppc-dev, sparclinux, linux-xtensa, linux-arch,
linux-api, linux-mm
In-Reply-To: <1439097776-27695-1-git-send-email-emunson@akamai.com>
The previous patch introduced a flag that specified pages in a VMA
should be placed on the unevictable LRU, but they should not be made
present when the area is created. This patch adds the ability to set
this state via the new mlock system calls.
We add MLOCK_ONFAULT for mlock2 and MCL_ONFAULT for mlockall.
MLOCK_ONFAULT will set the VM_LOCKONFAULT modifier for VM_LOCKED.
MCL_ONFAULT should be used as a modifier to the two other mlockall
flags. When used with MCL_CURRENT, all current mappings will be marked
with VM_LOCKED | VM_LOCKONFAULT. When used with MCL_FUTURE, the
mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT. When used
with both MCL_CURRENT and MCL_FUTURE, all current mappings and
mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT.
Prior to this patch, mlockall() will unconditionally clear the
mm->def_flags any time it is called without MCL_FUTURE. This behavior
is maintained after adding MCL_ONFAULT. If a call to
mlockall(MCL_FUTURE) is followed by mlockall(MCL_CURRENT), the
mm->def_flags will be cleared and new VMAs will be unlocked. This
remains true with or without MCL_ONFAULT in either mlockall()
invocation.
munlock() will unconditionally clear both vma flags. munlockall()
unconditionally clears for VMA flags on all VMAs and in the
mm->def_flags field.
Signed-off-by: Eric B Munson <emunson@akamai.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparclinux@vger.kernel.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-arch@vger.kernel.org
Cc: linux-api@vger.kernel.org
Cc: linux-mm@kvack.org
---
arch/alpha/include/uapi/asm/mman.h | 3 ++
arch/mips/include/uapi/asm/mman.h | 6 ++++
arch/parisc/include/uapi/asm/mman.h | 3 ++
arch/powerpc/include/uapi/asm/mman.h | 1 +
arch/sparc/include/uapi/asm/mman.h | 1 +
arch/tile/include/uapi/asm/mman.h | 1 +
arch/xtensa/include/uapi/asm/mman.h | 6 ++++
include/uapi/asm-generic/mman-common.h | 5 ++++
include/uapi/asm-generic/mman.h | 1 +
mm/mlock.c | 53 +++++++++++++++++++++++++---------
10 files changed, 67 insertions(+), 13 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
index 0086b47..f2f9496 100644
--- a/arch/alpha/include/uapi/asm/mman.h
+++ b/arch/alpha/include/uapi/asm/mman.h
@@ -37,6 +37,9 @@
#define MCL_CURRENT 8192 /* lock all currently mapped pages */
#define MCL_FUTURE 16384 /* lock all additions to address space */
+#define MCL_ONFAULT 32768 /* lock all pages that are faulted in */
+
+#define MLOCK_ONFAULT 0x01 /* Lock pages in range after they are faulted in, do not prefault */
#define MADV_NORMAL 0 /* no further special treatment */
#define MADV_RANDOM 1 /* expect random page references */
diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
index cfcb876..97c03f4 100644
--- a/arch/mips/include/uapi/asm/mman.h
+++ b/arch/mips/include/uapi/asm/mman.h
@@ -61,6 +61,12 @@
*/
#define MCL_CURRENT 1 /* lock all current mappings */
#define MCL_FUTURE 2 /* lock all future mappings */
+#define MCL_ONFAULT 4 /* lock all pages that are faulted in */
+
+/*
+ * Flags for mlock
+ */
+#define MLOCK_ONFAULT 0x01 /* Lock pages in range after they are faulted in, do not prefault */
#define MADV_NORMAL 0 /* no further special treatment */
#define MADV_RANDOM 1 /* expect random page references */
diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
index 294d251..ecc3ae1 100644
--- a/arch/parisc/include/uapi/asm/mman.h
+++ b/arch/parisc/include/uapi/asm/mman.h
@@ -31,6 +31,9 @@
#define MCL_CURRENT 1 /* lock all current mappings */
#define MCL_FUTURE 2 /* lock all future mappings */
+#define MCL_ONFAULT 4 /* lock all pages that are faulted in */
+
+#define MLOCK_ONFAULT 0x01 /* Lock pages in range after they are faulted in, do not prefault */
#define MADV_NORMAL 0 /* no further special treatment */
#define MADV_RANDOM 1 /* expect random page references */
diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
index 6ea26df..03c06ba 100644
--- a/arch/powerpc/include/uapi/asm/mman.h
+++ b/arch/powerpc/include/uapi/asm/mman.h
@@ -22,6 +22,7 @@
#define MCL_CURRENT 0x2000 /* lock all currently mapped pages */
#define MCL_FUTURE 0x4000 /* lock all additions to address space */
+#define MCL_ONFAULT 0x8000 /* lock all pages that are faulted in */
#define MAP_POPULATE 0x8000 /* populate (prefault) pagetables */
#define MAP_NONBLOCK 0x10000 /* do not block on IO */
diff --git a/arch/sparc/include/uapi/asm/mman.h b/arch/sparc/include/uapi/asm/mman.h
index 0b14df3..9765896 100644
--- a/arch/sparc/include/uapi/asm/mman.h
+++ b/arch/sparc/include/uapi/asm/mman.h
@@ -17,6 +17,7 @@
#define MCL_CURRENT 0x2000 /* lock all currently mapped pages */
#define MCL_FUTURE 0x4000 /* lock all additions to address space */
+#define MCL_ONFAULT 0x8000 /* lock all pages that are faulted in */
#define MAP_POPULATE 0x8000 /* populate (prefault) pagetables */
#define MAP_NONBLOCK 0x10000 /* do not block on IO */
diff --git a/arch/tile/include/uapi/asm/mman.h b/arch/tile/include/uapi/asm/mman.h
index 81b8fc3..63ee13f 100644
--- a/arch/tile/include/uapi/asm/mman.h
+++ b/arch/tile/include/uapi/asm/mman.h
@@ -36,6 +36,7 @@
*/
#define MCL_CURRENT 1 /* lock all current mappings */
#define MCL_FUTURE 2 /* lock all future mappings */
+#define MCL_ONFAULT 4 /* lock all pages that are faulted in */
#endif /* _ASM_TILE_MMAN_H */
diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
index 201aec0..360944e 100644
--- a/arch/xtensa/include/uapi/asm/mman.h
+++ b/arch/xtensa/include/uapi/asm/mman.h
@@ -74,6 +74,12 @@
*/
#define MCL_CURRENT 1 /* lock all current mappings */
#define MCL_FUTURE 2 /* lock all future mappings */
+#define MCL_ONFAULT 4 /* lock all pages that are faulted in */
+
+/*
+ * Flags for mlock
+ */
+#define MLOCK_ONFAULT 0x01 /* Lock pages in range after they are faulted in, do not prefault */
#define MADV_NORMAL 0 /* no further special treatment */
#define MADV_RANDOM 1 /* expect random page references */
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index ddc3b36..a74dd84 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -25,6 +25,11 @@
# define MAP_UNINITIALIZED 0x0 /* Don't support this flag */
#endif
+/*
+ * Flags for mlock
+ */
+#define MLOCK_ONFAULT 0x01 /* Lock pages in range after they are faulted in, do not prefault */
+
#define MS_ASYNC 1 /* sync memory asynchronously */
#define MS_INVALIDATE 2 /* invalidate the caches */
#define MS_SYNC 4 /* synchronous memory sync */
diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
index e9fe6fd..7162cd4 100644
--- a/include/uapi/asm-generic/mman.h
+++ b/include/uapi/asm-generic/mman.h
@@ -17,5 +17,6 @@
#define MCL_CURRENT 1 /* lock all current mappings */
#define MCL_FUTURE 2 /* lock all future mappings */
+#define MCL_ONFAULT 4 /* lock all pages that are faulted in */
#endif /* __ASM_GENERIC_MMAN_H */
diff --git a/mm/mlock.c b/mm/mlock.c
index 029a75b..7e1b8c5 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -506,7 +506,8 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
if (newflags == vma->vm_flags || (vma->vm_flags & VM_SPECIAL) ||
is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm))
- goto out; /* don't set VM_LOCKED, don't count */
+ /* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
+ goto out;
pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT);
*prev = vma_merge(mm, *prev, start, end, newflags, vma->anon_vma,
@@ -576,7 +577,8 @@ static int apply_vma_lock_flags(unsigned long start, size_t len,
prev = vma;
for (nstart = start ; ; ) {
- vm_flags_t newflags = vma->vm_flags & ~VM_LOCKED;
+ vm_flags_t newflags =
+ vma->vm_flags & ~(VM_LOCKED | VM_LOCKONFAULT);
newflags |= flags;
@@ -645,10 +647,15 @@ SYSCALL_DEFINE2(mlock, unsigned long, start, size_t, len)
SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
{
- if (flags)
+ vm_flags_t vm_flags = VM_LOCKED;
+
+ if (flags & ~MLOCK_ONFAULT)
return -EINVAL;
- return do_mlock(start, len, VM_LOCKED);
+ if (flags & MLOCK_ONFAULT)
+ vm_flags |= VM_LOCKONFAULT;
+
+ return do_mlock(start, len, vm_flags);
}
SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
@@ -665,24 +672,43 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
return ret;
}
+/*
+ * Take the MCL_* flags passed into mlockall (or 0 if called from munlockall)
+ * and translate into the appropriate modifications to mm->def_flags and/or the
+ * flags for all current VMAs.
+ *
+ * There are a couple of subtleties with this. If mlockall() is called multiple
+ * times with different flags, the values do not necessarily stack. If mlockall
+ * is called once including the MCL_FUTURE flag and then a second time without
+ * it, VM_LOCKED and VM_LOCKONFAULT will be cleared from mm->def_flags.
+ */
static int apply_mlockall_flags(int flags)
{
struct vm_area_struct * vma, * prev = NULL;
+ vm_flags_t to_add = 0;
- if (flags & MCL_FUTURE)
+ current->mm->def_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
+ if (flags & MCL_FUTURE) {
current->mm->def_flags |= VM_LOCKED;
- else
- current->mm->def_flags &= ~VM_LOCKED;
- if (flags == MCL_FUTURE)
- goto out;
+ if (flags & MCL_ONFAULT)
+ current->mm->def_flags |= VM_LOCKONFAULT;
+
+ if (!(flags & MCL_CURRENT))
+ goto out;
+ }
+
+ if (flags & MCL_CURRENT) {
+ to_add |= VM_LOCKED;
+ if (flags & MCL_ONFAULT)
+ to_add |= VM_LOCKONFAULT;
+ }
for (vma = current->mm->mmap; vma ; vma = prev->vm_next) {
vm_flags_t newflags;
- newflags = vma->vm_flags & ~VM_LOCKED;
- if (flags & MCL_CURRENT)
- newflags |= VM_LOCKED;
+ newflags = vma->vm_flags & ~(VM_LOCKED | VM_LOCKONFAULT);
+ newflags |= to_add;
/* Ignore errors */
mlock_fixup(vma, &prev, vma->vm_start, vma->vm_end, newflags);
@@ -697,7 +723,8 @@ SYSCALL_DEFINE1(mlockall, int, flags)
unsigned long lock_limit;
int ret = -EINVAL;
- if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE)))
+ if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT)) ||
+ flags == MCL_ONFAULT)
goto out;
ret = -EPERM;
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v7 3/6] mm: Introduce VM_LOCKONFAULT
From: Eric B Munson @ 2015-08-09 5:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Michal Hocko, Vlastimil Babka, Jonathan Corbet,
Kirill A. Shutemov, linux-kernel, dri-devel, linux-mm, linux-api
In-Reply-To: <1439097776-27695-1-git-send-email-emunson@akamai.com>
The cost of faulting in all memory to be locked can be very high when
working with large mappings. If only portions of the mapping will be
used this can incur a high penalty for locking.
For the example of a large file, this is the usage pattern for a large
statical language model (probably applies to other statical or graphical
models as well). For the security example, any application transacting
in data that cannot be swapped out (credit card data, medical records,
etc).
This patch introduces the ability to request that pages are not
pre-faulted, but are placed on the unevictable LRU when they are finally
faulted in. The VM_LOCKONFAULT flag will be used together with
VM_LOCKED and has no effect when set without VM_LOCKED. Setting the
VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
be added to the unevictable LRU when they are faulted or if they are
already present, but will not cause any missing pages to be faulted in.
Exposing this new lock state means that we cannot overload the meaning
of the FOLL_POPULATE flag any longer. Prior to this patch it was used
to mean that the VMA for a fault was locked. This means we need the
new FOLL_MLOCK flag to communicate the locked state of a VMA.
FOLL_POPULATE will now only control if the VMA should be populated and
in the case of VM_LOCKONFAULT, it will not be set.
Signed-off-by: Eric B Munson <emunson@akamai.com>
Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mm@kvack.org
Cc: linux-api@vger.kernel.org
---
Documentation/filesystems/proc.txt | 1 +
drivers/gpu/drm/drm_vm.c | 8 +++++++-
fs/proc/task_mmu.c | 1 +
include/linux/mm.h | 2 ++
kernel/fork.c | 2 +-
mm/debug.c | 1 +
mm/gup.c | 10 ++++++++--
mm/huge_memory.c | 2 +-
mm/hugetlb.c | 4 ++--
mm/mlock.c | 2 +-
mm/mmap.c | 2 +-
mm/rmap.c | 6 ++++--
12 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index 6f7fafd..ed21989 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -463,6 +463,7 @@ manner. The codes are the following:
rr - random read advise provided
dc - do not copy area on fork
de - do not expand area on remapping
+ lf - mark area to lock pages when faulted in, do not pre-populate
ac - area is accountable
nr - swap space is not reserved for the area
ht - area uses huge tlb pages
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index aab49ee..103a5f6 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -699,9 +699,15 @@ int drm_vma_info(struct seq_file *m, void *data)
(void *)(unsigned long)virt_to_phys(high_memory));
list_for_each_entry(pt, &dev->vmalist, head) {
+ char lock_flag = '-';
+
vma = pt->vma;
if (!vma)
continue;
+ if (vma->vm_flags & VM_LOCKONFAULT)
+ lock_flag = 'f';
+ else if (vma->vm_flags & VM_LOCKED)
+ lock_flag = 'l';
seq_printf(m,
"\n%5d 0x%pK-0x%pK %c%c%c%c%c%c 0x%08lx000",
pt->pid,
@@ -710,7 +716,7 @@ int drm_vma_info(struct seq_file *m, void *data)
vma->vm_flags & VM_WRITE ? 'w' : '-',
vma->vm_flags & VM_EXEC ? 'x' : '-',
vma->vm_flags & VM_MAYSHARE ? 's' : 'p',
- vma->vm_flags & VM_LOCKED ? 'l' : '-',
+ lock_flag,
vma->vm_flags & VM_IO ? 'i' : '-',
vma->vm_pgoff);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index ca1e091..8dcc297 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -585,6 +585,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
[ilog2(VM_RAND_READ)] = "rr",
[ilog2(VM_DONTCOPY)] = "dc",
[ilog2(VM_DONTEXPAND)] = "de",
+ [ilog2(VM_LOCKONFAULT)] = "lf",
[ilog2(VM_ACCOUNT)] = "ac",
[ilog2(VM_NORESERVE)] = "nr",
[ilog2(VM_HUGETLB)] = "ht",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 2e872f9..d6e1637 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -136,6 +136,7 @@ extern unsigned int kobjsize(const void *objp);
#define VM_DONTCOPY 0x00020000 /* Do not copy this vma on fork */
#define VM_DONTEXPAND 0x00040000 /* Cannot expand with mremap() */
+#define VM_LOCKONFAULT 0x00080000 /* Lock the pages covered when they are faulted in */
#define VM_ACCOUNT 0x00100000 /* Is a VM accounted object */
#define VM_NORESERVE 0x00200000 /* should the VM suppress accounting */
#define VM_HUGETLB 0x00400000 /* Huge TLB Page VM */
@@ -2043,6 +2044,7 @@ static inline struct page *follow_page(struct vm_area_struct *vma,
#define FOLL_NUMA 0x200 /* force NUMA hinting page fault */
#define FOLL_MIGRATION 0x400 /* wait for page to replace migration entry */
#define FOLL_TRIED 0x800 /* a retry, previous pass started an IO */
+#define FOLL_MLOCK 0x1000 /* lock present pages */
typedef int (*pte_fn_t)(pte_t *pte, pgtable_t token, unsigned long addr,
void *data);
diff --git a/kernel/fork.c b/kernel/fork.c
index dbd9b8d..a949228 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -454,7 +454,7 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
tmp->vm_mm = mm;
if (anon_vma_fork(tmp, mpnt))
goto fail_nomem_anon_vma_fork;
- tmp->vm_flags &= ~VM_LOCKED;
+ tmp->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
tmp->vm_next = tmp->vm_prev = NULL;
file = tmp->vm_file;
if (file) {
diff --git a/mm/debug.c b/mm/debug.c
index 76089dd..25176bb 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -121,6 +121,7 @@ static const struct trace_print_flags vmaflags_names[] = {
{VM_GROWSDOWN, "growsdown" },
{VM_PFNMAP, "pfnmap" },
{VM_DENYWRITE, "denywrite" },
+ {VM_LOCKONFAULT, "lockonfault" },
{VM_LOCKED, "locked" },
{VM_IO, "io" },
{VM_SEQ_READ, "seqread" },
diff --git a/mm/gup.c b/mm/gup.c
index 6297f6b..dce6ccd 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -92,7 +92,7 @@ retry:
*/
mark_page_accessed(page);
}
- if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
+ if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
/*
* The preliminary mapping check is mainly to avoid the
* pointless overhead of lock_page on the ZERO_PAGE
@@ -265,6 +265,9 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,
unsigned int fault_flags = 0;
int ret;
+ /* mlock all present pages, but do not fault in new pages */
+ if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
+ return -ENOENT;
/* For mm_populate(), just skip the stack guard page. */
if ((*flags & FOLL_POPULATE) &&
(stack_guard_page_start(vma, address) ||
@@ -850,7 +853,10 @@ long populate_vma_page_range(struct vm_area_struct *vma,
VM_BUG_ON_VMA(end > vma->vm_end, vma);
VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_sem), mm);
- gup_flags = FOLL_TOUCH | FOLL_POPULATE;
+ gup_flags = FOLL_TOUCH | FOLL_POPULATE | FOLL_MLOCK;
+ if (vma->vm_flags & VM_LOCKONFAULT)
+ gup_flags &= ~FOLL_POPULATE;
+
/*
* We want to touch writable mappings with a write fault in order
* to break COW, except for shared mappings because these don't COW
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 097c7a4..cba783e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1238,7 +1238,7 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
pmd, _pmd, 1))
update_mmu_cache_pmd(vma, addr, pmd);
}
- if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
+ if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
if (page->mapping && trylock_page(page)) {
lru_add_drain();
if (page->mapping)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a8c3087..4ed9e93 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3764,8 +3764,8 @@ static unsigned long page_table_shareable(struct vm_area_struct *svma,
unsigned long s_end = sbase + PUD_SIZE;
/* Allow segments to share if only one is marked locked */
- unsigned long vm_flags = vma->vm_flags & ~VM_LOCKED;
- unsigned long svm_flags = svma->vm_flags & ~VM_LOCKED;
+ unsigned long vm_flags = vma->vm_flags & ~(VM_LOCKED|VM_LOCKONFAULT);
+ unsigned long svm_flags = svma->vm_flags & ~(VM_LOCKED|VM_LOCKONFAULT);
/*
* match the virtual addresses, permission and the alignment of the
diff --git a/mm/mlock.c b/mm/mlock.c
index 3094f27..029a75b 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -422,7 +422,7 @@ static unsigned long __munlock_pagevec_fill(struct pagevec *pvec,
void munlock_vma_pages_range(struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
- vma->vm_flags &= ~VM_LOCKED;
+ vma->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
while (start < end) {
struct page *page = NULL;
diff --git a/mm/mmap.c b/mm/mmap.c
index aa632ad..bdbefc3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1651,7 +1651,7 @@ out:
vma == get_gate_vma(current->mm)))
mm->locked_vm += (len >> PAGE_SHIFT);
else
- vma->vm_flags &= ~VM_LOCKED;
+ vma->vm_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
}
if (file)
diff --git a/mm/rmap.c b/mm/rmap.c
index 171b687..14ce002 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -744,7 +744,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
if (vma->vm_flags & VM_LOCKED) {
spin_unlock(ptl);
- pra->vm_flags |= VM_LOCKED;
+ pra->vm_flags |=
+ (vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
return SWAP_FAIL; /* To break the loop */
}
@@ -765,7 +766,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
if (vma->vm_flags & VM_LOCKED) {
pte_unmap_unlock(pte, ptl);
- pra->vm_flags |= VM_LOCKED;
+ pra->vm_flags |=
+ (vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
return SWAP_FAIL; /* To break the loop */
}
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v7 2/6] mm: mlock: Add new mlock system call
From: Eric B Munson @ 2015-08-09 5:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Michal Hocko, Vlastimil Babka, Heiko Carstens,
Geert Uytterhoeven, Catalin Marinas, Stephen Rothwell,
Guenter Roeck, Andrea Arcangeli, linux-alpha, linux-kernel,
linux-arm-kernel, adi-buildroot-devel, linux-cris-kernel,
linux-ia64, linux-m68k, linux-am33-list, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-xtensa,
linux-api, linux-arch, linux-mm
In-Reply-To: <1439097776-27695-1-git-send-email-emunson@akamai.com>
With the refactored mlock code, introduce a new system call for mlock.
The new call will allow the user to specify what lock states are being
added. mlock2 is trivial at the moment, but a follow on patch will add
a new mlock state making it useful.
Signed-off-by: Eric B Munson <emunson@akamai.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: adi-buildroot-devel@lists.sourceforge.net
Cc: linux-cris-kernel@axis.com
Cc: linux-ia64@vger.kernel.org
Cc: linux-m68k@lists.linux-m68k.org
Cc: linux-am33-list@redhat.com
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-api@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
include/linux/syscalls.h | 2 ++
include/uapi/asm-generic/unistd.h | 4 +++-
kernel/sys_ni.c | 1 +
mm/mlock.c | 8 ++++++++
6 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index ef8187f..8e06da6 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -365,3 +365,4 @@
356 i386 memfd_create sys_memfd_create
357 i386 bpf sys_bpf
358 i386 execveat sys_execveat stub32_execveat
+360 i386 mlock2 sys_mlock2
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 9ef32d5..67601e7 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -329,6 +329,7 @@
320 common kexec_file_load sys_kexec_file_load
321 common bpf sys_bpf
322 64 execveat stub_execveat
+324 common mlock2 sys_mlock2
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index b45c45b..56a3d59 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -884,4 +884,6 @@ asmlinkage long sys_execveat(int dfd, const char __user *filename,
const char __user *const __user *argv,
const char __user *const __user *envp, int flags);
+asmlinkage long sys_mlock2(unsigned long start, size_t len, int flags);
+
#endif
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index e016bd9..14a6013 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -709,9 +709,11 @@ __SYSCALL(__NR_memfd_create, sys_memfd_create)
__SYSCALL(__NR_bpf, sys_bpf)
#define __NR_execveat 281
__SC_COMP(__NR_execveat, sys_execveat, compat_sys_execveat)
+#define __NR_mlock2 282
+__SYSCALL(__NR_mlock2, sys_mlock2)
#undef __NR_syscalls
-#define __NR_syscalls 282
+#define __NR_syscalls 283
/*
* All syscalls below here should go away really,
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 7995ef5..4818b71 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -193,6 +193,7 @@ cond_syscall(sys_mlock);
cond_syscall(sys_munlock);
cond_syscall(sys_mlockall);
cond_syscall(sys_munlockall);
+cond_syscall(sys_mlock2);
cond_syscall(sys_mincore);
cond_syscall(sys_madvise);
cond_syscall(sys_mremap);
diff --git a/mm/mlock.c b/mm/mlock.c
index 5692ee5..3094f27 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -643,6 +643,14 @@ SYSCALL_DEFINE2(mlock, unsigned long, start, size_t, len)
return do_mlock(start, len, VM_LOCKED);
}
+SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
+{
+ if (flags)
+ return -EINVAL;
+
+ return do_mlock(start, len, VM_LOCKED);
+}
+
SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
{
int ret;
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [PATCH v7 0/6] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-08-09 5:22 UTC (permalink / raw)
To: Andrew Morton
Cc: Eric B Munson, Shuah Khan, Michal Hocko, Michael Kerrisk,
Vlastimil Babka, Jonathan Corbet, Ralf Baechle, Andrea Arcangeli,
linux-alpha, linux-kernel, linux-mips, linux-parisc, linuxppc-dev,
sparclinux, linux-xtensa, linux-mm, linux-arch, linux-api
mlock() allows a user to control page out of program memory, but this
comes at the cost of faulting in the entire mapping when it is
allocated. For large mappings where the entire area is not necessary
this is not ideal. Instead of forcing all locked pages to be present
when they are allocated, this set creates a middle ground. Pages are
marked to be placed on the unevictable LRU (locked) when they are first
used, but they are not faulted in by the mlock call.
This series introduces a new mlock() system call that takes a flags
argument along with the start address and size. This flags argument
gives the caller the ability to request memory be locked in the
traditional way, or to be locked after the page is faulted in. A new
MCL flag is added to mirror the lock on fault behavior from mlock() in
mlockall().
There are two main use cases that this set covers. The first is the
security focussed mlock case. A buffer is needed that cannot be written
to swap. The maximum size is known, but on average the memory used is
significantly less than this maximum. With lock on fault, the buffer
is guaranteed to never be paged out without consuming the maximum size
every time such a buffer is created.
The second use case is focussed on performance. Portions of a large
file are needed and we want to keep the used portions in memory once
accessed. This is the case for large graphical models where the path
through the graph is not known until run time. The entire graph is
unlikely to be used in a given invocation, but once a node has been
used it needs to stay resident for further processing. Given these
constraints we have a number of options. We can potentially waste a
large amount of memory by mlocking the entire region (this can also
cause a significant stall at startup as the entire file is read in).
We can mlock every page as we access them without tracking if the page
is already resident but this introduces large overhead for each access.
The third option is mapping the entire region with PROT_NONE and using
a signal handler for SIGSEGV to mprotect(PROT_READ) and mlock() the
needed page. Doing this page at a time adds a significant performance
penalty. Batching can be used to mitigate this overhead, but in order
to safely avoid trying to mprotect pages outside of the mapping, the
boundaries of each mapping to be used in this way must be tracked and
available to the signal handler. This is precisely what the mm system
in the kernel should already be doing.
For mlock(MLOCK_ONFAULT) the user is charged against RLIMIT_MEMLOCK as
if mlock(MLOCK_LOCKED) or mmap(MAP_LOCKED) was used, so when the VMA is
created not when the pages are faulted in. For mlockall(MCL_ONFAULT)
the user is charged as if MCL_FUTURE was used. This decision was made
to keep the accounting checks out of the page fault path.
To illustrate the benefit of this set I wrote a test program that mmaps
a 5 GB file filled with random data and then makes 15,000,000 accesses
to random addresses in that mapping. The test program was run 20 times
for each setup. Results are reported for two program portions, setup
and execution. The setup phase is calling mmap and optionally mlock on
the entire region. For most experiments this is trivial, but it
highlights the cost of faulting in the entire region. Results are
averages across the 20 runs in milliseconds.
mmap with mlock(MLOCK_LOCKED) on entire range:
Setup avg: 8228.666
Processing avg: 8274.257
mmap with mlock(MLOCK_LOCKED) before each access:
Setup avg: 0.113
Processing avg: 90993.552
mmap with PROT_NONE and signal handler and batch size of 1 page:
With the default value in max_map_count, this gets ENOMEM as I attempt
to change the permissions, after upping the sysctl significantly I get:
Setup avg: 0.058
Processing avg: 69488.073
mmap with PROT_NONE and signal handler and batch size of 8 pages:
Setup avg: 0.068
Processing avg: 38204.116
mmap with PROT_NONE and signal handler and batch size of 16 pages:
Setup avg: 0.044
Processing avg: 29671.180
mmap with mlock(MLOCK_ONFAULT) on entire range:
Setup avg: 0.189
Processing avg: 17904.899
The signal handler in the batch cases faulted in memory in two steps to
avoid having to know the start and end of the faulting mapping. The
first step covers the page that caused the fault as we know that it will
be possible to lock. The second step speculatively tries to mlock and
mprotect the batch size - 1 pages that follow. There may be a clever
way to avoid this without having the program track each mapping to be
covered by this handeler in a globally accessible structure, but I could
not find it. It should be noted that with a large enough batch size
this two step fault handler can still cause the program to crash if it
reaches far beyond the end of the mapping.
These results show that if the developer knows that a majority of the
mapping will be used, it is better to try and fault it in at once,
otherwise mlock(MLOCK_ONFAULT) is significantly faster.
The performance cost of these patches are minimal on the two benchmarks
I have tested (stream and kernbench). The following are the average
values across 20 runs of stream and 10 runs of kernbench after a warmup
run whose results were discarded.
Avg throughput in MB/s from stream using 1000000 element arrays
Test 4.2-rc1 4.2-rc1+lock-on-fault
Copy: 10,566.5 10,421
Scale: 10,685 10,503.5
Add: 12,044.1 11,814.2
Triad: 12,064.8 11,846.3
Kernbench optimal load
4.2-rc1 4.2-rc1+lock-on-fault
Elapsed Time 78.453 78.991
User Time 64.2395 65.2355
System Time 9.7335 9.7085
Context Switches 22211.5 22412.1
Sleeps 14965.3 14956.1
---
Changes from V6:
* Bump the x86 system call number to avoid collision with userfaultfd
* Fix FOLL_POPULATE and FOLL_MLOCK usage when mmap is called with
MAP_POPULATE
* Add documentation for the proc smaps change
* checkpatch fixes
Changes from V5:
Drop MLOCK_LOCKED flag
* MLOCK_ONFAULT and MCL_ONFAULT are treated as a modifier to other locking
operations, mirroring the relationship between VM_LOCKED and
VM_LOCKONFAULT
* Drop mmap flag and related tests
* Fix clearing of MCL_CURRENT when mlockall is called with MCL_FUTURE,
mlockall behavoir now matches the old behavior WRT to ordering
Changes from V4:
Drop all architectures for new sys call entries except x86[_64] and MIPS
Drop munlock2 and munlockall2
Make VM_LOCKONFAULT a modifier to VM_LOCKED only to simplify book keeping
Adjust tests to match
Changes from V3:
Ensure that pages present when mlock2(MLOCK_ONFAULT) is called are locked
Ensure that VM_LOCKONFAULT is handled in cases that used to only check VM_LOCKED
Add tests for new system calls
Add missing syscall entries, fix NR_syscalls on multiple arch's
Add missing MAP_LOCKONFAULT for tile
Changes from V2:
Added new system calls for mlock, munlock, and munlockall with added
flags arguments for controlling how memory is locked or unlocked.
Eric B Munson (6):
mm: mlock: Refactor mlock, munlock, and munlockall code
mm: mlock: Add new mlock system call
mm: Introduce VM_LOCKONFAULT
mm: mlock: Add mlock flags to enable VM_LOCKONFAULT usage
selftests: vm: Add tests for lock on fault
mips: Add entry for new mlock2 syscall
Documentation/filesystems/proc.txt | 1 +
arch/alpha/include/uapi/asm/mman.h | 3 +
arch/mips/include/uapi/asm/mman.h | 6 +
arch/mips/include/uapi/asm/unistd.h | 15 +-
arch/mips/kernel/scall32-o32.S | 1 +
arch/mips/kernel/scall64-64.S | 1 +
arch/mips/kernel/scall64-n32.S | 1 +
arch/mips/kernel/scall64-o32.S | 1 +
arch/parisc/include/uapi/asm/mman.h | 3 +
arch/powerpc/include/uapi/asm/mman.h | 1 +
arch/sparc/include/uapi/asm/mman.h | 1 +
arch/tile/include/uapi/asm/mman.h | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/include/uapi/asm/mman.h | 6 +
drivers/gpu/drm/drm_vm.c | 8 +-
fs/proc/task_mmu.c | 1 +
include/linux/mm.h | 2 +
include/linux/syscalls.h | 2 +
include/uapi/asm-generic/mman-common.h | 5 +
include/uapi/asm-generic/mman.h | 1 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/fork.c | 2 +-
kernel/sys_ni.c | 1 +
mm/debug.c | 1 +
mm/gup.c | 10 +-
mm/huge_memory.c | 2 +-
mm/hugetlb.c | 4 +-
mm/mlock.c | 87 +++-
mm/mmap.c | 2 +-
mm/rmap.c | 6 +-
tools/testing/selftests/vm/Makefile | 2 +
tools/testing/selftests/vm/mlock2-tests.c | 661 ++++++++++++++++++++++++++++
tools/testing/selftests/vm/on-fault-limit.c | 47 ++
tools/testing/selftests/vm/run_vmtests | 22 +
35 files changed, 872 insertions(+), 41 deletions(-)
create mode 100644 tools/testing/selftests/vm/mlock2-tests.c
create mode 100644 tools/testing/selftests/vm/on-fault-limit.c
Cc: Shuah Khan <shuahkh@osg.samsung.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: linux-alpha@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-mips@linux-mips.org
Cc: linux-parisc@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: sparclinux@vger.kernel.org
Cc: linux-xtensa@linux-xtensa.org
Cc: linux-mm@kvack.org
Cc: linux-arch@vger.kernel.org
Cc: linux-api@vger.kernel.org
--
1.9.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v2] iio: Documentation: Add trigger name attribute ABI documentation
From: Jonathan Cameron @ 2015-08-08 11:42 UTC (permalink / raw)
To: Daniel Baluta, Cristina Opriceana
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAEnQRZDH3Gu_FSiSrceC3nK4SOwKj+6WCzxdc9iU9DMsZmmrTA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 06/08/15 13:00, Daniel Baluta wrote:
> On Thu, Aug 6, 2015 at 2:17 PM, Cristina Opriceana
> <cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> This patch adds an entry in ABI Documentation for the name attribute
>> issued when a trigger is created.
>>
>> Signed-off-by: Cristina Opriceana <cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Acked-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Applied.
Thanks,
>
>> ---
>> Changes in v2:
>> - deviceX to deviceY, to avoid confusion
>> - clearer description
>>
>> Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
>> index 5235e6c..0c1e843 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
>> @@ -9,3 +9,12 @@ Description:
>> automated testing or in situations, where other trigger methods
>> are not applicable. For example no RTC or spare GPIOs.
>> X is the IIO index of the trigger.
>> +
>> +What: /sys/bus/iio/devices/triggerX/name
>> +KernelVersion: 2.6.39
>> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> +Description:
>> + The name attribute holds a description string for the current
>> + trigger. In order to associate the trigger with an IIO device
>> + one should write this name string to
>> + /sys/bus/iio/devices/iio:deviceY/trigger/current_trigger.
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Michael Kerrisk (man-pages) @ 2015-08-08 6:57 UTC (permalink / raw)
To: Darren Hart
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Thomas Gleixner,
Torvald Riegel, Carlos O'Donell, Ingo Molnar, Jakub Jelinek,
linux-man, lkml, Davidlohr Bueso, Arnd Bergmann, Steven Rostedt,
Peter Zijlstra, Linux API, Roland McGrath, Anton Blanchard,
Eric Dumazet, bill o gallmeister, Jan Kiszka, Daniel Wagner,
Rich Felker, Andy Lutomirski, bert hubert, Rusty Russell,
Heinrich Schuchardt
In-Reply-To: <20150805222140.GA74817@vmdeb7>
Hi Darren,
Some of my comments below will refer to the reply I just sent
to tglx (and the list) a few minutes ago.
On 08/06/2015 12:21 AM, Darren Hart wrote:
> On Mon, Jul 27, 2015 at 02:07:15PM +0200, Michael Kerrisk (man-pages) wrote:
>> Hello all,
>>
>
> Michael, thank you for your diligence in following up and collecting
> reviews. I've attempted to respond to what I was specifically called out
> in or where I had something specific to add in addition to other
> replies.
Thanks!
> After this, will you send another version (numbered for reference
> maybe?) with any remaining FIXMEs that haven't yet been addressed
> according to your accounting?
Yes, I'll be sending out another draft (probably after a short delay,
while I see what further responses come back on the mails I just sent.)
In any case, the latest version of the page can be found at
http://git.kernel.org/cgit/docs/man-pages/man-pages.git/log/?h=draft_futex
>> Priority-inheritance futexes
>> Linux supports priority-inheritance (PI) futexes in order to han‐
>> dle priority-inversion problems that can be encountered with nor‐
>> mal futex locks. Priority inversion is the problem that occurs
>> when a high-priority task is blocked waiting to acquire a lock
>> held by a low-priority task, while tasks at an intermediate pri‐
>> ority continuously preempt the low-priority task from the CPU.
>> Consequently, the low-priority task makes no progress toward
>> releasing the lock, and the high-priority task remains blocked.
>>
>> Priority inheritance is a mechanism for dealing with the prior‐
>> ity-inversion problem. With this mechanism, when a high-priority
>> task becomes blocked by a lock held by a low-priority task, the
>> latter's priority is temporarily raised to that of the former, so
>> that it is not preempted by any intermediate level tasks, and can
>> thus make progress toward releasing the lock. To be effective,
>> priority inheritance must be transitive, meaning that if a high-
>> priority task blocks on a lock held by a lower-priority task that
>> is itself blocked by lock held by another intermediate-priority
>> task (and so on, for chains of arbitrary length), then both of
>> those task (or more generally, all of the tasks in a lock chain)
>> have their priorities raised to be the same as the high-priority
>> task.
>>
>> .\" FIXME XXX The following is my attempt at a definition of PI futexes,
>> .\" based on mail discussions with Darren Hart. Does it seem okay?
>>
>> From a user-space perspective, what makes a futex PI-aware is a
>> policy agreement between user space and the kernel about the
>> value of the futex word (described in a moment), coupled with the
>> use of the PI futex operations described below (in particular,
>> FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and FUTEX_CMP_REQUEUE_PI).
>
> Yes. Was this intended to be a complete opcode list?
No. I'll remove that list, in case its misunderstood that way.
> PI operations must
> use paired operations.
>
> (FUTEX_LOCK_PI | FUTEX_TRYLOCK_PI) : FUTEX_UNLOCK_PI
> FUTEX_WAIT_REQUEUE_PI : FUTEX_CMP_REQUEUE_PI
And now I've made that point explicitly in the page. See my comment
lower down.
> And their PRIVATE counterparts of course (which is assumed as it is a
> flag to the opcode).
Yes. But I don't think that needs to be called out explicitly here (?).
>> .\" FIXME XXX ===== Start of adapted Hart/Guniguntala text =====
>> .\" The following text is drawn from the Hart/Guniguntala paper
>> .\" (listed in SEE ALSO), but I have reworded some pieces
>> .\" significantly. Please check it.
>>
>> The PI futex operations described below differ from the other
>> futex operations in that they impose policy on the use of the
>> value of the futex word:
>>
>> * If the lock is not acquired, the futex word's value shall be
>> 0.
>>
>> * If the lock is acquired, the futex word's value shall be the
>> thread ID (TID; see gettid(2)) of the owning thread.
>>
>> * If the lock is owned and there are threads contending for the
>> lock, then the FUTEX_WAITERS bit shall be set in the futex
>> word's value; in other words, this value is:
>>
>> FUTEX_WAITERS | TID
>>
>>
>> Note that a PI futex word never just has the value FUTEX_WAITERS,
>> which is a permissible state for non-PI futexes.
>
> The second clause is inappropriate. I don't know if that was yours or
> mine, but non-PI futexes do not have a kernel defined value policy, so
> ==FUTEX_WAITERS cannot be a "permissible state" as any value is
> permissible for non-PI futexes, and none have a kernel defined state.
>
> Perhaps include a Note under the third bullet as:
>
> Note: It is invalid for a PI futex word to have no owner and
> FUTEX_WAITERS set.
Done.
>> With this policy in place, a user-space application can acquire a
>> not-acquired lock or release a lock that no other threads try to
>
> "that no other threads try to acquire" seems out of place. I think
> "atomic instructions" is sufficient to express how contention is
> handled.
Yup, changed.
>> acquire using atomic instructions executed in user space (e.g., a
>> compare-and-swap operation such as cmpxchg on the x86 architec‐
>> ture). Acquiring a lock simply consists of using compare-and-
>> swap to atomically set the futex word's value to the caller's TID
>> if its previous value was 0. Releasing a lock requires using
>> compare-and-swap to set the futex word's value to 0 if the previ‐
>> ous value was the expected TID.
>>
>> If a futex is already acquired (i.e., has a nonzero value), wait‐
>> ers must employ the FUTEX_LOCK_PI operation to acquire the lock.
>> If other threads are waiting for the lock, then the FUTEX_WAITERS
>> bit is set in the futex value; in this case, the lock owner must
>> employ the FUTEX_UNLOCK_PI operation to release the lock.
>>
>> In the cases where callers are forced into the kernel (i.e.,
>> required to perform a futex() call), they then deal directly with
>> a so-called RT-mutex, a kernel locking mechanism which implements
>> the required priority-inheritance semantics. After the RT-mutex
>> is acquired, the futex value is updated accordingly, before the
>> calling thread returns to user space.
>
> This last paragraph relies on kernel implementation rather than
> behavior. If the RT-mutex is renamed or the mechanism is implemented
> differently in futexes, this section will require updating. Is that
> appropriate for a user-space man page?
In the end, I'm not sure. This is (so far) my best attempt at trying
to convey an explanation of the behavior provided by the API.
>> .\" FIXME ===== End of adapted Hart/Guniguntala text =====
>>
>>
>>
>> .\" FIXME We need some explanation in the following paragraph of *why*
>> .\" it is important to note that "the kernel will update the
>> .\" futex word's value prior
>> It is important to note to returning to user space" . Can someone
>> explain? that the kernel will update the futex word's value
>> prior to returning to user space. Unlike the other futex opera‐
>> tions described above, the PI futex operations are designed for
>> the implementation of very specific IPC mechanisms.
>
> If the kernel didn't perform the update prior to returning to userspace,
> we could end up in an invalid state. Such as having an owner, but the
> value being 0. Or having waiters, but not having FUTEX_WAITERS set.
So I've now reworked this passage to read:
It is important to note that the kernel will update the futex
word's value prior to returning to user space. (This prevents
the possibility of the futex word's value ending up in an invalid
state, such as having an owner but the value being 0, or having
waiters but not having the FUTEX_WAITERS bit set.)
Okay?
>> .\"
>> .\" FIXME XXX In discussing errors for FUTEX_CMP_REQUEUE_PI, Darren Hart
>> .\" made the observation that "EINVAL is returned if the non-pi
>> .\" to pi or op pairing semantics are violated."
>> .\" Probably there needs to be a general statement about this
>> .\" requirement, probably located at about this point in the page.
>> .\" Darren (or someone else), care to take a shot at this?
>
> We can probably borrow from either the futex.c comments or the
> futex-requeue-pi.txt in Documentation. Also, it is important to note
> that the PI requeue operations require two distinct uadders (although
> that is implied by requiring "non-pi to pi" as a futex cannot be both.
>
> Or... perhaps something like:
>
> Due to the kernel imposed futex word value policy, PI futex
> operations have additional usage requirements:
>
> FUTEX_WAIT_REQUEUE_PI must be paired with FUTEX_CMP_REQUEUE_PI
> and be performed from a non-pi futex to a distinct pi futex.
> Failing to do so will return EINVAL.
For which operation does the EINVAL occur: FUTEX_WAIT_REQUEUE_PI or
FUTEX_CMP_REQUEUE_PI?
> Additionally,
> FUTEX_CMP_REQUEUE_PI requires that nr_wake=1. [We state in the
> docs that nr_requeue should be INT_MAX for broadcast and 0 for
> signal... but that may be overly specific to libc for this
> manual]
>
> Similarly, FUTEX_UNLOCK_PI must only be called on a futex owned
> by the calling thread as defined by the value policy, otherwise
> EPERM is returned.
>
> Were you looking for something like that - or were you looking for
> justification for these requirements?
That's good. I've reworked a piece of the page to be:
PI futexes are operated on by specifying one of the values listed
below in futex_op. Note that the PI futex operations must be
used as paired operations and are subject to some additional
requirements:
* FUTEX_LOCK_PI and FUTEX_TRYLOCK_PI pair with FUTEX_UNLOCK_PI.
FUTEX_UNLOCK_PI must be called only on a futex owned by the
calling thread, as defined by the value policy, otherwise the
error EPERM results.
* FUTEX_WAIT_REQUEUE_PI pairs with FUTEX_CMP_REQUEUE_PI. This
must be performed from a non-PI futex to a distinct PI futex
(or the error EINVAL results). Additionally, val (the number
of waiters to be woken) must be 1 (or the error EINVAL
results).
But see my question just above. I'll tweak the first bullet point a
little after I hear back from you.
> ...
>
>> FUTEX_LOCK_PI (since Linux 2.6.18)
>> .\" FIXME I did some significant rewording of tglx's text to create
>> .\" the text below.
>> .\" Please check the following paragraph, in case I injected
>> .\" errors.
>> This operation is used after after an attempt to acquire
>> the lock via an atomic user-space instruction failed
>> because the futex word has a nonzero value—specifically,
>> because it contained the namespace-specific TID of the
>> lock owner.
>
> Acked.
Thanks.
>> FUTEX_TRYLOCK_PI (since Linux 2.6.18)
>> .\" FIXME I think it would be helpful here to say a few more words about
>> .\" the difference(s) between FUTEX_LOCK_PI and FUTEX_TRYLOCK_PI.
>> .\" Can someone propose something?
>> This operation tries to acquire the futex at uaddr. It
>> deals with the situation where the TID value at uaddr is
>> 0, but the FUTEX_WAITERS bit is set. User space cannot
>> handle this condition in a race-free manner
>> .\" FIXME How does the situation in the previous sentence come about?
>> .\" Probably it would be helpful to say something about that in
>> .\" the man page.
>> .\" FIXME And *how* does FUTEX_TRYLOCK_PI deal with this situation?
>
> I guess I wouldn't expect to see this detail in the manual. That state
> should never exist in userspace as far as I understand it, which makes
> it a kernel implementation detail and not relevant to a usage manual.
See my recent reply to Thomas Gleixner.
>>
>> FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
>> This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
>> It requeues waiters that are blocked via
>> FUTEX_WAIT_REQUEUE_PI on uaddr from a non-PI source futex
>> (uaddr) to a PI target futex (uaddr2).
>>
>> As with FUTEX_CMP_REQUEUE, this operation wakes up a maxi‐
>> mum of val waiters that are waiting on the futex at uaddr.
>> However, for FUTEX_CMP_REQUEUE_PI, val is required to be 1
>> (since the main point is to avoid a thundering herd). The
>> remaining waiters are removed from the wait queue of the
>> source futex at uaddr and added to the wait queue of the
>> target futex at uaddr2.
>>
>> The val2 and val3 arguments serve the same purposes as for
>> FUTEX_CMP_REQUEUE.
>> .\" FIXME The page at http://locklessinc.com/articles/futex_cheat_sheet/
>> .\" notes that "priority-inheritance Futex to priority-inheritance
>> .\" Futex requeues are currently unsupported". Do we need to say
>> .\" something in the man page about that?
>>
>
> I noted this above in response to your request for detail about the
> *REQUEUE_PI semantics and error codes. Was that sufficient?
Between input from you and tglx, I think we're okay.
>> FUTEX_WAIT_REQUEUE_PI (since Linux 2.6.31)
>>
>> .\" FIXME I find the next sentence (from tglx) pretty hard to grok.
>> .\" Could someone explain it a bit more?
>>
>> Wait operation to wait on a non-PI futex at uaddr and
>> potentially be requeued onto a PI futex at uaddr2. The
>> wait operation on uaddr is the same as FUTEX_WAIT.
>
> The point tglx is making here is that you must know ahead of time and
> tell the kernel that you intend to use this futex in a REQUEUE_PI
> operation, and not a regular REQUEUE. This is determined by the both the
> op codes as well as the required arguments, which I also documented
> above. Is more detail required?
I think I've got it now. See my revised version of this text in my reply
to tglx, and let me know if anything is amiss.
>> .\" FIXME I'm not quite clear on the meaning of the following sentence.
>> .\" Is this trying to say that while blocked in a
>> .\" FUTEX_WAIT_REQUEUE_PI, it could happen that another
>> .\" task does a FUTEX_WAKE on uaddr that simply causes
>> .\" a normal wake, with the result that the FUTEX_WAIT_REQUEUE_PI
>> .\" does not complete? What happens then to the FUTEX_WAIT_REQUEUE_PI
>> .\" opertion? Does it remain blocked, or does it unblock
>> .\" In which case, what does user space see?
>>
>> The
>> waiter can be removed from the wait on uaddr via
>> FUTEX_WAKE without requeueing on uaddr2.
>
> Userspace should see the task wake and continue executing. This would
> effectively be a cancelation operation - which I didn't think was
> supported. Thomas?
Thomas responded on this point, and I revised my text. Please see
my reply to tglx and let me know if anything is amiss.
>> .\" FIXME XXX The following is a reworded version of Darren Hart's text.
>> .\" Please check that I did not introduce any errors.
>> EINVAL (FUTEX_CMP_REQUEUE_PI) An attempt was made to requeue a
>> waiter to a futex other than that specified by the match‐
>> ing FUTEX_WAIT_REQUEUE_PI call for that waiter.
>
> Acked.
Thanks!
Thanks for the help, Darren!
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Michael Kerrisk (man-pages) @ 2015-08-08 6:56 UTC (permalink / raw)
To: Thomas Gleixner, Peter Zijlstra
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Darren Hart, Torvald Riegel,
Carlos O'Donell, Ingo Molnar, Jakub Jelinek, linux-man, lkml,
Davidlohr Bueso, Arnd Bergmann, Steven Rostedt, Linux API,
Roland McGrath, Anton Blanchard, Eric Dumazet, bill o gallmeister,
Jan Kiszka, Daniel Wagner, Rich Felker, Andy Lutomirski,
bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <alpine.DEB.2.11.1507282303290.3825@nanos>
On 07/28/2015 11:03 PM, Thomas Gleixner wrote:
> On Tue, 28 Jul 2015, Peter Zijlstra wrote:
>
>> On Tue, Jul 28, 2015 at 10:23:51PM +0200, Thomas Gleixner wrote:
>>
>>>> FUTEX_WAKE (since Linux 2.6.0)
>>>> This operation wakes at most val of the waiters that are
>>>> waiting (e.g., inside FUTEX_WAIT) on the futex word at the
>>>> address uaddr. Most commonly, val is specified as either
>>>> 1 (wake up a single waiter) or INT_MAX (wake up all wait‐
>>>> ers). No guarantee is provided about which waiters are
>>>> awoken (e.g., a waiter with a higher scheduling priority
>>>> is not guaranteed to be awoken in preference to a waiter
>>>> with a lower priority).
>>>
>>> That's only correct up to Linux 2.6.21.
>>>
>>> Since 2.6.22 we have a priority ordered wakeup. For SCHED_OTHER
>>> threads this takes the nice level into account. Threads with the same
>>> priority are woken in FIFO order.
>>
>> Maybe don't mention the effects of SCHED_OTHER, order by nice value is
>> 'wrong'.
>
> Indeed.
>
>> Also, this code seems to use plist, which means it won't do the right
>> thing for SCHED_DEADLINE either.
>>
>> Do we want to go fix that?
>
> I think so.
So, no change to this piece of text then?
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Michael Kerrisk (man-pages) @ 2015-08-08 6:53 UTC (permalink / raw)
To: Thomas Gleixner
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w, Darren Hart, Torvald Riegel,
Carlos O'Donell, Ingo Molnar, Jakub Jelinek, linux-man, lkml,
Davidlohr Bueso, Arnd Bergmann, Steven Rostedt, Peter Zijlstra,
Linux API, Roland McGrath, Anton Blanchard, Eric Dumazet,
bill o gallmeister, Jan Kiszka, Daniel Wagner, Rich Felker,
Andy Lutomirski, bert hubert, Rusty Russell, Heinrich Schuchardt
In-Reply-To: <alpine.DEB.2.11.1507282054370.3825@nanos>
Hi Thomas,
Thank you for the comments below. This helps hugely:
more than 30 of my FIXMEs have now gone away!
I have a few open questions, which you can find
by searching for the string "???". If you would have
a chance to look at those, I'd appreciate it.
On 07/28/2015 10:23 PM, Thomas Gleixner wrote:
> On Mon, 27 Jul 2015, Michael Kerrisk (man-pages) wrote:
>> FUTEX_CLOCK_REALTIME (since Linux 2.6.28)
>> This option bit can be employed only with the
>> FUTEX_WAIT_BITSET and FUTEX_WAIT_REQUEUE_PI operations.
>>
>> If this option is set, the kernel treats timeout as an
>> absolute time based on CLOCK_REALTIME.
>>
>> .\" FIXME XXX I added CLOCK_MONOTONIC below. Okay?
>> If this option is not set, the kernel treats timeout as
>> relative time, measured against the CLOCK_MONOTONIC clock.
>
> That's correct.
Thanks.
>> The operation specified in futex_op is one of the following:
>>
>> FUTEX_WAIT (since Linux 2.6.0)
>> This operation tests that the value at the futex word
>> pointed to by the address uaddr still contains the
>> expected value val, and if so, then sleeps awaiting
>> FUTEX_WAKE on the futex word. The load of the value of
>> the futex word is an atomic memory access (i.e., using
>> atomic machine instructions of the respective architec‐
>> ture). This load, the comparison with the expected value,
>> and starting to sleep are performed atomically and totally
>> ordered with respect to other futex operations on the same
>> futex word. If the thread starts to sleep, it is consid‐
>> ered a waiter on this futex word. If the futex value does
>> not match val, then the call fails immediately with the
>> error EAGAIN.
>>
>> The purpose of the comparison with the expected value is
>> to prevent lost wake-ups: If another thread changed the
>> value of the futex word after the calling thread decided
>> to block based on the prior value, and if the other thread
>> executed a FUTEX_WAKE operation (or similar wake-up) after
>> the value change and before this FUTEX_WAIT operation,
>> then the latter will observe the value change and will not
>> start to sleep.
>>
>> If the timeout argument is non-NULL, its contents specify
>> a relative timeout for the wait, measured according to the
>> .\" FIXME XXX I added CLOCK_MONOTONIC below. Okay?
>
> Yes.
Thanks.
>
>> CLOCK_MONOTONIC clock. (This interval will be rounded up
>> to the system clock granularity, and kernel scheduling
>> delays mean that the blocking interval may overrun by a
>> small amount.)
>
> The given wait time will be rounded up to the system
> clock granularity and is guaranteed not to expire
> early.
>
> There are a gazillion reasons why it can expire late, but the
> guarantee is that it never expires prematurely.
>
>> If timeout is NULL, the call blocks indef‐
>> initely.
>
> Right.
Thanks. Reworded as you suggest.
>> The arguments uaddr2 and val3 are ignored.
>>
>>
>> FUTEX_WAKE (since Linux 2.6.0)
>> This operation wakes at most val of the waiters that are
>> waiting (e.g., inside FUTEX_WAIT) on the futex word at the
>> address uaddr. Most commonly, val is specified as either
>> 1 (wake up a single waiter) or INT_MAX (wake up all wait‐
>> ers). No guarantee is provided about which waiters are
>> awoken (e.g., a waiter with a higher scheduling priority
>> is not guaranteed to be awoken in preference to a waiter
>> with a lower priority).
>
> That's only correct up to Linux 2.6.21.
>
> Since 2.6.22 we have a priority ordered wakeup. For SCHED_OTHER
> threads this takes the nice level into account. Threads with the same
> priority are woken in FIFO order.
So, this got picked up in a little subthread by Peter Zijsltra. I'll
reply there.
>> The arguments timeout, uaddr2, and val3 are ignored.
>
>>
>> FUTEX_FD (from Linux 2.6.0 up to and including Linux 2.6.25)
>> This operation creates a file descriptor that is associ‐
>> ated with the futex at uaddr. The caller must close the
>> returned file descriptor after use. When another process
>> or thread performs a FUTEX_WAKE on the futex word, the
>> file descriptor indicates as being readable with
>> select(2), poll(2), and epoll(7)
>>
>> The file descriptor can be used to obtain asynchronous
>> notifications: if val is nonzero, then when another
>> process or thread executes a FUTEX_WAKE, the caller will
>> receive the signal number that was passed in val.
>>
>> The arguments timeout, uaddr2 and val3 are ignored.
>>
>> .\" FIXME(Torvald) We never define "upped". Maybe just remove the
>> .\" following sentence?
>> To prevent race conditions, the caller should test if the
>> futex has been upped after FUTEX_FD returns.
>
> Yes, just remove it.
Done.
>> Because it was inherently racy, FUTEX_FD has been removed
>> from Linux 2.6.26 onward.
>>
>> FUTEX_REQUEUE (since Linux 2.6.0)
>> .\" FIXME(Torvald) Is there some indication that FUTEX_REQUEUE is broken
>> .\" in general, or is this comment implicitly speaking about the
>> .\" condvar (?) use case? If the latter we might want to weaken the
>> .\" advice below a little.
>> .\" [Anyone else have input on this?]
>
> The condvar use case exposes the flaw nicely, but that's pretty much
> true for everything which wants a sane requeue operation.
Yep. I dealt with this in an earlier response to mail from Darren (where
you also replied). I've removed the warning that FUTEX_REQUEUE is broken.
>> Avoid using this operation. It is broken for its intended
>> purpose. Use FUTEX_CMP_REQUEUE instead.
>>
>> This operation performs the same task as
>> FUTEX_CMP_REQUEUE, except that no check is made using the
>> value in val3. (The argument val3 is ignored.)
>>
>> FUTEX_CMP_REQUEUE (since Linux 2.6.7)
>> This operation first checks whether the location uaddr
>> still contains the value val3. If not, the operation
>> fails with the error EAGAIN. Otherwise, the operation
>> wakes up a maximum of val waiters that are waiting on the
>> futex at uaddr. If there are more than val waiters, then
>> the remaining waiters are removed from the wait queue of
>> the source futex at uaddr and added to the wait queue of
>> the target futex at uaddr2. The val2 argument specifies
>> an upper limit on the number of waiters that are requeued
>> to the futex at uaddr2.
>>
>> .\" FIXME(Torvald) Is the following correct? Or is just the decision
>> .\" which threads to wake or requeue part of the atomic operation?
>>
>> The load from uaddr is an atomic memory access (i.e.,
>> using atomic machine instructions of the respective archi‐
>> tecture). This load, the comparison with val3, and the
>> requeueing of any waiters are performed atomically and
>> totally ordered with respect to other operations on the
>> same futex word.
>
> It's atomic as the other atomic operations on the futex word. It's
> always performed with the proper lock(s) held in the kernel. That
> means any concurrent operation will serialize on that lock(s). User
> space has to make sure, that depending on the observed value no
> concurrent operations happen, but that's something the kernel cannot
> control.
???
Sorry, I'm not clear here. Is the current text correct then? Or is some
change needed.
>> This operation was added as a replacement for the earlier
>> FUTEX_REQUEUE. The difference is that the check of the
>> value at uaddr can be used to ensure that requeueing hap‐
>> pens only under certain conditions. Both operations can
>> be used to avoid a "thundering herd" effect when
>> FUTEX_WAKE is used and all of the waiters that are woken
>> need to acquire another futex.
>>
>> .\" FIXME Please review the following new paragraph to see if it is
>> .\" accurate.
>> Typical values to specify for val are 0 or or 1. (Speci‐
>> fying INT_MAX is not useful, because it would make the
>> FUTEX_CMP_REQUEUE operation equivalent to FUTEX_WAKE.)
>> The limit value specified via val2 is typically either 1
>> or INT_MAX. (Specifying the argument as 0 is not useful,
>> because it would make the FUTEX_CMP_REQUEUE operation
>> equivalent to FUTEX_WAIT.)
>
> It's correct.
Thanks.
>> .\" FIXME Here, it would be helpful to have an example of how
>> .\" FUTEX_CMP_REQUEUE might be used, at the same time illustrating
>> .\" why FUTEX_WAKE is unsuitable for the same use case.
>
> Waiters:
>
> lock(A)
> while (!check_value(V)) {
> unlock(A);
> block_on(B);
> lock(A);
> };
> unlock(A);
>
> Note: B is a wait queue implemented with futexes.
>
> If the waker would use FUTEX_WAKE and wake all waiters waiting on B
> then those would all try to acquire lock A. That's called thundering
> herd and pointless because all except one would immediately block on
> lock A again.
>
> Requeueing prevents that because it only wakes one waiter and moves
> the other waiters to lock A. When that waiter unlocks A then the next
> waiter can proceed ...
Thanks, I used a lot of that text.
[...]
>> FUTEX_WAKE_BITSET (since Linux 2.6.25)
>> This operation is the same as FUTEX_WAKE except that the
>> val3 argument is used to provide a 32-bit bitset to the
>> kernel. This bitset is used to select which waiters
>> should be woken up. The selection is done by a bit-wise
>> AND of the "wake" bitset (i.e., the value in val3) and the
>> bitset which is stored in the kernel-internal state of the
>> waiter (the "wait" bitset that is set using
>> FUTEX_WAIT_BITSET). All of the waiters for which the
>> result of the AND is nonzero are woken up; the remaining
>> waiters are left sleeping.
>>
>> .\" FIXME XXX Is this next paragraph that I added okay?
>> The effect of FUTEX_WAIT_BITSET and FUTEX_WAKE_BITSET is
>> to allow selective wake-ups among multiple waiters that
>> are blocked on the same futex. Note, however, that using
>> this bitset multiplexing feature on a futex is less effi‐
>> cient than simply using multiple futexes, because employ‐
>
> s/is less efficient/can be less efficient/
>
> It really depends on the usecase.
Thanks. Amended as you suggest.
>> ing bitset multiplexing requires the kernel to check all
>> waiters on a futex, including those that are not inter‐
>> ested in being woken up (i.e., they do not have the rele‐
>> vant bit set in their "wait" bitset).
>>
>> The uaddr2 and timeout arguments are ignored.
>>
>> The FUTEX_WAIT and FUTEX_WAKE operations correspond to
>> FUTEX_WAIT_BITSET and FUTEX_WAKE_BITSET operations where
>> the bitsets are all ones.
>>
>> Priority-inheritance futexes
>> Linux supports priority-inheritance (PI) futexes in order to han‐
>> dle priority-inversion problems that can be encountered with nor‐
>> mal futex locks. Priority inversion is the problem that occurs
>> when a high-priority task is blocked waiting to acquire a lock
>> held by a low-priority task, while tasks at an intermediate pri‐
>> ority continuously preempt the low-priority task from the CPU.
>> Consequently, the low-priority task makes no progress toward
>> releasing the lock, and the high-priority task remains blocked.
>>
>> Priority inheritance is a mechanism for dealing with the prior‐
>> ity-inversion problem. With this mechanism, when a high-priority
>> task becomes blocked by a lock held by a low-priority task, the
>> latter's priority is temporarily raised to that of the former, so
>> that it is not preempted by any intermediate level tasks, and can
>> thus make progress toward releasing the lock. To be effective,
>> priority inheritance must be transitive, meaning that if a high-
>> priority task blocks on a lock held by a lower-priority task that
>> is itself blocked by lock held by another intermediate-priority
>> task (and so on, for chains of arbitrary length), then both of
>> those task (or more generally, all of the tasks in a lock chain)
>> have their priorities raised to be the same as the high-priority
>> task.
>>
>> .\" FIXME XXX The following is my attempt at a definition of PI futexes,
>> .\" based on mail discussions with Darren Hart. Does it seem okay?
>>
>> From a user-space perspective, what makes a futex PI-aware is a
>> policy agreement between user space and the kernel about the
>> value of the futex word (described in a moment), coupled with the
>> use of the PI futex operations described below (in particular,
>> FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and FUTEX_CMP_REQUEUE_PI).
>>
>> .\" FIXME XXX ===== Start of adapted Hart/Guniguntala text =====
>> .\" The following text is drawn from the Hart/Guniguntala paper
>> .\" (listed in SEE ALSO), but I have reworded some pieces
>> .\" significantly. Please check it.
>>
>> The PI futex operations described below differ from the other
>> futex operations in that they impose policy on the use of the
>> value of the futex word:
>>
>> * If the lock is not acquired, the futex word's value shall be
>> 0.
>>
>> * If the lock is acquired, the futex word's value shall be the
>> thread ID (TID; see gettid(2)) of the owning thread.
>>
>> * If the lock is owned and there are threads contending for the
>> lock, then the FUTEX_WAITERS bit shall be set in the futex
>> word's value; in other words, this value is:
>>
>> FUTEX_WAITERS | TID
>>
>>
>> Note that a PI futex word never just has the value FUTEX_WAITERS,
>> which is a permissible state for non-PI futexes.
>>
>> With this policy in place, a user-space application can acquire a
>> not-acquired lock or release a lock that no other threads try to
>> acquire using atomic instructions executed in user space (e.g., a
>> compare-and-swap operation such as cmpxchg on the x86 architec‐
>> ture). Acquiring a lock simply consists of using compare-and-
>> swap to atomically set the futex word's value to the caller's TID
>> if its previous value was 0. Releasing a lock requires using
>> compare-and-swap to set the futex word's value to 0 if the previ‐
>> ous value was the expected TID.
>>
>> If a futex is already acquired (i.e., has a nonzero value), wait‐
>> ers must employ the FUTEX_LOCK_PI operation to acquire the lock.
>> If other threads are waiting for the lock, then the FUTEX_WAITERS
>> bit is set in the futex value; in this case, the lock owner must
>> employ the FUTEX_UNLOCK_PI operation to release the lock.
>>
>> In the cases where callers are forced into the kernel (i.e.,
>> required to perform a futex() call), they then deal directly with
>> a so-called RT-mutex, a kernel locking mechanism which implements
>> the required priority-inheritance semantics. After the RT-mutex
>> is acquired, the futex value is updated accordingly, before the
>> calling thread returns to user space.
>> .\" FIXME ===== End of adapted Hart/Guniguntala text =====
>
> That's correct.
Thanks.
>> .\" FIXME We need some explanation in the following paragraph of *why*
>> .\" it is important to note that "the kernel will update the
>> .\" futex word's value prior
>> It is important to note to returning to user space" . Can someone
>> explain? that the kernel will update the futex word's value
>> prior to returning to user space. Unlike the other futex opera‐
>> tions described above, the PI futex operations are designed for
>> the implementation of very specific IPC mechanisms.
>
> If there are multiple waiters on a pi futex then a wake pi operation
> will wake the first waiter and hand over the lock to this waiter. This
> includes handing over the rtmutex which represents the futex in the
> kernel. The strict requirement is that the futex owner and the rtmutex
> owner must be the same, except for the update period which is
> serialized by the futex internal locking. That means the kernel must
> update the user space value prior to returning to user space.
Okay -- thanks. I've noted these details, but need to consider more about
what changes (if any) are needed to the page.
>> .\" FIXME XXX In discussing errors for FUTEX_CMP_REQUEUE_PI, Darren Hart
>> .\" made the observation that "EINVAL is returned if the non-pi
>> .\" to pi or op pairing semantics are violated."
>> .\" Probably there needs to be a general statement about this
>> .\" requirement, probably located at about this point in the page.
>> .\" Darren (or someone else), care to take a shot at this?
>
> Well, that's hard to describe because the kernel only has a limited
> way of detecting such mismatches. It only can detect it when there are
> non PI waiters on a futex and a PI function is called or vice versa.
Hmmm. Okay, I filed your comments away for reference, but
hopefully someone can help with some actual text.
>> .\" FIXME Somewhere on this page (I guess under the discussion of PI
>> .\" futexes) we need a discussion of the FUTEX_OWNER_DIED bit.
>> .\" Can someone propose a text?
>
> If a futex has a rtmutex associated in the kernel, i.e. when there are
> blocked waiters, and the owner of the futex/rtmutex dies unexpectedly,
> then the kernel cleans up the rtmutex (as it holds a reference to the
> dying task) and hands it over to the next waiter. That requires that
> the user space value is updated accordingly. The kernel sets the
> FUTEX_OWNER_DIED in the user space value along with the TID of the new
> owner. User space is responsible for cleaning this up, though there
> are cases where the kernel does the cleanup.
>
> The FUTEX_OWNER_DIED bit can also be set on uncontended futexes, where
> the kernel has no state associated. This happens via the robust futex
> mechanism. In that case the futex value will be set to
> FUTEX_OWNER_DIED. The robust futex mechanism is also available for non
> PI futexes.
???
So, I added part of that text to the page, as follows:
If a futex has an associated RT-mutex in the kernel (i.e., there
are blocked waiters) and the owner of the futex/RT-mutex dies
unexpectedly, then the kernel cleans up the RT-mutex and hands it
over to the next waiter. This in turn requires that the user-
space value is updated accordingly. To indicate that this is
required, the kernel sets the FUTEX_OWNER_DIED bit in the futex
word along with the thread ID of the new owner. User space is
then responsible for cleaning this up (though there are cases
where the kernel does the cleanup).
Okay?
I think the last sentence still requires a little work though. What does
user space need to do in terms of clean up?
>> PI futexes are operated on by specifying one of the following
>> values in futex_op:
>>
>> FUTEX_LOCK_PI (since Linux 2.6.18)
>> .\" FIXME I did some significant rewording of tglx's text to create
>> .\" the text below.
>> .\" Please check the following paragraph, in case I injected
>> .\" errors.
>> This operation is used after after an attempt to acquire
>> the lock via an atomic user-space instruction failed
>> because the futex word has a nonzero value—specifically,
>> because it contained the namespace-specific TID of the
>> lock owner.
>> .\" FIXME In the preceding line, what does "namespace-specific" mean?
>> .\" (I kept those words from tglx.)
>> .\" That is, what kind of namespace are we talking about?
>> .\" (I suppose we are talking PID namespaces here, but I want to
>> .\" be sure.)
>
> Yes.
Thanks.
>> The operation checks the value of the futex word at the
>> address uaddr. If the value is 0, then the kernel tries
>> to atomically set the futex value to the caller's TID.
>> .\" FIXME What would be the cause(s) of failure referred to
>> .\" in the following sentence?
>> If
>> that fails, or the futex word's value is nonzero, the ker‐
>
> 'If that fails' does not make sense. If the user space access fails we
> return -EFAULT and let user space deal with the mess.
Okay. , I removed "that fails, or"
>
> The operation here is similar to the FUTEX_WAIT logic. When the user
> space atomic acquire does not succeed because the futex value was non
> zero, then the waiter goes into the kernel, takes the kernel internal
> lock and retries the acquisition under the lock. If the acquisition
> does not succeed either, then it sets the FUTEX_WAITERS bit, to signal
> the lock owner that it needs to go into the kernel. Here is the pseudo
> code:
>
> lock(kernel_lock);
> retry:
>
> /*
> * Owner might have unlocked in userspace before we
> * were able to set the waiter bit.
> */
> if (atomic_acquire(futex) == SUCCESS) {
> unlock(kernel_lock());
> return 0;
> }
>
> /*
> * Owner might have unlocked after the above atomic_acquire()
> * attempt.
> */
> if (atomic_set_waiters_bit(futex) != SUCCESS)
> goto retry;
>
> queue_waiter();
> unlock(kernel_lock);
> block();
Thanks, I filed the above point away as a comment in the source.
>> nel atomically sets the FUTEX_WAITERS bit, which signals
>> the futex owner that it cannot unlock the futex in user
>> space atomically by setting the futex value to 0. After
>> that, the kernel tries to find the thread which is associ‐
>> ated with the owner TID, creates or reuses kernel state on
>> behalf of the owner and attaches the waiter to it.
>> .\" FIXME Could I get a bit more detail on the previous lines?
>> .\" What is "creates or reuses kernel state" about?
>> .\" (I think this needs to be clearer in the page)
>
> If this is the first waiter then there is no kernel state for this
> futex, so it is created. That means the rtmutex is locked and the
> futex owner established as the owner of the rtmutex. If there is a
> waiter, then the state is reused, i.e. the new waiter is enqueued into
> the rtmutex waiter list.
Thanks, I've reworked this passage somewhat, to read:
The operation checks the value of the futex word at the
address uaddr. If the value is 0, then the kernel tries
to atomically set the futex value to the caller's TID. If
the futex word's value is nonzero, the kernel atomically
sets the FUTEX_WAITERS bit, which signals the futex owner
that it cannot unlock the futex in user space atomically
by setting the futex value to 0. After that, the kernel:
1. Tries to find the thread which is associated with the
owner TID.
2. Creates or reuses kernel state on behalf of the owner.
(If this is the first waiter, there is no kernel state
for this futex, so kernel state is created by locking
the RT-mutex and the futex owner is made the owner of
the RT-mutex. If there are existing waiters, then the
existing state is reused.)
3. Attaches the waiter to it (i.e., the waiter is enqueued
on the RT-mutex waiter list).
>> .\" FIXME In the next line, what type of "priority" are we talking about?
>> .\" Realtime priorities for SCHED_FIFO and SCHED_RR?
>> .\" Or something else?
>>
>> The
>> enqueueing of the waiter is in descending priority order
>> if more than one waiter exists.
>
> That also covers sched deadline.
???
Thanks. If the realm is restricted purely to SCHED_OTHER (SCHED_NORMAL)
processes, does the nice value come into play also?
>> .\" FIXME In the next sentence, what type of "priority" are we talking about?
>> .\" Realtime priorities for SCHED_FIFO and SCHED_RR?
>> .\" Or something else?
>> .\" FIXME What does "bandwidth" refer to in the next sentence?
>>
>> The owner inherits either
>> the priority or the bandwidth of the waiter.
>
> If the highest priority waiter is SCHED_DEADLINE, then the owner
> inherits cpu bandwidth from the waiter as there is no priority
> associated to SCHED_DEADLINE tasks.
>
> If the highest priority waiter is SCHED_FIFO/RR, then the owner
> inherits the waiter priority.
Thanks!
>> .\" FIXME In the preceding sentence, what determines whether the
>> .\" owner inherits the priority versus the bandwidth?
>>
>> .\" FIXME Could I get some help translating the next sentence into
>> .\" something that user-space developers (and I) can understand?
>> .\" In particular, what are "nested locks" in this context?
>>
>> This inheri‐
>> tance follows the lock chain in the case of nested locking
>> and performs deadlock detection.
>
> T1 blocks on lock A held by T2
> T2 blocks on lock B held by T3
>
> So we have a lock chain A, B. The inheritance mechanism follows the
> lock chain and propagates the highest waiter priority up to the end of
> the chain.
Thanks.
By now, I have reworded this passage to read:
If more than one waiter exists, the enqueueing of the
waiter is in descending priority order. (For information
on priority ordering, see the discussion of the
SCHED_DEADLINE, SCHED_FIFO, and SCHED_RR scheduling poli‐
cies in sched(7).) The owner inherits either the waiter's
CPU bandwidth (if the waiter is scheduled under the
SCHED_DEADLINE policy) or the waiter's priority (if the
waiter is scheduled under the SCHED_RR or SCHED_FIFO pol‐
icy). This inheritance follows the lock chain in the case
of nested locking (i.e., task 1 blocks on lock A, held by
task 2, while task 2 blocks on lock B, held by task 3) and
performs deadlock detection.
>> .\" FIXME tglx said "The timeout argument is handled as described in
>> .\" FUTEX_WAIT." However, it appears to me that this is not right.
>> .\" Is the following formulation correct?
>> The timeout argument provides a timeout for the lock
>> attempt. It is interpreted as an absolute time, measured
>> against the CLOCK_REALTIME clock. If timeout is NULL, the
>> operation will block indefinitely.
>
> Indeed.
Thanks.
>> The uaddr2, val, and val3 arguments are ignored.
>>
>> FUTEX_TRYLOCK_PI (since Linux 2.6.18)
>> .\" FIXME I think it would be helpful here to say a few more words about
>> .\" the difference(s) between FUTEX_LOCK_PI and FUTEX_TRYLOCK_PI.
>> .\" Can someone propose something?
>> This operation tries to acquire the futex at uaddr. It
>> deals with the situation where the TID value at uaddr is
>> 0, but the FUTEX_WAITERS bit is set. User space cannot
>> handle this condition in a race-free manner
>> .\" FIXME How does the situation in the previous sentence come about?
>> .\" Probably it would be helpful to say something about that in
>> .\" the man page.
>> .\" FIXME And *how* does FUTEX_TRYLOCK_PI deal with this situation?
>
> That should be expressed differently:
>
> This operation tries to acquire the futex at uaddr. It's
> invoked when the user space atomic acquire did not
> succeed because the user space value was not 0.
>
> The trylock in kernel might succeed because the user space
> value contains stale state (FUTEX_WAITERS and or
> FUTEX_OWNER_DIED). This can happen when the owner of the
> futex died.
???
So by now, I've reworked this text to be:
FUTEX_TRYLOCK_PI (since Linux 2.6.18)
This operation tries to acquire the futex at uaddr. It is
invoked when a user-space atomic acquire did not succeed
because the futex word was not 0.
The trylock in kernel might succeed because the futex word
contains stale state (FUTEX_WAITERS and/or
FUTEX_OWNER_DIED). This can happen when the owner of the
futex died. User space cannot handle this condition in a
race-free manner
Okay?
I must admit that I find "the trylock in kernel might succeed"hard
to understand. Could you elaborate a little?
>> The uaddr2, val, timeout, and val3 arguments are ignored.
>>
>> FUTEX_UNLOCK_PI (since Linux 2.6.18)
>> This operation wakes the top priority waiter that is wait‐
>> ing in FUTEX_LOCK_PI on the futex address provided by the
>> uaddr argument.
>>
>> This is called when the user space value at uaddr cannot
>> be changed atomically from a TID (of the owner) to 0.
>>
>> The uaddr2, val, timeout, and val3 arguments are ignored.
>>
>> FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
>> This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
>> It requeues waiters that are blocked via
>> FUTEX_WAIT_REQUEUE_PI on uaddr from a non-PI source futex
>> (uaddr) to a PI target futex (uaddr2).
>>
>> As with FUTEX_CMP_REQUEUE, this operation wakes up a maxi‐
>> mum of val waiters that are waiting on the futex at uaddr.
>> However, for FUTEX_CMP_REQUEUE_PI, val is required to be 1
>> (since the main point is to avoid a thundering herd). The
>> remaining waiters are removed from the wait queue of the
>> source futex at uaddr and added to the wait queue of the
>> target futex at uaddr2.
>>
>> The val2 and val3 arguments serve the same purposes as for
>> FUTEX_CMP_REQUEUE.
>> .\" FIXME The page at http://locklessinc.com/articles/futex_cheat_sheet/
>> .\" notes that "priority-inheritance Futex to priority-inheritance
>> .\" Futex requeues are currently unsupported". Do we need to say
>> .\" something in the man page about that?
>>
>
> And they never will be supported because they make no sense at all.
Okay, thanks. I've removed that FIXME.
>>
>> FUTEX_WAIT_REQUEUE_PI (since Linux 2.6.31)
>>
>> .\" FIXME I find the next sentence (from tglx) pretty hard to grok.
>> .\" Could someone explain it a bit more?
>>
>> Wait operation to wait on a non-PI futex at uaddr and
>> potentially be requeued onto a PI futex at uaddr2. The
>> wait operation on uaddr is the same as FUTEX_WAIT.
>
> let me copy the pseudo code from cmp_requeue
>
> lock(A)
> while (!check_value(V)) {
> unlock(A);
> block_on(B);
> lock(A);
> };
> unlock(A);
>
> So in this case B is the non-PI futex (the wait queue) and A is a PI
> futex. So wait operation on B is the same as in FUTEX_WAIT.
Thanks. I've done a little rewording here. See below.
>> .\" FIXME I'm not quite clear on the meaning of the following sentence.
>> .\" Is this trying to say that while blocked in a
>> .\" FUTEX_WAIT_REQUEUE_PI, it could happen that another
>> .\" task does a FUTEX_WAKE on uaddr that simply causes
>> .\" a normal wake, with the result that the FUTEX_WAIT_REQUEUE_PI
>> .\" does not complete? What happens then to the FUTEX_WAIT_REQUEUE_PI
>> .\" opertion? Does it remain blocked, or does it unblock
>> .\" In which case, what does user space see?
>
> It unblocks and returns -EWOULDBLOCK.
Thanks.
>> The
>> waiter can be removed from the wait on uaddr via
>> FUTEX_WAKE without requeueing on uaddr2.
???
So now I've reworded the opening text describing FUTEX_WAIT_REQUEUE_PI
as follows:
FUTEX_WAIT_REQUEUE_PI (since Linux 2.6.31)
Wait on a non-PI futex at uaddr and potentially be
requeued (via a FUTEX_CMP_REQUEUE_PI operation in another
task) onto a PI futex at uaddr2. The wait operation on
uaddr is the same as for FUTEX_WAIT.
The waiter can be removed from the wait on uaddr without
requeueing on uaddr2 via a FUTEX_WAIT operation in another
task. In this case, the FUTEX_WAIT_REQUEUE_PI operation
returns with the error EWOULDBLOCK.
Okay?
>> .\" FIXME Please check the following. tglx said "The timeout argument
>> .\" is handled as described in FUTEX_WAIT.", but the truth is
>> .\" as below, AFAICS
>>
>> If timeout is not NULL, it specifies a timeout for the
>> wait operation; this timeout is interpreted as outlined
>> above in the description of the FUTEX_CLOCK_REALTIME
>> option. If timeout is NULL, the operation can block
>> indefinitely.
>>
>> The val3 argument is ignored.
>
> Correct
Thanks.
>> .\" FIXME Re the preceding sentence... Actually 'val3' is internally set to
>> .\" FUTEX_BITSET_MATCH_ANY before calling futex_wait_requeue_pi().
>> .\" I'm not sure we need to say anything about this though.
>> .\" Comments?
>
> That's a kernel internal and can be removed
Thanks.
>>
>> The FUTEX_WAIT_REQUEUE_PI and FUTEX_CMP_REQUEUE_PI were
>> added to support a fairly specific use case: support for
>> priority-inheritance-aware POSIX threads condition vari‐
>> ables. The idea is that these operations should always be
>> paired, in order to ensure that user space and the kernel
>> remain in sync. Thus, in the FUTEX_WAIT_REQUEUE_PI opera‐
>> tion, the user-space application pre-specifies the target
>> of the requeue that takes place in the
>> FUTEX_CMP_REQUEUE_PI operation.
>>
>> RETURN VALUE
[...]
>> ERRORS
>> EACCES No read access to the memory of a futex word.
>>
>> EAGAIN (FUTEX_WAIT, FUTEX_WAIT_BITSET, FUTEX_WAIT_REQUEUE_PI) The
>> value pointed to by uaddr was not equal to the expected
>> value val at the time of the call.
>>
>> Note: on Linux, the symbolic names EAGAIN and EWOULDBLOCK
>> (both of which appear in different parts of the kernel
>> futex code) have the same value.
>>
>> EAGAIN (FUTEX_CMP_REQUEUE, FUTEX_CMP_REQUEUE_PI) The value
>> pointed to by uaddr is not equal to the expected value
>> val3. (This probably indicates a race; use the safe
>> FUTEX_WAKE now.)
>> .\" FIXME: Is the preceding sentence "(This probably...") correct?
>> .\" [I would prefer to remove this sentence. --triegel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org]
>
> This part should be removed:
>
> "(This probably indicates a race; use the safe FUTEX_WAKE now.)
Thanks. Done.
>>
>> EAGAIN (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
>> The futex owner thread ID of uaddr (for
>> FUTEX_CMP_REQUEUE_PI: uaddr2) is about to exit, but has
>> not yet handled the internal state cleanup. Try again.
>>
>> .\" FIXME XXX Should there be an EAGAIN case for FUTEX_TRYLOCK_PI?
>> .\" It seems so, looking at the handling of the rt_mutex_trylock()
>> .\" call in futex_lock_pi()
>> .\" (Davidlohr also thinks so.)
>
> Yes. It's the same internal logic so it can return EAGAIN
Thanks.
>> EDEADLK
>> (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
>> The futex word at uaddr is already locked by the caller.
>>
>> EDEADLK
>>
>> .\" FIXME I reworded tglx's text somewhat; is the following okay?
>>
>> (FUTEX_CMP_REQUEUE_PI) While requeueing a waiter to the PI
>> futex for the futex word at uaddr2, the kernel detected a
>> deadlock.
>
> Yes
Thanks.
>>
>> .\" FIXME XXX I see that kernel/locking/rtmutex.c uses EDEADLK in some
>> .\" places, and EDEADLOCK in others. On almost all architectures
>> .\" these constants are synonymous. Is there a reason that both
>> .\" names are used?
>
> No. We should probably fix that.
Okay.
[...]
>> EINVAL (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_UNLOCK_PI) The
>> kernel detected an inconsistency between the user-space
>> state at uaddr and the kernel state. This indicates
>> either state corruption or that the kernel found a waiter
>> on uaddr which is waiting via FUTEX_WAIT or
>> FUTEX_WAIT_BITSET.
>
>> .\" FIXME Above, tglx did not mention the "state corruption" case for
>> .\" FUTEX_UNLOCK_PI, but I have added it, since I'm estimating
>> .\" that it also applied for FUTEX_UNLOCK_PI.
>> .\" So, does that case also apply for FUTEX_UNLOCK_PI?
>
> Yes
Thanks.
>>
>> EINVAL (FUTEX_CMP_REQUEUE_PI) The kernel detected an inconsis‐
>> tency between the user-space state at uaddr2 and the ker‐
>> nel state; that is, the kernel detected a waiter which
>> waits via FUTEX_WAIT on uaddr2.
>> .\" FIXME In the preceding sentence, tglx did not mention FUTEX_WAIT_BITSET,
>> .\" but should that not also be included here?
>
> Yes
Thanks. I added "[via FUTEX_WAIT] or FUTEX_WAIT_BITSET".
>>
>> EINVAL (FUTEX_CMP_REQUEUE_PI) The kernel detected an inconsis‐
>> tency between the user-space state at uaddr and the kernel
>> state; that is, the kernel detected a waiter which waits
>> via FUTEX_WAIT or FUTEX_WAIT_BITESET on uaddr.
>>
>> EINVAL (FUTEX_CMP_REQUEUE_PI) The kernel detected an inconsis‐
>> tency between the user-space state at uaddr and the kernel
>> state; that is, the kernel detected a waiter which waits
>> on uaddr via FUTEX_LOCK_PI (instead of
>> FUTEX_WAIT_REQUEUE_PI).
>>
>> .\" FIXME XXX The following is a reworded version of Darren Hart's text.
>> .\" Please check that I did not introduce any errors.
>> EINVAL (FUTEX_CMP_REQUEUE_PI) An attempt was made to requeue a
>> waiter to a futex other than that specified by the match‐
>> ing FUTEX_WAIT_REQUEUE_PI call for that waiter.
>
> Correct. That handles the case:
>
> wait_requeue_pi(A, B);
> requeue_pi(A, C);
Thanks.
[...]
>> ESRCH (FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, FUTEX_CMP_REQUEUE_PI)
>>
>> .\" FIXME I reworded the following sentence a bit differently from
>> .\" tglx's formulation. Is it okay?
>>
>> The thread ID in the futex word at uaddr does not exist.
>
> Right.
Thanks.
>> ESRCH (FUTEX_CMP_REQUEUE_PI)
>>
>> .\" FIXME I reworded the following sentence a bit differently from
>> .\" tglx's formulation. Is it okay?
>>
>> The thread ID in the futex word at
>> uaddr2 does not exist.
>
> Right
Thanks.
Cheers,
Michael
PS: The latest version of the page can be found in its entirety at
http://git.kernel.org/cgit/docs/man-pages/man-pages.git/log/?h=draft_futex
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 8/9] xen/privcmd: Further s/MFN/GFN/ clean-up
From: Julien Grall @ 2015-08-07 16:34 UTC (permalink / raw)
To: xen-devel
Cc: x86, Russell King, ian.campbell, stefano.stabellini, linux-api,
linux-kernel, Julien Grall, Ingo Molnar, David Vrabel,
H. Peter Anvin, Boris Ostrovsky, Thomas Gleixner,
linux-arm-kernel
In-Reply-To: <1438965282-19103-1-git-send-email-julien.grall@citrix.com>
The privcmd code is mixing the usage of GFN and MFN within the same
functions which make the code difficult to understand when you only work
with auto-translated guests.
The privcmd driver is only dealing with GFN so replace all the mention
of MFN into GFN.
The ioctl structure used to map foreign change has been left unchanged
given that the userspace is using it. Nonetheless, add a comment to
explain the expected value within the "mfn" field.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-api@vger.kernel.org
Changes in v2:
- Add David's reviewed-by
---
arch/arm/xen/enlighten.c | 18 +++++++++---------
arch/x86/xen/mmu.c | 32 ++++++++++++++++----------------
drivers/xen/privcmd.c | 44 ++++++++++++++++++++++----------------------
drivers/xen/xlate_mmu.c | 18 +++++++++---------
include/uapi/xen/privcmd.h | 4 ++++
include/xen/xen-ops.h | 10 +++++-----
6 files changed, 65 insertions(+), 61 deletions(-)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index c50c8d3..eeeab07 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -49,35 +49,35 @@ static __read_mostly unsigned int xen_events_irq;
static __initdata struct device_node *xen_node;
-int xen_remap_domain_mfn_array(struct vm_area_struct *vma,
+int xen_remap_domain_gfn_array(struct vm_area_struct *vma,
unsigned long addr,
- xen_pfn_t *mfn, int nr,
+ xen_pfn_t *gfn, int nr,
int *err_ptr, pgprot_t prot,
unsigned domid,
struct page **pages)
{
- return xen_xlate_remap_gfn_array(vma, addr, mfn, nr, err_ptr,
+ return xen_xlate_remap_gfn_array(vma, addr, gfn, nr, err_ptr,
prot, domid, pages);
}
-EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_array);
+EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_array);
/* Not used by XENFEAT_auto_translated guests. */
-int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
+int xen_remap_domain_gfn_range(struct vm_area_struct *vma,
unsigned long addr,
- xen_pfn_t mfn, int nr,
+ xen_pfn_t gfn, int nr,
pgprot_t prot, unsigned domid,
struct page **pages)
{
return -ENOSYS;
}
-EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
+EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_range);
-int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
+int xen_unmap_domain_gfn_range(struct vm_area_struct *vma,
int nr, struct page **pages)
{
return xen_xlate_unmap_gfn_range(vma, nr, pages);
}
-EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
+EXPORT_SYMBOL_GPL(xen_unmap_domain_gfn_range);
static void xen_percpu_init(void)
{
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 59ec3c3..53447a7 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -2812,9 +2812,9 @@ static int remap_area_mfn_pte_fn(pte_t *ptep, pgtable_t token,
return 0;
}
-static int do_remap_mfn(struct vm_area_struct *vma,
+static int do_remap_gfn(struct vm_area_struct *vma,
unsigned long addr,
- xen_pfn_t *mfn, int nr,
+ xen_pfn_t *gfn, int nr,
int *err_ptr, pgprot_t prot,
unsigned domid,
struct page **pages)
@@ -2830,14 +2830,14 @@ static int do_remap_mfn(struct vm_area_struct *vma,
if (xen_feature(XENFEAT_auto_translated_physmap)) {
#ifdef CONFIG_XEN_PVH
/* We need to update the local page tables and the xen HAP */
- return xen_xlate_remap_gfn_array(vma, addr, mfn, nr, err_ptr,
+ return xen_xlate_remap_gfn_array(vma, addr, gfn, nr, err_ptr,
prot, domid, pages);
#else
return -EINVAL;
#endif
}
- rmd.mfn = mfn;
+ rmd.mfn = gfn;
rmd.prot = prot;
/* We use the err_ptr to indicate if there we are doing a contigious
* mapping or a discontigious mapping. */
@@ -2865,8 +2865,8 @@ static int do_remap_mfn(struct vm_area_struct *vma,
batch_left, &done, domid);
/*
- * @err_ptr may be the same buffer as @mfn, so
- * only clear it after each chunk of @mfn is
+ * @err_ptr may be the same buffer as @gfn, so
+ * only clear it after each chunk of @gfn is
* used.
*/
if (err_ptr) {
@@ -2896,19 +2896,19 @@ out:
return err < 0 ? err : mapped;
}
-int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
+int xen_remap_domain_gfn_range(struct vm_area_struct *vma,
unsigned long addr,
- xen_pfn_t mfn, int nr,
+ xen_pfn_t gfn, int nr,
pgprot_t prot, unsigned domid,
struct page **pages)
{
- return do_remap_mfn(vma, addr, &mfn, nr, NULL, prot, domid, pages);
+ return do_remap_gfn(vma, addr, &gfn, nr, NULL, prot, domid, pages);
}
-EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
+EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_range);
-int xen_remap_domain_mfn_array(struct vm_area_struct *vma,
+int xen_remap_domain_gfn_array(struct vm_area_struct *vma,
unsigned long addr,
- xen_pfn_t *mfn, int nr,
+ xen_pfn_t *gfn, int nr,
int *err_ptr, pgprot_t prot,
unsigned domid, struct page **pages)
{
@@ -2917,13 +2917,13 @@ int xen_remap_domain_mfn_array(struct vm_area_struct *vma,
* cause of "wrong memory was mapped in".
*/
BUG_ON(err_ptr == NULL);
- return do_remap_mfn(vma, addr, mfn, nr, err_ptr, prot, domid, pages);
+ return do_remap_gfn(vma, addr, gfn, nr, err_ptr, prot, domid, pages);
}
-EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_array);
+EXPORT_SYMBOL_GPL(xen_remap_domain_gfn_array);
/* Returns: 0 success */
-int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
+int xen_unmap_domain_gfn_range(struct vm_area_struct *vma,
int numpgs, struct page **pages)
{
if (!pages || !xen_feature(XENFEAT_auto_translated_physmap))
@@ -2935,4 +2935,4 @@ int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
return -EINVAL;
#endif
}
-EXPORT_SYMBOL_GPL(xen_unmap_domain_mfn_range);
+EXPORT_SYMBOL_GPL(xen_unmap_domain_gfn_range);
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5a29616..c6deb87 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -193,16 +193,16 @@ static int traverse_pages_block(unsigned nelem, size_t size,
return ret;
}
-struct mmap_mfn_state {
+struct mmap_gfn_state {
unsigned long va;
struct vm_area_struct *vma;
domid_t domain;
};
-static int mmap_mfn_range(void *data, void *state)
+static int mmap_gfn_range(void *data, void *state)
{
struct privcmd_mmap_entry *msg = data;
- struct mmap_mfn_state *st = state;
+ struct mmap_gfn_state *st = state;
struct vm_area_struct *vma = st->vma;
int rc;
@@ -216,7 +216,7 @@ static int mmap_mfn_range(void *data, void *state)
((msg->va+(msg->npages<<PAGE_SHIFT)) > vma->vm_end))
return -EINVAL;
- rc = xen_remap_domain_mfn_range(vma,
+ rc = xen_remap_domain_gfn_range(vma,
msg->va & PAGE_MASK,
msg->mfn, msg->npages,
vma->vm_page_prot,
@@ -236,7 +236,7 @@ static long privcmd_ioctl_mmap(void __user *udata)
struct vm_area_struct *vma;
int rc;
LIST_HEAD(pagelist);
- struct mmap_mfn_state state;
+ struct mmap_gfn_state state;
/* We only support privcmd_ioctl_mmap_batch for auto translated. */
if (xen_feature(XENFEAT_auto_translated_physmap))
@@ -273,7 +273,7 @@ static long privcmd_ioctl_mmap(void __user *udata)
rc = traverse_pages(mmapcmd.num, sizeof(struct privcmd_mmap_entry),
&pagelist,
- mmap_mfn_range, &state);
+ mmap_gfn_range, &state);
out_up:
@@ -299,18 +299,18 @@ struct mmap_batch_state {
int global_error;
int version;
- /* User-space mfn array to store errors in the second pass for V1. */
- xen_pfn_t __user *user_mfn;
+ /* User-space gfn array to store errors in the second pass for V1. */
+ xen_pfn_t __user *user_gfn;
/* User-space int array to store errors in the second pass for V2. */
int __user *user_err;
};
-/* auto translated dom0 note: if domU being created is PV, then mfn is
- * mfn(addr on bus). If it's auto xlated, then mfn is pfn (input to HAP).
+/* auto translated dom0 note: if domU being created is PV, then gfn is
+ * mfn(addr on bus). If it's auto xlated, then gfn is pfn (input to HAP).
*/
static int mmap_batch_fn(void *data, int nr, void *state)
{
- xen_pfn_t *mfnp = data;
+ xen_pfn_t *gfnp = data;
struct mmap_batch_state *st = state;
struct vm_area_struct *vma = st->vma;
struct page **pages = vma->vm_private_data;
@@ -321,8 +321,8 @@ static int mmap_batch_fn(void *data, int nr, void *state)
cur_pages = &pages[st->index];
BUG_ON(nr < 0);
- ret = xen_remap_domain_mfn_array(st->vma, st->va & PAGE_MASK, mfnp, nr,
- (int *)mfnp, st->vma->vm_page_prot,
+ ret = xen_remap_domain_gfn_array(st->vma, st->va & PAGE_MASK, gfnp, nr,
+ (int *)gfnp, st->vma->vm_page_prot,
st->domain, cur_pages);
/* Adjust the global_error? */
@@ -347,22 +347,22 @@ static int mmap_return_error(int err, struct mmap_batch_state *st)
if (st->version == 1) {
if (err) {
- xen_pfn_t mfn;
+ xen_pfn_t gfn;
- ret = get_user(mfn, st->user_mfn);
+ ret = get_user(gfn, st->user_gfn);
if (ret < 0)
return ret;
/*
* V1 encodes the error codes in the 32bit top
- * nibble of the mfn (with its known
+ * nibble of the gfn (with its known
* limitations vis-a-vis 64 bit callers).
*/
- mfn |= (err == -ENOENT) ?
+ gfn |= (err == -ENOENT) ?
PRIVCMD_MMAPBATCH_PAGED_ERROR :
PRIVCMD_MMAPBATCH_MFN_ERROR;
- return __put_user(mfn, st->user_mfn++);
+ return __put_user(gfn, st->user_gfn++);
} else
- st->user_mfn++;
+ st->user_gfn++;
} else { /* st->version == 2 */
if (err)
return __put_user(err, st->user_err++);
@@ -388,7 +388,7 @@ static int mmap_return_errors(void *data, int nr, void *state)
return 0;
}
-/* Allocate pfns that are then mapped with gmfns from foreign domid. Update
+/* Allocate pfns that are then mapped with gfns from foreign domid. Update
* the vma with the page info to use later.
* Returns: 0 if success, otherwise -errno
*/
@@ -526,7 +526,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
if (state.global_error) {
/* Write back errors in second pass. */
- state.user_mfn = (xen_pfn_t *)m.arr;
+ state.user_gfn = (xen_pfn_t *)m.arr;
state.user_err = m.err;
ret = traverse_pages_block(m.num, sizeof(xen_pfn_t),
&pagelist, mmap_return_errors, &state);
@@ -587,7 +587,7 @@ static void privcmd_close(struct vm_area_struct *vma)
if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
return;
- rc = xen_unmap_domain_mfn_range(vma, numpgs, pages);
+ rc = xen_unmap_domain_gfn_range(vma, numpgs, pages);
if (rc == 0)
free_xenballooned_pages(numpgs, pages);
else
diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index 58a5389..cff2387 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -38,8 +38,8 @@
#include <xen/interface/xen.h>
#include <xen/interface/memory.h>
-/* map fgmfn of domid to lpfn in the current domain */
-static int map_foreign_page(unsigned long lpfn, unsigned long fgmfn,
+/* map fgfn of domid to lpfn in the current domain */
+static int map_foreign_page(unsigned long lpfn, unsigned long fgfn,
unsigned int domid)
{
int rc;
@@ -49,7 +49,7 @@ static int map_foreign_page(unsigned long lpfn, unsigned long fgmfn,
.size = 1,
.space = XENMAPSPACE_gmfn_foreign,
};
- xen_ulong_t idx = fgmfn;
+ xen_ulong_t idx = fgfn;
xen_pfn_t gpfn = lpfn;
int err = 0;
@@ -62,13 +62,13 @@ static int map_foreign_page(unsigned long lpfn, unsigned long fgmfn,
}
struct remap_data {
- xen_pfn_t *fgmfn; /* foreign domain's gmfn */
+ xen_pfn_t *fgfn; /* foreign domain's gfn */
pgprot_t prot;
domid_t domid;
struct vm_area_struct *vma;
int index;
struct page **pages;
- struct xen_remap_mfn_info *info;
+ struct xen_remap_gfn_info *info;
int *err_ptr;
int mapped;
};
@@ -82,20 +82,20 @@ static int remap_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
pte_t pte = pte_mkspecial(pfn_pte(pfn, info->prot));
int rc;
- rc = map_foreign_page(pfn, *info->fgmfn, info->domid);
+ rc = map_foreign_page(pfn, *info->fgfn, info->domid);
*info->err_ptr++ = rc;
if (!rc) {
set_pte_at(info->vma->vm_mm, addr, ptep, pte);
info->mapped++;
}
- info->fgmfn++;
+ info->fgfn++;
return 0;
}
int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
unsigned long addr,
- xen_pfn_t *mfn, int nr,
+ xen_pfn_t *gfn, int nr,
int *err_ptr, pgprot_t prot,
unsigned domid,
struct page **pages)
@@ -108,7 +108,7 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
x86 PVOPS */
BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
- data.fgmfn = mfn;
+ data.fgfn = gfn;
data.prot = prot;
data.domid = domid;
data.vma = vma;
diff --git a/include/uapi/xen/privcmd.h b/include/uapi/xen/privcmd.h
index a853168..7ddeeda 100644
--- a/include/uapi/xen/privcmd.h
+++ b/include/uapi/xen/privcmd.h
@@ -44,6 +44,10 @@ struct privcmd_hypercall {
struct privcmd_mmap_entry {
__u64 va;
+ /*
+ * This should be a GFN. It's not possible to change the name because
+ * it's exposed to the user-space.
+ */
__u64 mfn;
__u64 npages;
};
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 0ce4f32..e4e214a 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -30,7 +30,7 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order);
struct vm_area_struct;
/*
- * xen_remap_domain_mfn_array() - map an array of foreign frames
+ * xen_remap_domain_gfn_array() - map an array of foreign frames
* @vma: VMA to map the pages into
* @addr: Address at which to map the pages
* @gfn: Array of GFNs to map
@@ -46,14 +46,14 @@ struct vm_area_struct;
* Returns the number of successfully mapped frames, or a -ve error
* code.
*/
-int xen_remap_domain_mfn_array(struct vm_area_struct *vma,
+int xen_remap_domain_gfn_array(struct vm_area_struct *vma,
unsigned long addr,
xen_pfn_t *gfn, int nr,
int *err_ptr, pgprot_t prot,
unsigned domid,
struct page **pages);
-/* xen_remap_domain_mfn_range() - map a range of foreign frames
+/* xen_remap_domain_gfn_range() - map a range of foreign frames
* @vma: VMA to map the pages into
* @addr: Address at which to map the pages
* @gfn: First GFN to map.
@@ -65,12 +65,12 @@ int xen_remap_domain_mfn_array(struct vm_area_struct *vma,
* Returns the number of successfully mapped frames, or a -ve error
* code.
*/
-int xen_remap_domain_mfn_range(struct vm_area_struct *vma,
+int xen_remap_domain_gfn_range(struct vm_area_struct *vma,
unsigned long addr,
xen_pfn_t gfn, int nr,
pgprot_t prot, unsigned domid,
struct page **pages);
-int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
+int xen_unmap_domain_gfn_range(struct vm_area_struct *vma,
int numpgs, struct page **pages);
int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
unsigned long addr,
--
2.1.4
^ permalink raw reply related
* [PATCH v3 0/9] Use correctly the Xen memory terminologies
From: Julien Grall @ 2015-08-07 16:34 UTC (permalink / raw)
To: xen-devel
Cc: linux-fbdev, x86, netdev, H. Peter Anvin, Jiri Slaby,
Thomas Gleixner, Russell King, linux-scsi, Tomi Valkeinen,
stefano.stabellini, Ingo Molnar, linux-input,
Jean-Christophe Plagniol-Villard, ian.campbell,
Konrad Rzeszutek Wilk, James E.J. Bottomley, Boris Ostrovsky,
linux-arm-kernel, Juergen Gross, Wei Liu, Greg Kroah-Hartman,
Dmitry Torokhov, linux-kernel, Julien Grall,
David Vrabel <david.v>
Hi all,
This patch series aims to use the memory terminologies described in
include/xen/mm.h [1] for Linux xen code.
The differences from v2 is minor but I resent it because my 64K series depends
on this series.
Linux is using mistakenly MFN when GFN is meant, I suspect this is because the
first support of Xen was for PV. This has brought some misimplementation
of memory helpers on ARM and make the developper confused about the expected
behavior.
For instance, with pfn_to_mfn, we expect to get a MFN based on the name.
Although, if we look at the implementation on x86, it's returning a GFN.
Most of the callers are also using it this way.
The first 2 patches of this series is ARM related in order to remove
PV specific helpers which should not be used and fixing the implementation of
pfn_to_mfn.
The rest of the series is here rename most of the usage in the common code
of MFN to GFN. I also took the opportunity to replace most of the call to
pfn_to_gfn in the common code by page_to_gfn avoid construction such
as pfn_to_gfn(page_to_pfn(...).
Note the one xen-blkfront will be dropped by 64K series [2], I can include
here if necessary.
Major changes in v3:
- More typoes
- Rename page_to_gfn to xen_page_to_gfn to avoid confusing with the
KVM function gfn_to_page
Major changes in v2:
- Use bfn rather than dfn for the DMA address
- Re-introduced pfn_to_mfn for PV guests only
- Typoes
For all the changes see in each patch.
This series is based on xentip for-linus-4.3 branch. A branch with all the
patches can be found here:
git://xenbits.xen.org/people/julieng/linux-arm.git branch page-renaming-v3
It has been boot tested on ARM64 and ARM32 and only built for x86.
I would be happy if someone can give a try on x86 as I don't have a x86
box setup with Xen.
Sincerely yours,
[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=e758ed14f390342513405dd766e874934573e6cb
[2] https://lkml.org/lkml/2015/7/9/628
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "James E.J. Bottomley" <JBottomley@odin.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: linux-api@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-scsi@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: x86@kernel.org
Julien Grall (9):
arm/xen: Remove helpers which are PV specific
xen: Make clear that swiotlb and biomerge are dealing with DMA address
arm/xen: implement correctly pfn_to_mfn
xen: Use correctly the Xen memory terminologies
xen/tmem: Use xen_page_to_gfn rather than pfn_to_gfn
video/xen-fbfront: Further s/MFN/GFN clean-up
hvc/xen: Further s/MFN/GFN clean-up
xen/privcmd: Further s/MFN/GFN/ clean-up
xen/xenbus: Rename the variable xen_store_mfn to xen_store_gfn
arch/arm/include/asm/xen/page.h | 44 ++++++++++++++++-----------------
arch/arm/xen/enlighten.c | 18 +++++++-------
arch/arm/xen/mm.c | 4 +--
arch/x86/include/asm/xen/page.h | 35 +++++++++++++++++++++-----
arch/x86/xen/mmu.c | 32 ++++++++++++------------
arch/x86/xen/smp.c | 2 +-
drivers/block/xen-blkfront.c | 6 ++---
drivers/input/misc/xen-kbdfront.c | 4 +--
drivers/net/xen-netback/netback.c | 4 +--
drivers/net/xen-netfront.c | 12 +++++----
drivers/scsi/xen-scsifront.c | 10 ++++----
drivers/tty/hvc/hvc_xen.c | 18 ++++++--------
drivers/video/fbdev/xen-fbfront.c | 20 +++++++--------
drivers/xen/balloon.c | 2 +-
drivers/xen/biomerge.c | 6 ++---
drivers/xen/events/events_base.c | 2 +-
drivers/xen/events/events_fifo.c | 4 +--
drivers/xen/gntalloc.c | 3 ++-
drivers/xen/manage.c | 2 +-
drivers/xen/privcmd.c | 44 ++++++++++++++++-----------------
drivers/xen/swiotlb-xen.c | 16 ++++++------
drivers/xen/tmem.c | 21 ++++++----------
drivers/xen/xenbus/xenbus_client.c | 2 +-
drivers/xen/xenbus/xenbus_dev_backend.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 16 ++++++------
drivers/xen/xlate_mmu.c | 18 +++++++-------
include/uapi/xen/privcmd.h | 4 +++
include/xen/page.h | 4 +--
include/xen/xen-ops.h | 10 ++++----
29 files changed, 191 insertions(+), 174 deletions(-)
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Kevin Hilman @ 2015-08-07 16:30 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Kevin Hilman, linux-arm-kernel@lists.infradead.org,
Greg Kroah-Hartman, Rob Herring, Mark Brown, Sascha Hauer,
linux-api, lkml, devicetree@vger.kernel.org, linux-arm-msm,
Arnd Bergmann, sboyd@codeaurora.org, Pantelis Antoniou, mporter,
Stefan Wahren, Caesar Wang
In-Reply-To: <55C4DA42.909@linaro.org>
On Fri, Aug 7, 2015 at 9:18 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Thanks Kevin,
>
> On 07/08/15 17:13, Kevin Hilman wrote:
>>
>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> writes:
>>
>>> This patchset adds a new simple NVMEM framework to kernel, and it is
>>> tested
>>> with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
>>> "begal bone cape manager" and few more on the way.
>>>
>>> Can you please consider this as 4.3 material, AFAIK there are more than 3
>>> drivers
>>> depending on this framework which are wating since last 2 merge windows.
>>>
>>> Should these patches go via ARM-SOC tree or via Greg's tree?
>>
>>
>> With Greg's ack, we can take this through the arm-soc tree (unless Greg
>> wants to queue it, which is fine also.)
>>
>
> Greg already merged these patches yesterday on to
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> in the char-misc-next branch.
Ah, great. I missed that (still catching up on post-vacation mail backlog.)
Thanks,
Kevin
^ 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