* Re: [PATCH v4 2/5] nohz: support PR_CPU_ISOLATED_STRICT mode
From: Chris Metcalf @ 2015-07-21 19:34 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Viresh Kumar, Catalin Marinas, Will Deacon,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrUvg+Dix=jG2_1J=mgQC+uRk4dthCYDcb4E5ooEfQjqtQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 07/13/2015 05:47 PM, Andy Lutomirski wrote:
> On Mon, Jul 13, 2015 at 12:57 PM, Chris Metcalf <cmetcalf-d5a29ZRxExrQT0dZR+AlfA@public.gmane.org> wrote:
>> With cpu_isolated mode, the task is in principle guaranteed not to be
>> interrupted by the kernel, but only if it behaves. In particular, if it
>> enters the kernel via system call, page fault, or any of a number of other
>> synchronous traps, it may be unexpectedly exposed to long latencies.
>> Add a simple flag that puts the process into a state where any such
>> kernel entry is fatal.
>>
> To me, this seems like the wrong design. If nothing else, it seems
> too much like an abusable anti-debugging mechanism. I can imagine
> some per-task flag "I think I shouldn't be interrupted now" and a
> tracepoint that fires if the task is interrupted with that flag set.
> But the strong cpu isolation stuff requires systemwide configuration,
> and I think that monitoring that it works should work similarly.
First, you mention a per-task flag, but not specifically whether the
proposed prctl() mechanism is a reasonable way to set that flag.
Just wanted to clarify that this wasn't an issue in and of itself for you.
Second, you suggest a tracepoint. I'm OK with creating a tracepoint
dedicated to cpu_isolated strict failures and making that the only
way this mechanism works. But, earlier community feedback seemed to
suggest that the signal mechanism was OK; one piece of feedback
just requested being able to set which signal was delivered. Do you
think the signal idea is a bad one? Are you proposing potentially
having a signal and/or a tracepoint?
Last, you mention systemwide configuration for monitoring. Can you
expand on what you mean by that? We already support the monitoring
only on the nohz_full cores, so to that extent it's already systemwide.
And the per-task flag has to be set by the running process when it's
ready for this state, so that can't really be systemwide configuration.
I don't understand your suggestion on this point.
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index d882b833dbdb..7315b1579cbd 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -1150,6 +1150,10 @@ static void tracehook_report_syscall(struct pt_regs *regs,
>>
>> asmlinkage int syscall_trace_enter(struct pt_regs *regs)
>> {
>> + /* Ensure we report cpu_isolated violations in all circumstances. */
>> + if (test_thread_flag(TIF_NOHZ) && tick_nohz_cpu_isolated_strict())
>> + tick_nohz_cpu_isolated_syscall(regs->syscallno);
> IMO this is pointless. If a user wants a syscall to kill them, use
> seccomp. The kernel isn't at fault if the user does a syscall when it
> didn't want to enter the kernel.
Interesting! I didn't realize how close SECCOMP_SET_MODE_STRICT
was to what I wanted here. One concern is that there doesn't seem
to be a way to "escape" from seccomp strict mode, i.e. you can't
call seccomp() again to turn it off - which makes sense for seccomp
since it's a security issue, but not so much sense with cpu_isolated.
So, do you think there's a good role for the seccomp() API to play
in achieving this goal? It's certainly not a question of "the kernel at
fault" but rather "asking the kernel to help catch user mistakes"
(typically third-party libraries in our customers' experience). You
could imagine a SECCOMP_SET_MODE_ISOLATED or something.
Alternatively, we could stick with the API proposed in my patch
series, or something similar, and just try to piggy-back on the seccomp
internals to make it happen. It would require Kconfig to ensure
that SECCOMP was enabled though, which obviously isn't currently
required to do cpu isolation.
>> @@ -35,8 +36,12 @@ static inline enum ctx_state exception_enter(void)
>> return 0;
>>
>> prev_ctx = this_cpu_read(context_tracking.state);
>> - if (prev_ctx != CONTEXT_KERNEL)
>> - context_tracking_exit(prev_ctx);
>> + if (prev_ctx != CONTEXT_KERNEL) {
>> + if (context_tracking_exit(prev_ctx)) {
>> + if (tick_nohz_cpu_isolated_strict())
>> + tick_nohz_cpu_isolated_exception();
>> + }
>> + }
> NACK. I'm cautiously optimistic that an x86 kernel 4.3 or newer will
> simply never call exception_enter. It certainly won't call it
> frequently unless something goes wrong with the patches that are
> already in -tip.
This is intended to catch user exceptions like page faults, GPV or
(on platforms where this would happen) unaligned data traps.
The kernel still has a role to play here and cpu_isolated mode
needs to let the user know they have accidentally entered
the kernel in this case.
>> --- a/kernel/context_tracking.c
>> +++ b/kernel/context_tracking.c
>> @@ -147,15 +147,16 @@ NOKPROBE_SYMBOL(context_tracking_user_enter);
>> * This call supports re-entrancy. This way it can be called from any exception
>> * handler without needing to know if we came from userspace or not.
>> */
>> -void context_tracking_exit(enum ctx_state state)
>> +bool context_tracking_exit(enum ctx_state state)
>> {
>> unsigned long flags;
>> + bool from_user = false;
>>
> IMO the internal context tracking API (e.g. context_tracking_exit) are
> mostly of the form "hey context tracking: I don't really know what
> you're doing or what I'm doing, but let me call you and make both of
> us feel better." You're making it somewhat worse: now it's all of the
> above plus "I don't even know whether I just entered the kernel --
> maybe you have a better idea".
>
> Starting with 4.3, x86 kernels will know *exactly* when they enter the
> kernel. All of this context tracking what-was-my-previous-state stuff
> will remain until someone kills it, but when it goes away we'll get a
> nice performance boost.
>
> So, no, let's implement this for real if we're going to implement it.
I'm certainly OK with rebasing on top of 4.3 after the context
tracking stuff is better. That said, I think it makes sense to continue
to debate the intent of the patch series even if we pull this one
patch out and defer it until after 4.3, or having it end up pulled
into some other repo that includes the improvements and
is being pulled for 4.3.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
^ permalink raw reply
* Re: [PATCH v4 1/5] nohz_full: add support for "cpu_isolated" mode
From: Andy Lutomirski @ 2015-07-21 19:26 UTC (permalink / raw)
To: Chris Metcalf, Paul McKenney
Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
Thomas Gleixner, Christoph Lameter, Viresh Kumar,
linux-doc@vger.kernel.org, Linux API,
linux-kernel@vger.kernel.org
In-Reply-To: <55AE993E.6040501@ezchip.com>
On Tue, Jul 21, 2015 at 12:10 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> Sorry for the delay in responding; some other priorities came up internally.
>
> On 07/13/2015 05:45 PM, Andy Lutomirski wrote:
>>
>> On Mon, Jul 13, 2015 at 2:01 PM, Chris Metcalf <cmetcalf@ezchip.com>
>> wrote:
>>>
>>> On 07/13/2015 04:40 PM, Andy Lutomirski wrote:
>>>>
>>>> On Mon, Jul 13, 2015 at 12:57 PM, Chris Metcalf <cmetcalf@ezchip.com>
>>>>
>>>> wrote:
>>>>>
>>>>> The existing nohz_full mode makes tradeoffs to minimize userspace
>>>>> interruptions while still attempting to avoid overheads in the
>>>>> kernel entry/exit path, to provide 100% kernel semantics, etc.
>>>>>
>>>>> However, some applications require a stronger commitment from the
>>>>> kernel to avoid interruptions, in particular userspace device
>>>>> driver style applications, such as high-speed networking code.
>>>>>
>>>>> This change introduces a framework to allow applications to elect
>>>>> to have the stronger semantics as needed, specifying
>>>>> prctl(PR_SET_CPU_ISOLATED, PR_CPU_ISOLATED_ENABLE) to do so.
>>>>> Subsequent commits will add additional flags and additional
>>>>> semantics.
>>>>
>>>> I thought the general consensus was that this should be the default
>>>> behavior and that any associated bugs should be fixed.
>>>
>>>
>>> I think it comes down to dividing the set of use cases in two:
>>>
>>> - "Regular" nohz_full, as used to improve performance and limit
>>> interruptions, possibly for power benefits, etc. But, stray
>>> interrupts are not particularly bad, and you don't want to take
>>> extreme measures to avoid them.
>>>
>>> - What I'm calling "cpu_isolated" mode where when you return to
>>> userspace, you expect that by God, the kernel doesn't interrupt you
>>> again, and if it does, it's a flat-out bug.
>>>
>>> There are a few things that cpu_isolated mode currently does to
>>> accomplish its goals that are pretty heavy-weight:
>>>
>>> Processes are held in kernel space until ticks are quiesced; this is
>>> not necessarily what every nohz_full task wants. If a task makes a
>>> kernel call, there may well be arbitrary timer fallout, and having a
>>> way to select whether or not you are willing to take a timer tick after
>>> return to userspace is pretty important.
>>
>> Then shouldn't deferred work be done immediately in nohz_full mode
>> regardless? What is this delayed work that's being done?
>
>
> I'm thinking of things like needing to wait for an RCU quiesce
> period to complete.
rcu_nocbs does this, right?
>
> In the current version, there's also the vmstat_update() that
> may schedule delayed work and interrupt the core again
> shortly before realizing that there are no more counter updates
> happening, at which point it quiesces. Currently we handle
> this in cpu_isolated mode simply by spinning and waiting for
> the timer interrupts to complete.
Perhaps we should fix that?
>
>>> Likewise, there are things that you may want to do on return to
>>> userspace that are designed to prevent further interruptions in
>>> cpu_isolated mode, even at a possible future performance cost if and
>>> when you return to the kernel, such as flushing the per-cpu free page
>>> list so that you won't be interrupted by an IPI to flush it later.
>>
>> Why not just kick the per-cpu free page over to whatever cpu is
>> monitoring your RCU state, etc? That should be very quick.
>
>
> So just for the sake of precision, the thing I'm talking about
> is the lru_add_drain() call on kernel exit. Are you proposing
> that we call that for every nohz_full core on kernel exit?
> I'm not opposed to this, but I don't know if other nohz
> developers feel like this is the right tradeoff.
I'm proposing either that we do that or that we arrange for other cpus
to be able to steal our LRU list while we're in RCU user/idle.
>> Let's fix them instead of adding new ABIs to work around them.
>
>
> Well, in principle if we accepted my proposed patch series
> and then over time came to decide that it was reasonable
> for nohz_full to have these complete cpu isolation
> semantics, the one proposed ABI simply becomes a no-op.
> So it's not as problematic an ABI as some.
What if we made it a debugfs thing instead of a prctl? Have a mode
where the system tries really hard to quiesce itself even at the cost
of performance.
>
> My issue is this: I'm totally happy with submitting a revised
> patch series that does all the stuff for pure nohz_full that
> I'm currently proposing for cpu_isolated. But, is it what
> the community wants? Should I propose it and see?
>
> Frederic, do you have any insight here? Thanks!
>
> --
> Chris Metcalf, EZChip Semiconductor
> http://www.ezchip.com
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [RFC] Input: Add ps2emu module
From: Greg KH @ 2015-07-21 19:16 UTC (permalink / raw)
To: Stephen Chandler Paul
Cc: Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab,
Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
Sebastian Ott, linux-doc, linux-kernel, linux-input, linux-api,
Benjamin Tissoires, Hans de Goede
In-Reply-To: <1437505634-8633-2-git-send-email-cpaul@redhat.com>
On Tue, Jul 21, 2015 at 03:07:14PM -0400, Stephen Chandler Paul wrote:
> Debugging input devices, specifically laptop touchpads, can be tricky
> without having the physical device handy. Here we try to remedy that
> with ps2emu. This module allows an application to connect to a character
> device provided by the kernel, and simulate any PS/2 device. In
> combination with userspace programs that can record PS/2 devices and
> replay them through the /dev/ps2emu device, this allows developers to
> debug driver issues on the PS/2 level with devices simply by requesting
> a recording from the user experiencing the issue without having to have
> the physical hardware in front of them.
>
> Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> Documentation/input/ps2emu.txt | 72 ++++++++++++
> MAINTAINERS | 6 +
> drivers/input/serio/Kconfig | 10 ++
> drivers/input/serio/Makefile | 1 +
> drivers/input/serio/ps2emu.c | 253 +++++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/ps2emu.h | 42 +++++++
> 6 files changed, 384 insertions(+)
> create mode 100644 Documentation/input/ps2emu.txt
> create mode 100644 drivers/input/serio/ps2emu.c
> create mode 100644 include/uapi/linux/ps2emu.h
>
> diff --git a/Documentation/input/ps2emu.txt b/Documentation/input/ps2emu.txt
> new file mode 100644
> index 0000000..560298c
> --- /dev/null
> +++ b/Documentation/input/ps2emu.txt
> @@ -0,0 +1,72 @@
> + The ps2emu Protocol
> + (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
> + Sponsored by Red Hat
> +--------------------------------------------------------------------------------
> +
> +1. Introduction
> +~~~~~~~~~~~~~~~
> + This module is intended to try to make the lives of input driver developers
> +easier by allowing them to test various PS/2 devices (mainly the various
> +touchpads found on laptops) without having to have the physical device in front
> +of them. ps2emu accomplishes this by allowing any privileged userspace program
> +to directly interact with the kernel's serio driver and pretend to be a PS/2
> +device.
> +
> +2. Usage overview
> +~~~~~~~~~~~~~~~~~
> + In order to interact with the ps2emu kernel module, one simply opens the
> +/dev/ps2emu character device in their applications. Commands are sent to the
> +kernel module by writing to the device, and any data received from the serio
> +driver is read as-is from the /dev/ps2emu device. All of the structures and
> +macros you need to interact with the device are defined in <linux/ps2emu.h>.
> +
> +3. Command Structure
> +~~~~~~~~~~~~~~~~~~~~
> + The struct used for sending commands to /dev/ps2emu is as follows:
> +
> + struct ps2emu_cmd {
> + __u8 type;
> + __u8 data;
> + };
> +
> + "type" describes the type of command that is being sent. This can be any one
> +of the PS2EMU_CMD macros defined in <linux/ps2emu.h>. "data" is the argument
> +that goes along with the command. In the event that the command doesn't have an
> +argument, this field can be left untouched and will be ignored by the kernel.
> +Each command should be sent by writing the struct directly to the character
> +device. In the event that the command you send is invalid, an error will be
> +returned by the character device and a more descriptive error will be printed
> +to the kernel log. Only one command can be sent at a time, any additional data
> +written to the character device after the initial command will be ignored.
> + To close the virtual PS/2 port, just close /dev/ps2emu.
> +
> +4. Commands
> +~~~~~~~~~~~
> +
> +4.1 PS2EMU_CMD_REGISTER
> +~~~~~~~~~~~~~~~~~~~~~~~
> + Registers the port with the serio driver and begins transmitting data back and
> +forth. Registration can only be performed once a port type is set with
> +PS2EMU_CMD_SET_PORT_TYPE. Has no argument.
> +
> +4.2 PS2EMU_CMD_SET_PORT_TYPE
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + Sets the type of port we're emulating, where "data" is the port type being
> +set. Can be any of the following macros from <linux/serio.h>:
> +
> + SERIO_8042
> + SERIO_8042_XL
> + SERIO_PS_PSTHRU
> +
> +4.3 PS2EMU_CMD_SEND_INTERRUPT
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + Sends an interrupt through the virtual PS/2 port to the serio driver, where
> +"data" is the interrupt data being sent.
> +
> +5. Userspace tools
> +~~~~~~~~~~~~~~~~~~
> + The ps2emu userspace tools are able to record PS/2 devices using some of the
> +debugging information from i8042, and play back the devices on /dev/ps2emu. The
> +latest version of these tools can be found at:
> +
> + https://github.com/Lyude/ps2emu
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a226416..68a0977 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10877,6 +10877,12 @@ S: Maintained
> F: drivers/media/v4l2-core/videobuf2-*
> F: include/media/videobuf2-*
>
> +VIRTUAL PS/2 DEVICE DRIVER
> +M: Stephen Chandler Paul <thatslyude@gmail.com>
> +S: Maintained
> +F: drivers/input/serio/ps2emu.c
> +F: include/uapi/linux/ps2emu.h
> +
> VIRTIO CONSOLE DRIVER
> M: Amit Shah <amit.shah@redhat.com>
> L: virtualization@lists.linux-foundation.org
> diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
> index 200841b..cc3563f 100644
> --- a/drivers/input/serio/Kconfig
> +++ b/drivers/input/serio/Kconfig
> @@ -292,4 +292,14 @@ config SERIO_SUN4I_PS2
> To compile this driver as a module, choose M here: the
> module will be called sun4i-ps2.
>
> +config PS2EMU
> + tristate "Virtual PS/2 device support"
> + help
> + Say Y here if you want to emulate PS/2 devices using the ps2emu tools.
> +
> + To compile this driver as a module, choose M here: the module will be
> + called ps2emu.
> +
> + If you are unsure, say N.
> +
> endif
> diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
> index c600089..7b20936 100644
> --- a/drivers/input/serio/Makefile
> +++ b/drivers/input/serio/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2) += apbps2.o
> obj-$(CONFIG_SERIO_OLPC_APSP) += olpc_apsp.o
> obj-$(CONFIG_HYPERV_KEYBOARD) += hyperv-keyboard.o
> obj-$(CONFIG_SERIO_SUN4I_PS2) += sun4i-ps2.o
> +obj-$(CONFIG_PS2EMU) += ps2emu.o
> diff --git a/drivers/input/serio/ps2emu.c b/drivers/input/serio/ps2emu.c
> new file mode 100644
> index 0000000..77b5ca8
> --- /dev/null
> +++ b/drivers/input/serio/ps2emu.c
> @@ -0,0 +1,253 @@
> +/*
> + * ps2emu kernel PS/2 device emulation module
> + * Copyright (C) 2015 Red Hat
> + * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
> + * details.
> + */
> +#include <linux/circ_buf.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/serio.h>
> +#include <linux/libps2.h>
> +#include <linux/slab.h>
> +#include <linux/fs.h>
> +#include <linux/miscdevice.h>
> +#include <linux/sched.h>
> +#include <linux/poll.h>
> +#include <uapi/linux/ps2emu.h>
> +
> +#define PS2EMU_NAME "ps2emu"
> +#define PS2EMU_MINOR MISC_DYNAMIC_MINOR
Don't redefine existing values like this, it makes it hard to review,
just use MISC_DYNAMIC_MINOR.
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC] Input: Add ps2emu module
From: Greg KH @ 2015-07-21 19:15 UTC (permalink / raw)
To: Stephen Chandler Paul
Cc: Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab,
Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
Sebastian Ott, linux-doc, linux-kernel, linux-input, linux-api,
Benjamin Tissoires, Hans de Goede
In-Reply-To: <1437505634-8633-2-git-send-email-cpaul@redhat.com>
On Tue, Jul 21, 2015 at 03:07:14PM -0400, Stephen Chandler Paul wrote:
> +#define ps2emu_warn(format, args...) \
> + dev_warn(ps2emu_misc.this_device, format, ## args)
Don't make a wrapper function for another wrapper function, just spell
the thing out in the code, makes it much easier to debug and maintain
over time.
It will also cause you to rename "this_device" to "dev" to make it
easier to type :)
> +static int ps2emu_char_open(struct inode *inode, struct file *file)
> +{
> + struct ps2emu_device *ps2emu = NULL;
> +
> + ps2emu = kzalloc(sizeof(struct ps2emu_device), GFP_KERNEL);
> + if (!ps2emu)
> + return -ENOMEM;
> +
> + init_waitqueue_head(&ps2emu->waitq);
> +
> + ps2emu->serio.write = ps2emu_device_write;
> + ps2emu->serio.port_data = ps2emu;
> +
> + file->private_data = ps2emu;
> +
> + nonseekable_open(inode, file);
Why call this at all?
> + ret = copy_to_user(buffer, &ps2emu->buf[ps2emu->tail], copylen);
> + if (ret)
> + return ret;
Wrong error value, please return -EFAULT.
> + rc = copy_from_user(&cmd, buffer, sizeof(cmd));
> + if (rc)
> + return rc;
Same thing here, -EFAULT.
> + case PS2EMU_CMD_SEND_INTERRUPT:
> + if (unlikely(!ps2emu->running)) {
Unless you can verify likely/unlikely actually makes a difference in
your code, don't ever use it. Hint, it's not needed here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v4 1/5] nohz_full: add support for "cpu_isolated" mode
From: Chris Metcalf @ 2015-07-21 19:10 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
Viresh Kumar, linux-doc@vger.kernel.org, Linux API,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrUMGx+ZC9rtAAErKaGg-LEtYXMOSVD1dCi7im3b1SMrVg@mail.gmail.com>
Sorry for the delay in responding; some other priorities came up internally.
On 07/13/2015 05:45 PM, Andy Lutomirski wrote:
> On Mon, Jul 13, 2015 at 2:01 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
>> On 07/13/2015 04:40 PM, Andy Lutomirski wrote:
>>> On Mon, Jul 13, 2015 at 12:57 PM, Chris Metcalf <cmetcalf@ezchip.com>
>>> wrote:
>>>> The existing nohz_full mode makes tradeoffs to minimize userspace
>>>> interruptions while still attempting to avoid overheads in the
>>>> kernel entry/exit path, to provide 100% kernel semantics, etc.
>>>>
>>>> However, some applications require a stronger commitment from the
>>>> kernel to avoid interruptions, in particular userspace device
>>>> driver style applications, such as high-speed networking code.
>>>>
>>>> This change introduces a framework to allow applications to elect
>>>> to have the stronger semantics as needed, specifying
>>>> prctl(PR_SET_CPU_ISOLATED, PR_CPU_ISOLATED_ENABLE) to do so.
>>>> Subsequent commits will add additional flags and additional
>>>> semantics.
>>> I thought the general consensus was that this should be the default
>>> behavior and that any associated bugs should be fixed.
>>
>> I think it comes down to dividing the set of use cases in two:
>>
>> - "Regular" nohz_full, as used to improve performance and limit
>> interruptions, possibly for power benefits, etc. But, stray
>> interrupts are not particularly bad, and you don't want to take
>> extreme measures to avoid them.
>>
>> - What I'm calling "cpu_isolated" mode where when you return to
>> userspace, you expect that by God, the kernel doesn't interrupt you
>> again, and if it does, it's a flat-out bug.
>>
>> There are a few things that cpu_isolated mode currently does to
>> accomplish its goals that are pretty heavy-weight:
>>
>> Processes are held in kernel space until ticks are quiesced; this is
>> not necessarily what every nohz_full task wants. If a task makes a
>> kernel call, there may well be arbitrary timer fallout, and having a
>> way to select whether or not you are willing to take a timer tick after
>> return to userspace is pretty important.
> Then shouldn't deferred work be done immediately in nohz_full mode
> regardless? What is this delayed work that's being done?
I'm thinking of things like needing to wait for an RCU quiesce
period to complete.
In the current version, there's also the vmstat_update() that
may schedule delayed work and interrupt the core again
shortly before realizing that there are no more counter updates
happening, at which point it quiesces. Currently we handle
this in cpu_isolated mode simply by spinning and waiting for
the timer interrupts to complete.
>> Likewise, there are things that you may want to do on return to
>> userspace that are designed to prevent further interruptions in
>> cpu_isolated mode, even at a possible future performance cost if and
>> when you return to the kernel, such as flushing the per-cpu free page
>> list so that you won't be interrupted by an IPI to flush it later.
> Why not just kick the per-cpu free page over to whatever cpu is
> monitoring your RCU state, etc? That should be very quick.
So just for the sake of precision, the thing I'm talking about
is the lru_add_drain() call on kernel exit. Are you proposing
that we call that for every nohz_full core on kernel exit?
I'm not opposed to this, but I don't know if other nohz
developers feel like this is the right tradeoff.
Similarly, addressing the vmstat_update() issue above, in
cpu_isolated mode we might want to have a follow-on
patch that forces the vmstat system into quiesced state
on return to userspace. We would need to do this
unconditionally on all nohz_full cores if we tried to combine
the current nohz_full with my proposed cpu_isolated
functionality. Again, I'm not necessarily opposed, but
I suspect other nohz developers might not want this.
(I didn't want to introduce such a patch as part of this
series since it pulls in even more interested parties, and
it gets harder and harder to get to consensus.)
>> If you're arguing that the cpu_isolated semantic is really the only
>> one that makes sense for nohz_full, my sense is that it might be
>> surprising to many of the folks who do nohz_full work. But, I'm happy
>> to be wrong on this point, and maybe all the nohz_full community is
>> interested in making the same tradeoffs for nohz_full generally that
>> I've proposed in this patch series just for cpu_isolated?
> nohz_full is currently dog slow for no particularly good reasons. I
> suspect that the interrupts you're seeing are also there for no
> particularly good reasons as well.
>
> Let's fix them instead of adding new ABIs to work around them.
Well, in principle if we accepted my proposed patch series
and then over time came to decide that it was reasonable
for nohz_full to have these complete cpu isolation
semantics, the one proposed ABI simply becomes a no-op.
So it's not as problematic an ABI as some.
My issue is this: I'm totally happy with submitting a revised
patch series that does all the stuff for pure nohz_full that
I'm currently proposing for cpu_isolated. But, is it what
the community wants? Should I propose it and see?
Frederic, do you have any insight here? Thanks!
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
^ permalink raw reply
* [RFC] Input: Add ps2emu module
From: Stephen Chandler Paul @ 2015-07-21 19:07 UTC (permalink / raw)
To: Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab, Greg KH,
Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
Sebastian Ott, linux-doc, linux-kernel, linux-input, linux-api
Cc: Benjamin Tissoires, Hans de Goede, Stephen Chandler Paul
In-Reply-To: <1437505634-8633-1-git-send-email-cpaul@redhat.com>
Debugging input devices, specifically laptop touchpads, can be tricky
without having the physical device handy. Here we try to remedy that
with ps2emu. This module allows an application to connect to a character
device provided by the kernel, and simulate any PS/2 device. In
combination with userspace programs that can record PS/2 devices and
replay them through the /dev/ps2emu device, this allows developers to
debug driver issues on the PS/2 level with devices simply by requesting
a recording from the user experiencing the issue without having to have
the physical hardware in front of them.
Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Documentation/input/ps2emu.txt | 72 ++++++++++++
MAINTAINERS | 6 +
drivers/input/serio/Kconfig | 10 ++
drivers/input/serio/Makefile | 1 +
drivers/input/serio/ps2emu.c | 253 +++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/ps2emu.h | 42 +++++++
6 files changed, 384 insertions(+)
create mode 100644 Documentation/input/ps2emu.txt
create mode 100644 drivers/input/serio/ps2emu.c
create mode 100644 include/uapi/linux/ps2emu.h
diff --git a/Documentation/input/ps2emu.txt b/Documentation/input/ps2emu.txt
new file mode 100644
index 0000000..560298c
--- /dev/null
+++ b/Documentation/input/ps2emu.txt
@@ -0,0 +1,72 @@
+ The ps2emu Protocol
+ (c) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ Sponsored by Red Hat
+--------------------------------------------------------------------------------
+
+1. Introduction
+~~~~~~~~~~~~~~~
+ This module is intended to try to make the lives of input driver developers
+easier by allowing them to test various PS/2 devices (mainly the various
+touchpads found on laptops) without having to have the physical device in front
+of them. ps2emu accomplishes this by allowing any privileged userspace program
+to directly interact with the kernel's serio driver and pretend to be a PS/2
+device.
+
+2. Usage overview
+~~~~~~~~~~~~~~~~~
+ In order to interact with the ps2emu kernel module, one simply opens the
+/dev/ps2emu character device in their applications. Commands are sent to the
+kernel module by writing to the device, and any data received from the serio
+driver is read as-is from the /dev/ps2emu device. All of the structures and
+macros you need to interact with the device are defined in <linux/ps2emu.h>.
+
+3. Command Structure
+~~~~~~~~~~~~~~~~~~~~
+ The struct used for sending commands to /dev/ps2emu is as follows:
+
+ struct ps2emu_cmd {
+ __u8 type;
+ __u8 data;
+ };
+
+ "type" describes the type of command that is being sent. This can be any one
+of the PS2EMU_CMD macros defined in <linux/ps2emu.h>. "data" is the argument
+that goes along with the command. In the event that the command doesn't have an
+argument, this field can be left untouched and will be ignored by the kernel.
+Each command should be sent by writing the struct directly to the character
+device. In the event that the command you send is invalid, an error will be
+returned by the character device and a more descriptive error will be printed
+to the kernel log. Only one command can be sent at a time, any additional data
+written to the character device after the initial command will be ignored.
+ To close the virtual PS/2 port, just close /dev/ps2emu.
+
+4. Commands
+~~~~~~~~~~~
+
+4.1 PS2EMU_CMD_REGISTER
+~~~~~~~~~~~~~~~~~~~~~~~
+ Registers the port with the serio driver and begins transmitting data back and
+forth. Registration can only be performed once a port type is set with
+PS2EMU_CMD_SET_PORT_TYPE. Has no argument.
+
+4.2 PS2EMU_CMD_SET_PORT_TYPE
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ Sets the type of port we're emulating, where "data" is the port type being
+set. Can be any of the following macros from <linux/serio.h>:
+
+ SERIO_8042
+ SERIO_8042_XL
+ SERIO_PS_PSTHRU
+
+4.3 PS2EMU_CMD_SEND_INTERRUPT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ Sends an interrupt through the virtual PS/2 port to the serio driver, where
+"data" is the interrupt data being sent.
+
+5. Userspace tools
+~~~~~~~~~~~~~~~~~~
+ The ps2emu userspace tools are able to record PS/2 devices using some of the
+debugging information from i8042, and play back the devices on /dev/ps2emu. The
+latest version of these tools can be found at:
+
+ https://github.com/Lyude/ps2emu
diff --git a/MAINTAINERS b/MAINTAINERS
index a226416..68a0977 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10877,6 +10877,12 @@ S: Maintained
F: drivers/media/v4l2-core/videobuf2-*
F: include/media/videobuf2-*
+VIRTUAL PS/2 DEVICE DRIVER
+M: Stephen Chandler Paul <thatslyude@gmail.com>
+S: Maintained
+F: drivers/input/serio/ps2emu.c
+F: include/uapi/linux/ps2emu.h
+
VIRTIO CONSOLE DRIVER
M: Amit Shah <amit.shah@redhat.com>
L: virtualization@lists.linux-foundation.org
diff --git a/drivers/input/serio/Kconfig b/drivers/input/serio/Kconfig
index 200841b..cc3563f 100644
--- a/drivers/input/serio/Kconfig
+++ b/drivers/input/serio/Kconfig
@@ -292,4 +292,14 @@ config SERIO_SUN4I_PS2
To compile this driver as a module, choose M here: the
module will be called sun4i-ps2.
+config PS2EMU
+ tristate "Virtual PS/2 device support"
+ help
+ Say Y here if you want to emulate PS/2 devices using the ps2emu tools.
+
+ To compile this driver as a module, choose M here: the module will be
+ called ps2emu.
+
+ If you are unsure, say N.
+
endif
diff --git a/drivers/input/serio/Makefile b/drivers/input/serio/Makefile
index c600089..7b20936 100644
--- a/drivers/input/serio/Makefile
+++ b/drivers/input/serio/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_SERIO_APBPS2) += apbps2.o
obj-$(CONFIG_SERIO_OLPC_APSP) += olpc_apsp.o
obj-$(CONFIG_HYPERV_KEYBOARD) += hyperv-keyboard.o
obj-$(CONFIG_SERIO_SUN4I_PS2) += sun4i-ps2.o
+obj-$(CONFIG_PS2EMU) += ps2emu.o
diff --git a/drivers/input/serio/ps2emu.c b/drivers/input/serio/ps2emu.c
new file mode 100644
index 0000000..77b5ca8
--- /dev/null
+++ b/drivers/input/serio/ps2emu.c
@@ -0,0 +1,253 @@
+/*
+ * ps2emu kernel PS/2 device emulation module
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Stephen Chandler Paul <thatslyude@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ */
+#include <linux/circ_buf.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/serio.h>
+#include <linux/libps2.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/miscdevice.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <uapi/linux/ps2emu.h>
+
+#define PS2EMU_NAME "ps2emu"
+#define PS2EMU_MINOR MISC_DYNAMIC_MINOR
+#define PS2EMU_BUFSIZE 16
+
+static const struct file_operations ps2emu_fops;
+static struct miscdevice ps2emu_misc;
+
+struct ps2emu_device {
+ struct serio serio;
+
+ bool running;
+
+ u8 head;
+ u8 tail;
+ unsigned char buf[PS2EMU_BUFSIZE];
+
+ wait_queue_head_t waitq;
+};
+
+#define ps2emu_warn(format, args...) \
+ dev_warn(ps2emu_misc.this_device, format, ## args)
+
+/**
+ * ps2emu_device_write - Write data from serio to a ps2emu device in userspace
+ * @id: The serio port for the ps2emu device
+ * @val: The data to write to the device
+ */
+static int ps2emu_device_write(struct serio *id, unsigned char val)
+{
+ struct ps2emu_device *ps2emu = id->port_data;
+ u8 newhead;
+
+ ps2emu->buf[ps2emu->head] = val;
+
+ newhead = ps2emu->head + 1;
+
+ if (newhead < PS2EMU_BUFSIZE)
+ ps2emu->head = newhead;
+ else
+ ps2emu->head = 0;
+
+ if (unlikely(newhead == ps2emu->tail))
+ ps2emu_warn("Buffer overflowed, ps2emu client isn't keeping up");
+
+ wake_up_interruptible(&ps2emu->waitq);
+
+ return 0;
+}
+
+static int ps2emu_char_open(struct inode *inode, struct file *file)
+{
+ struct ps2emu_device *ps2emu = NULL;
+
+ ps2emu = kzalloc(sizeof(struct ps2emu_device), GFP_KERNEL);
+ if (!ps2emu)
+ return -ENOMEM;
+
+ init_waitqueue_head(&ps2emu->waitq);
+
+ ps2emu->serio.write = ps2emu_device_write;
+ ps2emu->serio.port_data = ps2emu;
+
+ file->private_data = ps2emu;
+
+ nonseekable_open(inode, file);
+
+ return 0;
+}
+
+static int ps2emu_char_release(struct inode *inode, struct file *file)
+{
+ struct ps2emu_device *ps2emu = file->private_data;
+
+ /*
+ * We can rely on serio_unregister_port() to free the ps2emu struct on
+ * it's own
+ */
+ if (ps2emu->running)
+ serio_unregister_port(&ps2emu->serio);
+ else
+ kfree(ps2emu);
+
+ return 0;
+}
+
+static ssize_t ps2emu_char_read(struct file *file, char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ps2emu_device *ps2emu = file->private_data;
+ int ret;
+ size_t nonwrap_len, copylen;
+ u8 head; /* So we only access ps2emu->head once */
+
+ if (file->f_flags & O_NONBLOCK) {
+ head = ps2emu->head;
+
+ if (head == ps2emu->tail)
+ return -EAGAIN;
+ } else {
+ ret = wait_event_interruptible(
+ ps2emu->waitq, (head = ps2emu->head) != ps2emu->tail);
+
+ if (ret)
+ return ret;
+ }
+
+ nonwrap_len = CIRC_CNT_TO_END(head, ps2emu->tail, PS2EMU_BUFSIZE);
+ copylen = min(nonwrap_len, count);
+
+ ret = copy_to_user(buffer, &ps2emu->buf[ps2emu->tail], copylen);
+ if (ret)
+ return ret;
+
+ ps2emu->tail += copylen;
+ if (ps2emu->tail == PS2EMU_BUFSIZE)
+ ps2emu->tail = 0;
+
+ return copylen;
+}
+
+static ssize_t ps2emu_char_write(struct file *file, const char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ps2emu_device *ps2emu = file->private_data;
+ struct ps2emu_cmd cmd;
+ int rc;
+
+ if (count < sizeof(cmd))
+ return -EINVAL;
+
+ rc = copy_from_user(&cmd, buffer, sizeof(cmd));
+ if (rc)
+ return rc;
+
+ switch (cmd.type) {
+ case PS2EMU_CMD_REGISTER:
+ if (!ps2emu->serio.id.type) {
+ ps2emu_warn("No port type given on /dev/ps2emu\n");
+
+ return -EINVAL;
+ }
+ if (ps2emu->running) {
+ ps2emu_warn("Begin command sent, but we're already running\n");
+
+ return -EINVAL;
+ }
+
+ ps2emu->running = true;
+ serio_register_port(&ps2emu->serio);
+ break;
+
+ case PS2EMU_CMD_SET_PORT_TYPE:
+ if (ps2emu->running) {
+ ps2emu_warn("Can't change port type on an already running ps2emu instance\n");
+
+ return -EINVAL;
+ }
+
+ switch (cmd.data) {
+ case SERIO_8042:
+ case SERIO_8042_XL:
+ case SERIO_PS_PSTHRU:
+ ps2emu->serio.id.type = cmd.data;
+ break;
+
+ default:
+ ps2emu_warn("Invalid port type 0x%hhx\n", cmd.data);
+
+ return -EINVAL;
+ }
+
+ break;
+
+ case PS2EMU_CMD_SEND_INTERRUPT:
+ if (unlikely(!ps2emu->running)) {
+ ps2emu_warn("The device must be registered before sending interrupts\n");
+
+ return -EINVAL;
+ }
+
+ serio_interrupt(&ps2emu->serio, cmd.data, 0);
+
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return sizeof(cmd);
+}
+
+static unsigned int ps2emu_char_poll(struct file *file, poll_table *wait)
+{
+ struct ps2emu_device *ps2emu = file->private_data;
+
+ poll_wait(file, &ps2emu->waitq, wait);
+
+ if (ps2emu->head != ps2emu->tail)
+ return POLLIN | POLLRDNORM;
+
+ return 0;
+}
+
+static const struct file_operations ps2emu_fops = {
+ .owner = THIS_MODULE,
+ .open = ps2emu_char_open,
+ .release = ps2emu_char_release,
+ .read = ps2emu_char_read,
+ .write = ps2emu_char_write,
+ .poll = ps2emu_char_poll,
+ .llseek = no_llseek,
+};
+
+static struct miscdevice ps2emu_misc = {
+ .fops = &ps2emu_fops,
+ .minor = PS2EMU_MINOR,
+ .name = PS2EMU_NAME,
+};
+
+MODULE_AUTHOR("Stephen Chandler Paul <thatslyude@gmail.com>");
+MODULE_DESCRIPTION("ps2emu");
+MODULE_LICENSE("GPL");
+
+module_driver(ps2emu_misc, misc_register, misc_deregister);
diff --git a/include/uapi/linux/ps2emu.h b/include/uapi/linux/ps2emu.h
new file mode 100644
index 0000000..63f5cc9
--- /dev/null
+++ b/include/uapi/linux/ps2emu.h
@@ -0,0 +1,42 @@
+/*
+ * ps2emu.h
+ * Copyright (C) 2015 Red Hat
+ * Copyright (C) 2015 Lyude (Stephen Chandler Paul) <cpaul@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more
+ * details.
+ *
+ * This is the public header used for user-space communication with the ps2emu
+ * driver. __attribute__((__packed__)) is used for all structs to keep ABI
+ * compatibility between all architectures.
+ */
+
+#ifndef _PS2EMU_H
+#define _PS2EMU_H
+
+#include <linux/types.h>
+
+#define PS2EMU_CMD_REGISTER 0
+#define PS2EMU_CMD_SET_PORT_TYPE 1
+#define PS2EMU_CMD_SEND_INTERRUPT 2
+
+/*
+ * ps2emu Commands
+ * All commands sent to /dev/ps2emu are encoded using this structure. The type
+ * field should contain a PS2EMU_CMD* value that indicates what kind of command
+ * is being sent to ps2emu. The data field should contain the accompanying
+ * argument for the command, if there is one.
+ */
+struct ps2emu_cmd {
+ __u8 type;
+ __u8 data;
+} __attribute__((__packed__));
+
+#endif /* !_PS2EMU_H */
--
2.4.3
^ permalink raw reply related
* [RFC] ps2emu - PS/2 emulation module
From: Stephen Chandler Paul @ 2015-07-21 19:07 UTC (permalink / raw)
To: Dmitry Torokhov, Andrew Morton, Mauro Carvalho Chehab, Greg KH,
Arnd Bergmann, Joe Perches, Jiri Slaby, Vishnu Patekar,
Sebastian Ott, linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
Cc: Benjamin Tissoires, Hans de Goede
Hi! So, following this is a patch to add the ps2emu module to the kernel. This
module basically allows for us to create virtual PS/2 devices and control them
from userspace. With this, we can do useful things such as playing back
recordings of PS/2 devices in a similar manner to evemu-replay and hid-replay.
This lets us debug PS/2 devices that we may not have physical access to, such
as laptop touchpads. This is a huge help when we receive bug reports for devices that
we can't get access to. We've already used this module to assist with fixing
bugs[1] where we needed to be able to replicate the device on a lower level then
we could with evemu-replay or hid-replay. This module could also come into use
when trying to debug the initialization process of devices, along with testing
for regressions with various devices.
The module itself creates a character device at /dev/ps2emu, which userspace
applications can connect to and use to send/receive data straight through the
serio driver. It doesn't need to do much more then that, so it's not a very
large driver. There is a very basic command protocol used for this which has
room for potentially being extended upon in the future if the situation calls
for it.
Right now we make use of this module with the ps2emu userland tools[2] that I've
wrote recently. Recording of PS/2 devices is done by enabling the debugging
output from i8042, triggering a rescan of the PS/2 ports, and doing some clever
trimming of data to remove anything that goes across the i8042 chip that isn't
relevant to the PS/2 protocol. The replay application is pretty simple, and just
sends all of the events from the log through /dev/ps2emu. No additional
processing of the log has to be done, making the replay process very trivial so
long as the behavior of the driver in question is the same as it was when the
recording was done. This also means any bugs encountered during the recording
are easy to reproduce when we play the device back, and we don't need to be
concerned about a virtual device potentially exhibiting different behaviors in
the kernel than the physical one.
If you have any questions or comments, please don't hesitate.
Cheers,
Stephen Chandler Paul
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1235175
[2] https://github.com/Lyude/ps2emu/releases/tag/v0.1.2
Stephen Chandler Paul (1):
Input: Add ps2emu module
Documentation/input/ps2emu.txt | 72 ++++++++++++
MAINTAINERS | 6 +
drivers/input/serio/Kconfig | 10 ++
drivers/input/serio/Makefile | 1 +
drivers/input/serio/ps2emu.c | 251 +++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/ps2emu.h | 42 +++++++
6 files changed, 382 insertions(+)
create mode 100644 Documentation/input/ps2emu.txt
create mode 100644 drivers/input/serio/ps2emu.c
create mode 100644 include/uapi/linux/ps2emu.h
--
2.4.3
^ permalink raw reply
* Re: [PATCH v8 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
From: Srinivas Kandagatla @ 2015-07-21 18:51 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-arm-kernel, Greg Kroah-Hartman, Rob Herring, Mark Brown,
s.hauer, linux-api, linux-kernel, devicetree, linux-arm-msm, arnd,
pantelis.antoniou, mporter, stefan.wahren, wxt, Maxime Ripard
In-Reply-To: <55AE8864.6020608@codeaurora.org>
On 21/07/15 18:59, Stephen Boyd wrote:
> On 07/21/2015 02:41 AM, Srinivas Kandagatla wrote:
>> Thanks Stephen for review,
>>
>> On 20/07/15 22:11, Stephen Boyd wrote:
>>> On 07/20/2015 07:43 AM, Srinivas Kandagatla wrote:
>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>> new file mode 100644
>>>> index 0000000..bde5528
>>>> --- /dev/null
>>>> +++ b/drivers/nvmem/core.c
>>>> @@ -0,0 +1,384 @@
>>>>
>>>> +
>>>> +static int nvmem_add_cells(struct nvmem_device *nvmem,
>>>> + const struct nvmem_config *cfg)
>>>> +{
>>>> + struct nvmem_cell **cells;
>>>> + const struct nvmem_cell_info *info = cfg->cells;
>>>> + int i, rval;
>>>> +
>>>> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
>>>
>>> kcalloc?
>>
>> Only reason for using kzalloc is to give the code more flexibility to
>> free any pointer in the array in case of errors.
>
> Still lost. The arrays are allocated down below in the for loop. This is
> allocating a bunch of pointers so using kcalloc() here avoids problems
> with overflows causing kzalloc() to allocate fewer pointers than
> requested. I'm not suggesting we replace the for loop with a kcalloc,
> just this single line.
My bad, I think I miss understood your suggestion, Yes, we can allocate
pointers using kzalloc.
--srini
>
>>
>>>
>>>> + if (!cells)
>>>> + return -ENOMEM;
>>>> +
>>>> + for (i = 0; i < cfg->ncells; i++) {
>>>> + cells[i] = kzalloc(sizeof(**cells), GFP_KERNEL);
>>>> + if (!cells[i]) {
>>>> + rval = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>
^ permalink raw reply
* Re: [PATCH v2 3/3] console_codes.4: Add CSI sequence for cursor blink interval
From: Scot Doyle @ 2015-07-21 18:45 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: lkml, linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Linux API, linux-man,
Pavel Machek
In-Reply-To: <CAKgNAkgqTckfydFTDFsoqB1b_fAEiDZGYP=7b-bhN8d4Mp-bsg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 21 Jul 2015, Michael Kerrisk (man-pages) wrote:
> On 5 July 2015 at 19:41, Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org> wrote:
> > On Thu, 26 Mar 2015, Scot Doyle wrote:
> >> Add a Console Private CSI sequence to specify the current console's
> >> cursor blink interval. The interval is specified as a number of
> >> milliseconds until the next cursor display state toggle, from 50 to
> >> 65535.
> >>
> >> Signed-off-by: Scot Doyle <lkml14-enLWO88E2pdl57MIdRCFDg@public.gmane.org>
>
> I've applied this, adding Pavel's Acked-by.
>
> I also added some text to note that this appeared in Linux 4.2. Okay?
Yes, thank you.
--
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
* Re: [PATCH v8 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
From: Srinivas Kandagatla @ 2015-07-21 18:40 UTC (permalink / raw)
To: Stephen Boyd
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Greg Kroah-Hartman, Rob Herring, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w, stefan.wahren-eS4NqCHxEME,
wxt-TNX95d0MmH7DzftRWevZcw, Maxime Ripard
In-Reply-To: <55AE8864.6020608-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 21/07/15 18:59, Stephen Boyd wrote:
> On 07/21/2015 02:41 AM, Srinivas Kandagatla wrote:
>> Thanks Stephen for review,
>>
>> On 20/07/15 22:11, Stephen Boyd wrote:
>>> On 07/20/2015 07:43 AM, Srinivas Kandagatla wrote:
>>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>>> new file mode 100644
>>>> index 0000000..bde5528
>>>> --- /dev/null
>>>> +++ b/drivers/nvmem/core.c
>>>> @@ -0,0 +1,384 @@
>>>>
>>>> +
>>>> +static int nvmem_add_cells(struct nvmem_device *nvmem,
>>>> + const struct nvmem_config *cfg)
>>>> +{
>>>> + struct nvmem_cell **cells;
>>>> + const struct nvmem_cell_info *info = cfg->cells;
>>>> + int i, rval;
>>>> +
>>>> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
>>>
>>> kcalloc?
>>
>> Only reason for using kzalloc is to give the code more flexibility to
>> free any pointer in the array in case of errors.
>
> Still lost. The arrays are allocated down below in the for loop. This is
> allocating a bunch of pointers so using kcalloc() here avoids problems
> with overflows causing kzalloc() to allocate fewer pointers than
> requested. I'm not suggesting we replace the for loop with a kcalloc,
> just this single line.
>
Yes we could replace the loop with kcalloc, but the problem is how can
we handle freeing an element from that array?
AFAIK we can only free the full array rather than each element if we
allocate it via kcalloc, correct me if Am wrong?
>>
>>>
>>>> + if (!cells)
>>>> + return -ENOMEM;
>>>> +
>>>> + for (i = 0; i < cfg->ncells; i++) {
>>>> + cells[i] = kzalloc(sizeof(**cells), GFP_KERNEL);
>>>> + if (!cells[i]) {
>>>> + rval = -ENOMEM;
>>>> + goto err;
>>>> + }
>>>> +
>
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-21 18:18 UTC (permalink / raw)
To: Ondřej Bílka
Cc: Linus Torvalds, Andy Lutomirski, Ben Maurer, Ingo Molnar,
libc-alpha, Andrew Morton, linux-api, rostedt, Paul E. McKenney,
Florian Weimer, Josh Triplett, Lai Jiangshan, Paul Turner,
Andrew Hunter, Peter Zijlstra
In-Reply-To: <20150721180051.GA24053@domone>
----- On Jul 21, 2015, at 2:00 PM, Ondřej Bílka neleai@seznam.cz wrote:
> On Tue, Jul 21, 2015 at 05:45:26PM +0000, Mathieu Desnoyers wrote:
>> ----- On Jul 21, 2015, at 11:16 AM, Ondřej Bílka neleai@seznam.cz wrote:
>>
>> > On Tue, Jul 21, 2015 at 12:58:13PM +0000, Mathieu Desnoyers wrote:
>> >> ----- On Jul 21, 2015, at 3:30 AM, Ondřej Bílka neleai@seznam.cz wrote:
>> >>
>> >> > On Tue, Jul 21, 2015 at 12:25:00AM +0000, Mathieu Desnoyers wrote:
>> >> >> >> Does it solve the Wine problem? If Wine uses gs for something and
>> >> >> >> calls a function that does this, Wine still goes boom, right?
>> >> >> >
>> >> >> > So the advantage of just making a global segment descriptor available
>> >> >> > is that it's not *that* expensive to just save/restore segments. So
>> >> >> > either wine could do it, or any library users would do it.
>> >> >> >
>> >> >> > But anyway, I'm not sure this is a good idea. The advantage of it is
>> >> >> > that the kernel support really is _very_ minimal.
>> >> >>
>> >> >> Considering that we'd at least also want this feature on ARM and
>> >> >> PowerPC 32/64, and that the gs segment selector approach clashes with
>> >> >> existing apps (wine), I'm not sure that implementing a gs segment
>> >> >> selector based approach to cpu number caching would lead to an overall
>> >> >> decrease in complexity if it leads to performance similar to those of
>> >> >> portable approaches.
>> >> >>
>> >> >> I'm perfectly fine with architecture-specific tweaks that lead to
>> >> >> fast-path speedups, but if we have to bite the bullet and implement
>> >> >> an approach based on TLS and registering a memory area at thread start
>> >> >> through a system call on other architectures anyway, it might end up
>> >> >> being less complex to add a new system call on x86 too, especially if
>> >> >> fast path overhead is similar.
>> >> >>
>> >> >> But I'm inclined to think that some aspect of the question eludes me,
>> >> >> especially given the amount of interest generated by the gs-segment
>> >> >> selector approach. What am I missing ?
>> >> >>
>> >> > As I wrote before you don't have to bite bullet as I said before. It
>> >> > suffices to create 128k element array with cpu for each tid, make that
>> >> > mmapable file and userspace could get cpu with nearly same performance
>> >> > without hacks.
>> >>
>> >> I don't see how this would be acceptable on memory-constrained embedded
>> >> systems. They have multiple cores, and performance requirements, so
>> >> having a fast getcpu would be useful there (e.g. telecom industry),
>> >> but they clearly cannot afford a 512kB table per process just for that.
>> >>
>> > Which just means that you need more complicated api and implementation
>> > for that but idea stays same. You would need syscalls
>> > register/deregister_cpuid_idx that would give you index used instead
>> > tid. A kernel would need to handle that many ids could be registered for
>> > each thread and resize mmaped file in syscalls.
>>
>> I feel we're talking past each other here. What I propose is to implement
>> a system call that registers a TLS area. It can be invoked at thread start.
>> The kernel can then keep the current CPU number within that registered
>> area up-to-date. This system call does not care how the TLS is implemented
>> underneath.
>>
>> My understanding is that you are suggesting a way to speed up TLS accesses
>> by creating a table indexed by TID. Although it might lead to interesting
>> speed ups useful when reading the TLS, I don't see how you proposal is
>> useful in addressing the problem of caching the current CPU number (other
>> than possibly speeding up TLS accesses).
>>
>> Or am I missing something fundamental to your proposal ?
>>
> No, I still talk about getting cpu number. My first proposal is that
> kernel allocates table of current cpu numbers accessed by tid. That
> could process mmap and get cpu with cpu_tid_table[tid]. As you said that
> size is problem I replied that you need to be more careful. Instead tid
> you will use different id that you get with say register_cpucache, store
> in tls variable and get cpu with cpu_cid_table[cid]. That decreases
> space used to only threads that use this.
>
> A tls speedup was side remark when you would implement per-cpu page then
> you could speedup tls. As tls access speed and getting tid these are
> equivalent as you could easily implement one with other.
Thanks for the clarification. There is then a fundamental question
I need to ask: what is the upside of going for a dedicated array of
current cpu number values rather than using a TLS variable ?
The main downside I see with the array of cpu number is false sharing
caused by having many current cpu number variables sitting on the same
cache line. It seems like an overall performance loss there.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH v4 10/10] mm: madvise allow remove operation for hugetlbfs
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
Now that we have hole punching support for hugetlbfs, we can
also support the MADV_REMOVE interface to it.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
mm/madvise.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/madvise.c b/mm/madvise.c
index 67d5fe7..fa6479a 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -468,7 +468,7 @@ static long madvise_remove(struct vm_area_struct *vma,
*prev = NULL; /* tell sys_madvise we drop mmap_sem */
- if (vma->vm_flags & (VM_LOCKED | VM_HUGETLB))
+ if (vma->vm_flags & VM_LOCKED)
return -EINVAL;
f = vma->vm_file;
--
2.1.0
--
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 v4 09/10] hugetlbfs: add hugetlbfs_fallocate()
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
This is based on the shmem version, but it has diverged quite
a bit. We have no swap to worry about, nor the new file sealing.
Add synchronication via the fault mutex table to coordinate
page faults, fallocate allocation and fallocate hole punch.
What this allows us to do is move physical memory in and out of
a hugetlbfs file without having it mapped. This also gives us
the ability to support MADV_REMOVE since it is currently
implemented using fallocate(). MADV_REMOVE lets madvise() remove
pages from the middle of a hugetlbfs file, which wasn't possible
before.
hugetlbfs fallocate only operates on whole huge pages.
Based-on code-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
fs/hugetlbfs/inode.c | 158 +++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/hugetlb.h | 3 +
mm/hugetlb.c | 2 +-
3 files changed, 161 insertions(+), 2 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index a974e4b..6e565a4 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -12,6 +12,7 @@
#include <linux/thread_info.h>
#include <asm/current.h>
#include <linux/sched.h> /* remove ASAP */
+#include <linux/falloc.h>
#include <linux/fs.h>
#include <linux/mount.h>
#include <linux/file.h>
@@ -479,6 +480,160 @@ static int hugetlb_vmtruncate(struct inode *inode, loff_t offset)
return 0;
}
+static long hugetlbfs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
+{
+ struct hstate *h = hstate_inode(inode);
+ loff_t hpage_size = huge_page_size(h);
+ loff_t hole_start, hole_end;
+
+ /*
+ * For hole punch round up the beginning offset of the hole and
+ * round down the end.
+ */
+ hole_start = round_up(offset, hpage_size);
+ hole_end = round_down(offset + len, hpage_size);
+
+ if (hole_end > hole_start) {
+ struct address_space *mapping = inode->i_mapping;
+
+ mutex_lock(&inode->i_mutex);
+ i_mmap_lock_write(mapping);
+ if (!RB_EMPTY_ROOT(&mapping->i_mmap))
+ hugetlb_vmdelete_list(&mapping->i_mmap,
+ hole_start >> PAGE_SHIFT,
+ hole_end >> PAGE_SHIFT);
+ i_mmap_unlock_write(mapping);
+ remove_inode_hugepages(inode, hole_start, hole_end);
+ mutex_unlock(&inode->i_mutex);
+ }
+
+ return 0;
+}
+
+static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
+ loff_t len)
+{
+ struct inode *inode = file_inode(file);
+ struct address_space *mapping = inode->i_mapping;
+ struct hstate *h = hstate_inode(inode);
+ struct vm_area_struct pseudo_vma;
+ struct mm_struct *mm = current->mm;
+ loff_t hpage_size = huge_page_size(h);
+ unsigned long hpage_shift = huge_page_shift(h);
+ pgoff_t start, index, end;
+ int error;
+ u32 hash;
+
+ if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
+ return -EOPNOTSUPP;
+
+ if (mode & FALLOC_FL_PUNCH_HOLE)
+ return hugetlbfs_punch_hole(inode, offset, len);
+
+ /*
+ * Default preallocate case.
+ * For this range, start is rounded down and end is rounded up
+ * as well as being converted to page offsets.
+ */
+ start = offset >> hpage_shift;
+ end = (offset + len + hpage_size - 1) >> hpage_shift;
+
+ mutex_lock(&inode->i_mutex);
+
+ /* We need to check rlimit even when FALLOC_FL_KEEP_SIZE */
+ error = inode_newsize_ok(inode, offset + len);
+ if (error)
+ goto out;
+
+ /*
+ * Initialize a pseudo vma that just contains the policy used
+ * when allocating the huge pages. The actual policy field
+ * (vm_policy) is determined based on the index in the loop below.
+ */
+ memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
+ pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
+ pseudo_vma.vm_file = file;
+
+ for (index = start; index < end; index++) {
+ /*
+ * This is supposed to be the vaddr where the page is being
+ * faulted in, but we have no vaddr here.
+ */
+ struct page *page;
+ unsigned long addr;
+ int avoid_reserve = 0;
+
+ cond_resched();
+
+ /*
+ * fallocate(2) manpage permits EINTR; we may have been
+ * interrupted because we are using up too much memory.
+ */
+ if (signal_pending(current)) {
+ error = -EINTR;
+ break;
+ }
+
+ /* Get policy based on index */
+ pseudo_vma.vm_policy =
+ mpol_shared_policy_lookup(&HUGETLBFS_I(inode)->policy,
+ index);
+
+ /* addr is the offset within the file (zero based) */
+ addr = index * hpage_size;
+
+ /* mutex taken here, fault path and hole punch */
+ hash = hugetlb_fault_mutex_hash(h, mm, &pseudo_vma, mapping,
+ index, addr);
+ mutex_lock(&hugetlb_fault_mutex_table[hash]);
+
+ /* See if already present in mapping to avoid alloc/free */
+ page = find_get_page(mapping, index);
+ if (page) {
+ put_page(page);
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+ mpol_cond_put(pseudo_vma.vm_policy);
+ continue;
+ }
+
+ /* Allocate page and add to page cache */
+ page = alloc_huge_page(&pseudo_vma, addr, avoid_reserve);
+ mpol_cond_put(pseudo_vma.vm_policy);
+ if (IS_ERR(page)) {
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+ error = PTR_ERR(page);
+ goto out;
+ }
+ clear_huge_page(page, addr, pages_per_huge_page(h));
+ __SetPageUptodate(page);
+ error = huge_add_to_page_cache(page, mapping, index);
+ if (unlikely(error)) {
+ put_page(page);
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+ goto out;
+ }
+
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+
+ /*
+ * page_put due to reference from alloc_huge_page()
+ * unlock_page because locked by add_to_page_cache()
+ */
+ put_page(page);
+ unlock_page(page);
+ }
+
+ if (!(mode & FALLOC_FL_KEEP_SIZE) && offset + len > inode->i_size)
+ i_size_write(inode, offset + len);
+ inode->i_ctime = CURRENT_TIME;
+ spin_lock(&inode->i_lock);
+ inode->i_private = NULL;
+ spin_unlock(&inode->i_lock);
+out:
+ mutex_unlock(&inode->i_mutex);
+ return error;
+}
+
static int hugetlbfs_setattr(struct dentry *dentry, struct iattr *attr)
{
struct inode *inode = d_inode(dentry);
@@ -790,7 +945,8 @@ const struct file_operations hugetlbfs_file_operations = {
.mmap = hugetlbfs_file_mmap,
.fsync = noop_fsync,
.get_unmapped_area = hugetlb_get_unmapped_area,
- .llseek = default_llseek,
+ .llseek = default_llseek,
+ .fallocate = hugetlbfs_fallocate,
};
static const struct inode_operations hugetlbfs_dir_inode_operations = {
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 657ef26..386dcbf 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -330,6 +330,8 @@ struct huge_bootmem_page {
#endif
};
+struct page *alloc_huge_page(struct vm_area_struct *vma,
+ unsigned long addr, int avoid_reserve);
struct page *alloc_huge_page_node(struct hstate *h, int nid);
struct page *alloc_huge_page_noerr(struct vm_area_struct *vma,
unsigned long addr, int avoid_reserve);
@@ -483,6 +485,7 @@ static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
#else /* CONFIG_HUGETLB_PAGE */
struct hstate {};
+#define alloc_huge_page(v, a, r) NULL
#define alloc_huge_page_node(h, nid) NULL
#define alloc_huge_page_noerr(v, a, r) NULL
#define alloc_bootmem_huge_page(h) NULL
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 9812785..e9acf24 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1729,7 +1729,7 @@ static void vma_end_reservation(struct hstate *h,
(void)__vma_reservation_common(h, vma, addr, VMA_END_RESV);
}
-static struct page *alloc_huge_page(struct vm_area_struct *vma,
+struct page *alloc_huge_page(struct vm_area_struct *vma,
unsigned long addr, int avoid_reserve)
{
struct hugepage_subpool *spool = subpool_vma(vma);
--
2.1.0
--
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 v4 08/10] hugetlbfs: New huge_add_to_page_cache helper routine
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
Currently, there is only a single place where hugetlbfs pages are
added to the page cache. The new fallocate code be adding a second
one, so break the functionality out into its own helper.
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
include/linux/hugetlb.h | 2 ++
mm/hugetlb.c | 27 ++++++++++++++++++---------
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index e7825c9..657ef26 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -333,6 +333,8 @@ struct huge_bootmem_page {
struct page *alloc_huge_page_node(struct hstate *h, int nid);
struct page *alloc_huge_page_noerr(struct vm_area_struct *vma,
unsigned long addr, int avoid_reserve);
+int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
+ pgoff_t idx);
/* arch callback */
int __init alloc_bootmem_huge_page(struct hstate *h);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index efcd58c..9812785 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3377,6 +3377,23 @@ static bool hugetlbfs_pagecache_present(struct hstate *h,
return page != NULL;
}
+int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
+ pgoff_t idx)
+{
+ struct inode *inode = mapping->host;
+ struct hstate *h = hstate_inode(inode);
+ int err = add_to_page_cache(page, mapping, idx, GFP_KERNEL);
+
+ if (err)
+ return err;
+ ClearPagePrivate(page);
+
+ spin_lock(&inode->i_lock);
+ inode->i_blocks += blocks_per_huge_page(h);
+ spin_unlock(&inode->i_lock);
+ return 0;
+}
+
static int hugetlb_no_page(struct mm_struct *mm, struct vm_area_struct *vma,
struct address_space *mapping, pgoff_t idx,
unsigned long address, pte_t *ptep, unsigned int flags)
@@ -3424,21 +3441,13 @@ retry:
set_page_huge_active(page);
if (vma->vm_flags & VM_MAYSHARE) {
- int err;
- struct inode *inode = mapping->host;
-
- err = add_to_page_cache(page, mapping, idx, GFP_KERNEL);
+ int err = huge_add_to_page_cache(page, mapping, idx);
if (err) {
put_page(page);
if (err == -EEXIST)
goto retry;
goto out;
}
- ClearPagePrivate(page);
-
- spin_lock(&inode->i_lock);
- inode->i_blocks += blocks_per_huge_page(h);
- spin_unlock(&inode->i_lock);
} else {
lock_page(page);
if (unlikely(anon_vma_prepare(vma))) {
--
2.1.0
--
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 v4 07/10] mm/hugetlb: alloc_huge_page handle areas hole punched by fallocate
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
Areas hole punched by fallocate will not have entries in the
region/reserve map. However, shared mappings with min_size subpool
reservations may still have reserved pages. alloc_huge_page needs
to handle this special case and do the proper accounting.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
mm/hugetlb.c | 54 +++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 15 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f9d3faf..efcd58c 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1735,34 +1735,58 @@ static struct page *alloc_huge_page(struct vm_area_struct *vma,
struct hugepage_subpool *spool = subpool_vma(vma);
struct hstate *h = hstate_vma(vma);
struct page *page;
- long chg, commit;
+ long map_chg, map_commit;
+ long gbl_chg;
int ret, idx;
struct hugetlb_cgroup *h_cg;
idx = hstate_index(h);
/*
- * Processes that did not create the mapping will have no
- * reserves and will not have accounted against subpool
- * limit. Check that the subpool limit can be made before
- * satisfying the allocation MAP_NORESERVE mappings may also
- * need pages and subpool limit allocated allocated if no reserve
- * mapping overlaps.
+ * Examine the region/reserve map to determine if the process
+ * has a reservation for the page to be allocated. A return
+ * code of zero indicates a reservation exists (no change).
*/
- chg = vma_needs_reservation(h, vma, addr);
- if (chg < 0)
+ map_chg = gbl_chg = vma_needs_reservation(h, vma, addr);
+ if (map_chg < 0)
return ERR_PTR(-ENOMEM);
- if (chg || avoid_reserve)
- if (hugepage_subpool_get_pages(spool, 1) < 0) {
+
+ /*
+ * Processes that did not create the mapping will have no
+ * reserves as indicated by the region/reserve map. Check
+ * that the allocation will not exceed the subpool limit.
+ * Allocations for MAP_NORESERVE mappings also need to be
+ * checked against any subpool limit.
+ */
+ if (map_chg || avoid_reserve) {
+ gbl_chg = hugepage_subpool_get_pages(spool, 1);
+ if (gbl_chg < 0) {
vma_end_reservation(h, vma, addr);
return ERR_PTR(-ENOSPC);
}
+ /*
+ * Even though there was no reservation in the region/reserve
+ * map, there could be reservations associated with the
+ * subpool that can be used. This would be indicated if the
+ * return value of hugepage_subpool_get_pages() is zero.
+ * However, if avoid_reserve is specified we still avoid even
+ * the subpool reservations.
+ */
+ if (avoid_reserve)
+ gbl_chg = 1;
+ }
+
ret = hugetlb_cgroup_charge_cgroup(idx, pages_per_huge_page(h), &h_cg);
if (ret)
goto out_subpool_put;
spin_lock(&hugetlb_lock);
- page = dequeue_huge_page_vma(h, vma, addr, avoid_reserve, chg);
+ /*
+ * glb_chg is passed to indicate whether or not a page must be taken
+ * from the global free pool (global change). gbl_chg == 0 indicates
+ * a reservation exists for the allocation.
+ */
+ page = dequeue_huge_page_vma(h, vma, addr, avoid_reserve, gbl_chg);
if (!page) {
spin_unlock(&hugetlb_lock);
page = alloc_buddy_huge_page(h, NUMA_NO_NODE);
@@ -1778,8 +1802,8 @@ static struct page *alloc_huge_page(struct vm_area_struct *vma,
set_page_private(page, (unsigned long)spool);
- commit = vma_commit_reservation(h, vma, addr);
- if (unlikely(chg > commit)) {
+ map_commit = vma_commit_reservation(h, vma, addr);
+ if (unlikely(map_chg > map_commit)) {
/*
* The page was added to the reservation map between
* vma_needs_reservation and vma_commit_reservation.
@@ -1799,7 +1823,7 @@ static struct page *alloc_huge_page(struct vm_area_struct *vma,
out_uncharge_cgroup:
hugetlb_cgroup_uncharge_cgroup(idx, pages_per_huge_page(h), h_cg);
out_subpool_put:
- if (chg || avoid_reserve)
+ if (map_chg || avoid_reserve)
hugepage_subpool_put_pages(spool, 1);
vma_end_reservation(h, vma, addr);
return ERR_PTR(-ENOSPC);
--
2.1.0
--
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 v4 06/10] mm/hugetlb: vma_has_reserves() needs to handle fallocate hole punch
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
In vma_has_reserves(), the current assumption is that reserves are
always present for shared mappings. However, this will not be the
case with fallocate hole punch. When punching a hole, the present
page will be deleted as well as the region/reserve map entry (and
hence any reservation). vma_has_reserves is passed "chg" which
indicates whether or not a region/reserve map is present. Use
this to determine if reserves are actually present or were removed
via hole punch.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
mm/hugetlb.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index db9caea..f9d3faf 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -803,8 +803,19 @@ static bool vma_has_reserves(struct vm_area_struct *vma, long chg)
}
/* Shared mappings always use reserves */
- if (vma->vm_flags & VM_MAYSHARE)
- return true;
+ if (vma->vm_flags & VM_MAYSHARE) {
+ /*
+ * We know VM_NORESERVE is not set. Therefore, there SHOULD
+ * be a region map for all pages. The only situation where
+ * there is no region map is if a hole was punched via
+ * fallocate. In this case, there really are no reverves to
+ * use. This situation is indicated if chg != 0.
+ */
+ if (chg)
+ return false;
+ else
+ return true;
+ }
/*
* Only the process that called mmap() has reserves for
--
2.1.0
--
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 v4 05/10] hugetlbfs: truncate_hugepages() takes a range of pages
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
Modify truncate_hugepages() to take a range of pages (start, end)
instead of simply start. If an end value of LLONG_MAX is passed,
the current "truncate" functionality is maintained. Existing
callers are modified to pass LLONG_MAX as end of range. By keying
off end == LLONG_MAX, the routine behaves differently for truncate
and hole punch. Page removal is now synchronized with page
allocation via faults by using the fault mutex table. The hole
punch case can experience the rare region_del error and must
handle accordingly.
Add the routine hugetlb_fix_reserve_counts to fix up reserve counts
in the case where region_del returns an error.
Since the routine handles more than just the truncate case, it is
renamed to remove_inode_hugepages(). To be consistent, the routine
truncate_huge_page() is renamed remove_huge_page().
Downstream of remove_inode_hugepages(), the routine
hugetlb_unreserve_pages() is also modified to take a range of pages.
hugetlb_unreserve_pages is modified to detect an error from
region_del and pass it back to the caller.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
fs/hugetlbfs/inode.c | 98 ++++++++++++++++++++++++++++++++++++++++++++-----
include/linux/hugetlb.h | 4 +-
mm/hugetlb.c | 40 ++++++++++++++++++--
3 files changed, 128 insertions(+), 14 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index ed40f56..a974e4b 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -293,26 +293,61 @@ static int hugetlbfs_write_end(struct file *file, struct address_space *mapping,
return -EINVAL;
}
-static void truncate_huge_page(struct page *page)
+static void remove_huge_page(struct page *page)
{
ClearPageDirty(page);
ClearPageUptodate(page);
delete_from_page_cache(page);
}
-static void truncate_hugepages(struct inode *inode, loff_t lstart)
+
+/*
+ * remove_inode_hugepages handles two distinct cases: truncation and hole
+ * punch. There are subtle differences in operation for each case.
+
+ * truncation is indicated by end of range being LLONG_MAX
+ * In this case, we first scan the range and release found pages.
+ * After releasing pages, hugetlb_unreserve_pages cleans up region/reserv
+ * maps and global counts.
+ * hole punch is indicated if end is not LLONG_MAX
+ * In the hole punch case we scan the range and release found pages.
+ * Only when releasing a page is the associated region/reserv map
+ * deleted. The region/reserv map for ranges without associated
+ * pages are not modified.
+ * Note: If the passed end of range value is beyond the end of file, but
+ * not LLONG_MAX this routine still performs a hole punch operation.
+ */
+static void remove_inode_hugepages(struct inode *inode, loff_t lstart,
+ loff_t lend)
{
struct hstate *h = hstate_inode(inode);
struct address_space *mapping = &inode->i_data;
const pgoff_t start = lstart >> huge_page_shift(h);
+ const pgoff_t end = lend >> huge_page_shift(h);
+ struct vm_area_struct pseudo_vma;
struct pagevec pvec;
pgoff_t next;
int i, freed = 0;
+ long lookup_nr = PAGEVEC_SIZE;
+ bool truncate_op = (lend == LLONG_MAX);
+ memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
+ pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
pagevec_init(&pvec, 0);
next = start;
- while (1) {
- if (!pagevec_lookup(&pvec, mapping, next, PAGEVEC_SIZE)) {
+ while (next < end) {
+ /*
+ * Make sure to never grab more pages that we
+ * might possibly need.
+ */
+ if (end - next < lookup_nr)
+ lookup_nr = end - next;
+
+ /*
+ * This pagevec_lookup() may return pages past 'end',
+ * so we must check for page->index > end.
+ */
+ if (!pagevec_lookup(&pvec, mapping, next, lookup_nr)) {
if (next == start)
break;
next = start;
@@ -321,26 +356,69 @@ static void truncate_hugepages(struct inode *inode, loff_t lstart)
for (i = 0; i < pagevec_count(&pvec); ++i) {
struct page *page = pvec.pages[i];
+ u32 hash;
+
+ hash = hugetlb_fault_mutex_hash(h, current->mm,
+ &pseudo_vma,
+ mapping, next, 0);
+ mutex_lock(&hugetlb_fault_mutex_table[hash]);
lock_page(page);
+ if (page->index >= end) {
+ unlock_page(page);
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
+ next = end; /* we are done */
+ break;
+ }
+
+ /*
+ * If page is mapped, it was faulted in after being
+ * unmapped. Do nothing in this race case. In the
+ * normal case page is not mapped.
+ */
+ if (!page_mapped(page)) {
+ bool rsv_on_error = !PagePrivate(page);
+ /*
+ * We must free the huge page and remove
+ * from page cache (remove_huge_page) BEFORE
+ * removing the region/reserve map
+ * (hugetlb_unreserve_pages). In rare out
+ * of memory conditions, removal of the
+ * region/reserve map could fail. Before
+ * free'ing the page, note PagePrivate which
+ * is used in case of error.
+ */
+ remove_huge_page(page);
+ freed++;
+ if (!truncate_op) {
+ if (unlikely(hugetlb_unreserve_pages(
+ inode, next,
+ next + 1, 1)))
+ hugetlb_fix_reserve_counts(
+ inode, rsv_on_error);
+ }
+ }
+
if (page->index > next)
next = page->index;
+
++next;
- truncate_huge_page(page);
unlock_page(page);
- freed++;
+
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
}
huge_pagevec_release(&pvec);
}
- BUG_ON(!lstart && mapping->nrpages);
- hugetlb_unreserve_pages(inode, start, freed);
+
+ if (truncate_op)
+ (void)hugetlb_unreserve_pages(inode, start, LONG_MAX, freed);
}
static void hugetlbfs_evict_inode(struct inode *inode)
{
struct resv_map *resv_map;
- truncate_hugepages(inode, 0);
+ remove_inode_hugepages(inode, 0, LLONG_MAX);
resv_map = (struct resv_map *)inode->i_mapping->private_data;
/* root inode doesn't have the resv_map, so we should check it */
if (resv_map)
@@ -397,7 +475,7 @@ static int hugetlb_vmtruncate(struct inode *inode, loff_t offset)
if (!RB_EMPTY_ROOT(&mapping->i_mmap))
hugetlb_vmdelete_list(&mapping->i_mmap, pgoff, 0);
i_mmap_unlock_write(mapping);
- truncate_hugepages(inode, offset);
+ remove_inode_hugepages(inode, offset, LLONG_MAX);
return 0;
}
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 933da39..e7825c9 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -83,11 +83,13 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
int hugetlb_reserve_pages(struct inode *inode, long from, long to,
struct vm_area_struct *vma,
vm_flags_t vm_flags);
-void hugetlb_unreserve_pages(struct inode *inode, long offset, long freed);
+long hugetlb_unreserve_pages(struct inode *inode, long start, long end,
+ long freed);
int dequeue_hwpoisoned_huge_page(struct page *page);
bool isolate_huge_page(struct page *page, struct list_head *list);
void putback_active_hugepage(struct page *page);
void free_huge_page(struct page *page);
+void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve);
extern struct mutex *hugetlb_fault_mutex_table;
u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
struct vm_area_struct *vma,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index db19f4e..db9caea 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -549,6 +549,28 @@ retry:
}
/*
+ * A rare out of memory error was encountered which prevented removal of
+ * the reserve map region for a page. The huge page itself was free'ed
+ * and removed from the page cache. This routine will adjust the subpool
+ * usage count, and the global reserve count if needed. By incrementing
+ * these counts, the reserve map entry which could not be deleted will
+ * appear as a "reserved" entry instead of simply dangling with incorrect
+ * counts.
+ */
+void hugetlb_fix_reserve_counts(struct inode *inode, bool restore_reserve)
+{
+ struct hugepage_subpool *spool = subpool_inode(inode);
+ long rsv_adjust;
+
+ rsv_adjust = hugepage_subpool_get_pages(spool, 1);
+ if (restore_reserve && rsv_adjust) {
+ struct hstate *h = hstate_inode(inode);
+
+ hugetlb_acct_memory(h, 1);
+ }
+}
+
+/*
* Count and return the number of huge pages in the reserve map
* that intersect with the range [f, t).
*/
@@ -3911,7 +3933,8 @@ out_err:
return ret;
}
-void hugetlb_unreserve_pages(struct inode *inode, long offset, long freed)
+long hugetlb_unreserve_pages(struct inode *inode, long start, long end,
+ long freed)
{
struct hstate *h = hstate_inode(inode);
struct resv_map *resv_map = inode_resv_map(inode);
@@ -3919,8 +3942,17 @@ void hugetlb_unreserve_pages(struct inode *inode, long offset, long freed)
struct hugepage_subpool *spool = subpool_inode(inode);
long gbl_reserve;
- if (resv_map)
- chg = region_del(resv_map, offset, LONG_MAX);
+ if (resv_map) {
+ chg = region_del(resv_map, start, end);
+ /*
+ * region_del() can fail in the rare case where a region
+ * must be split and another region descriptor can not be
+ * allocated. If end == LONG_MAX, it will not fail.
+ */
+ if (chg < 0)
+ return chg;
+ }
+
spin_lock(&inode->i_lock);
inode->i_blocks -= (blocks_per_huge_page(h) * freed);
spin_unlock(&inode->i_lock);
@@ -3931,6 +3963,8 @@ void hugetlb_unreserve_pages(struct inode *inode, long offset, long freed)
*/
gbl_reserve = hugepage_subpool_put_pages(spool, (chg - freed));
hugetlb_acct_memory(h, -gbl_reserve);
+
+ return 0;
}
#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
--
2.1.0
--
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 v4 04/10] hugetlbfs: hugetlb_vmtruncate_list() needs to take a range to delete
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
fallocate hole punch will want to unmap a specific range of pages.
Modify the existing hugetlb_vmtruncate_list() routine to take a
start/end range. If end is 0, this indicates all pages after start
should be unmapped. This is the same as the existing truncate
functionality. Modify existing callers to add 0 as end of range.
Since the routine will be used in hole punch as well as truncate
operations, it is more appropriately renamed to hugetlb_vmdelete_list().
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
fs/hugetlbfs/inode.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 0cf74df..ed40f56 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -349,11 +349,15 @@ static void hugetlbfs_evict_inode(struct inode *inode)
}
static inline void
-hugetlb_vmtruncate_list(struct rb_root *root, pgoff_t pgoff)
+hugetlb_vmdelete_list(struct rb_root *root, pgoff_t start, pgoff_t end)
{
struct vm_area_struct *vma;
- vma_interval_tree_foreach(vma, root, pgoff, ULONG_MAX) {
+ /*
+ * end == 0 indicates that the entire range after
+ * start should be unmapped.
+ */
+ vma_interval_tree_foreach(vma, root, start, end ? end : ULONG_MAX) {
unsigned long v_offset;
/*
@@ -362,13 +366,20 @@ hugetlb_vmtruncate_list(struct rb_root *root, pgoff_t pgoff)
* which overlap the truncated area starting at pgoff,
* and no vma on a 32-bit arch can span beyond the 4GB.
*/
- if (vma->vm_pgoff < pgoff)
- v_offset = (pgoff - vma->vm_pgoff) << PAGE_SHIFT;
+ if (vma->vm_pgoff < start)
+ v_offset = (start - vma->vm_pgoff) << PAGE_SHIFT;
else
v_offset = 0;
- unmap_hugepage_range(vma, vma->vm_start + v_offset,
- vma->vm_end, NULL);
+ if (end) {
+ end = ((end - start) << PAGE_SHIFT) +
+ vma->vm_start + v_offset;
+ if (end > vma->vm_end)
+ end = vma->vm_end;
+ } else
+ end = vma->vm_end;
+
+ unmap_hugepage_range(vma, vma->vm_start + v_offset, end, NULL);
}
}
@@ -384,7 +395,7 @@ static int hugetlb_vmtruncate(struct inode *inode, loff_t offset)
i_size_write(inode, offset);
i_mmap_lock_write(mapping);
if (!RB_EMPTY_ROOT(&mapping->i_mmap))
- hugetlb_vmtruncate_list(&mapping->i_mmap, pgoff);
+ hugetlb_vmdelete_list(&mapping->i_mmap, pgoff, 0);
i_mmap_unlock_write(mapping);
truncate_hugepages(inode, offset);
return 0;
--
2.1.0
--
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 v4 03/10] mm/hugetlb: expose hugetlb fault mutex for use by fallocate
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
hugetlb page faults are currently synchronized by the table of
mutexes (htlb_fault_mutex_table). fallocate code will need to
synchronize with the page fault code when it allocates or
deletes pages. Expose interfaces so that fallocate operations
can be synchronized with page faults. Minor name changes to
be more consistent with other global hugetlb symbols.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
include/linux/hugetlb.h | 5 +++++
mm/hugetlb.c | 20 ++++++++++----------
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 667cf44..933da39 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -88,6 +88,11 @@ int dequeue_hwpoisoned_huge_page(struct page *page);
bool isolate_huge_page(struct page *page, struct list_head *list);
void putback_active_hugepage(struct page *page);
void free_huge_page(struct page *page);
+extern struct mutex *hugetlb_fault_mutex_table;
+u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
+ struct vm_area_struct *vma,
+ struct address_space *mapping,
+ pgoff_t idx, unsigned long address);
#ifdef CONFIG_ARCH_WANT_HUGE_PMD_SHARE
pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a573396..db19f4e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -64,7 +64,7 @@ DEFINE_SPINLOCK(hugetlb_lock);
* prevent spurious OOMs when the hugepage pool is fully utilized.
*/
static int num_fault_mutexes;
-static struct mutex *htlb_fault_mutex_table ____cacheline_aligned_in_smp;
+struct mutex *hugetlb_fault_mutex_table ____cacheline_aligned_in_smp;
/* Forward declaration */
static int hugetlb_acct_memory(struct hstate *h, long delta);
@@ -2484,7 +2484,7 @@ static void __exit hugetlb_exit(void)
}
kobject_put(hugepages_kobj);
- kfree(htlb_fault_mutex_table);
+ kfree(hugetlb_fault_mutex_table);
}
module_exit(hugetlb_exit);
@@ -2517,12 +2517,12 @@ static int __init hugetlb_init(void)
#else
num_fault_mutexes = 1;
#endif
- htlb_fault_mutex_table =
+ hugetlb_fault_mutex_table =
kmalloc(sizeof(struct mutex) * num_fault_mutexes, GFP_KERNEL);
- BUG_ON(!htlb_fault_mutex_table);
+ BUG_ON(!hugetlb_fault_mutex_table);
for (i = 0; i < num_fault_mutexes; i++)
- mutex_init(&htlb_fault_mutex_table[i]);
+ mutex_init(&hugetlb_fault_mutex_table[i]);
return 0;
}
module_init(hugetlb_init);
@@ -3456,7 +3456,7 @@ backout_unlocked:
}
#ifdef CONFIG_SMP
-static u32 fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
+u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
struct vm_area_struct *vma,
struct address_space *mapping,
pgoff_t idx, unsigned long address)
@@ -3481,7 +3481,7 @@ static u32 fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
* For uniprocesor systems we always use a single mutex, so just
* return 0 and avoid the hashing overhead.
*/
-static u32 fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
+u32 hugetlb_fault_mutex_hash(struct hstate *h, struct mm_struct *mm,
struct vm_area_struct *vma,
struct address_space *mapping,
pgoff_t idx, unsigned long address)
@@ -3529,8 +3529,8 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
* get spurious allocation failures if two CPUs race to instantiate
* the same page in the page cache.
*/
- hash = fault_mutex_hash(h, mm, vma, mapping, idx, address);
- mutex_lock(&htlb_fault_mutex_table[hash]);
+ hash = hugetlb_fault_mutex_hash(h, mm, vma, mapping, idx, address);
+ mutex_lock(&hugetlb_fault_mutex_table[hash]);
entry = huge_ptep_get(ptep);
if (huge_pte_none(entry)) {
@@ -3615,7 +3615,7 @@ out_ptl:
put_page(pagecache_page);
}
out_mutex:
- mutex_unlock(&htlb_fault_mutex_table[hash]);
+ mutex_unlock(&hugetlb_fault_mutex_table[hash]);
/*
* Generally it's safe to hold refcount during waiting page lock. But
* here we just wait to defer the next page fault to avoid busy loop and
--
2.1.0
--
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 v4 02/10] mm/hugetlb: add region_del() to delete a specific range of entries
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
fallocate hole punch will want to remove a specific range of pages.
The existing region_truncate() routine deletes all region/reserve
map entries after a specified offset. region_del() will provide
this same functionality if the end of region is specified as LONG_MAX.
Hence, region_del() can replace region_truncate().
Unlike region_truncate(), region_del() can return an error in the
rare case where it can not allocate memory for a region descriptor.
This ONLY happens in the case where an existing region must be split.
Current callers passing LONG_MAX as end of range will never experience
this error and do not need to deal with error handling. Future
callers of region_del() (such as fallocate hole punch) will need to
handle this error.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
mm/hugetlb.c | 122 +++++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 85 insertions(+), 37 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index c3923a1..a573396 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -462,43 +462,90 @@ static void region_abort(struct resv_map *resv, long f, long t)
}
/*
- * Truncate the reserve map at index 'end'. Modify/truncate any
- * region which contains end. Delete any regions past end.
- * Return the number of huge pages removed from the map.
+ * Delete the specified range [f, t) from the reserve map. If the
+ * t parameter is LONG_MAX, this indicates that ALL regions after f
+ * should be deleted. Locate the regions which intersect [f, t)
+ * and either trim, delete or split the existing regions.
+ *
+ * Returns the number of huge pages deleted from the reserve map.
+ * In the normal case, the return value is zero or more. In the
+ * case where a region must be split, a new region descriptor must
+ * be allocated. If the allocation fails, -ENOMEM will be returned.
+ * NOTE: If the parameter t == LONG_MAX, then we will never split
+ * a region and possibly return -ENOMEM. Callers specifying
+ * t == LONG_MAX do not need to check for -ENOMEM error.
*/
-static long region_truncate(struct resv_map *resv, long end)
+static long region_del(struct resv_map *resv, long f, long t)
{
struct list_head *head = &resv->regions;
struct file_region *rg, *trg;
- long chg = 0;
+ struct file_region *nrg = NULL;
+ long del = 0;
+retry:
spin_lock(&resv->lock);
- /* Locate the region we are either in or before. */
- list_for_each_entry(rg, head, link)
- if (end <= rg->to)
+ list_for_each_entry_safe(rg, trg, head, link) {
+ if (rg->to <= f)
+ continue;
+ if (rg->from >= t)
break;
- if (&rg->link == head)
- goto out;
- /* If we are in the middle of a region then adjust it. */
- if (end > rg->from) {
- chg = rg->to - end;
- rg->to = end;
- rg = list_entry(rg->link.next, typeof(*rg), link);
- }
+ if (f > rg->from && t < rg->to) { /* Must split region */
+ /*
+ * Check for an entry in the cache before dropping
+ * lock and attempting allocation.
+ */
+ if (!nrg &&
+ resv->rgn_cache_count > resv->adds_in_progress) {
+ nrg = list_first_entry(&resv->rgn_cache,
+ struct file_region,
+ link);
+ list_del(&nrg->link);
+ resv->rgn_cache_count--;
+ }
- /* Drop any remaining regions. */
- list_for_each_entry_safe(rg, trg, rg->link.prev, link) {
- if (&rg->link == head)
+ if (!nrg) {
+ spin_unlock(&resv->lock);
+ nrg = kmalloc(sizeof(*nrg), GFP_KERNEL);
+ if (!nrg)
+ return -ENOMEM;
+ goto retry;
+ }
+
+ del += t - f;
+
+ /* New entry for end of split region */
+ nrg->from = t;
+ nrg->to = rg->to;
+ INIT_LIST_HEAD(&nrg->link);
+
+ /* Original entry is trimmed */
+ rg->to = f;
+
+ list_add(&nrg->link, &rg->link);
+ nrg = NULL;
break;
- chg += rg->to - rg->from;
- list_del(&rg->link);
- kfree(rg);
+ }
+
+ if (f <= rg->from && t >= rg->to) { /* Remove entire region */
+ del += rg->to - rg->from;
+ list_del(&rg->link);
+ kfree(rg);
+ continue;
+ }
+
+ if (f <= rg->from) { /* Trim beginning of region */
+ del += t - rg->from;
+ rg->from = t;
+ } else { /* Trim end of region */
+ del += rg->to - f;
+ rg->to = f;
+ }
}
-out:
spin_unlock(&resv->lock);
- return chg;
+ kfree(nrg);
+ return del;
}
/*
@@ -649,7 +696,7 @@ void resv_map_release(struct kref *ref)
struct file_region *rg, *trg;
/* Clear out any active regions before we release the map. */
- region_truncate(resv_map, 0);
+ region_del(resv_map, 0, LONG_MAX);
/* ... and any entries left in the cache */
list_for_each_entry_safe(rg, trg, head, link) {
@@ -1574,7 +1621,7 @@ static void return_unused_surplus_pages(struct hstate *h,
/*
- * vma_needs_reservation, vma_commit_reservation and vma_abort_reservation
+ * vma_needs_reservation, vma_commit_reservation and vma_end_reservation
* are used by the huge page allocation routines to manage reservations.
*
* vma_needs_reservation is called to determine if the huge page at addr
@@ -1582,8 +1629,9 @@ static void return_unused_surplus_pages(struct hstate *h,
* needed, the value 1 is returned. The caller is then responsible for
* managing the global reservation and subpool usage counts. After
* the huge page has been allocated, vma_commit_reservation is called
- * to add the page to the reservation map. If the reservation must be
- * aborted instead of committed, vma_abort_reservation is called.
+ * to add the page to the reservation map. If the page allocation fails,
+ * the reservation must be ended instead of committed. vma_end_reservation
+ * is called in such cases.
*
* In the normal case, vma_commit_reservation returns the same value
* as the preceding vma_needs_reservation call. The only time this
@@ -1594,7 +1642,7 @@ static void return_unused_surplus_pages(struct hstate *h,
enum vma_resv_mode {
VMA_NEEDS_RESV,
VMA_COMMIT_RESV,
- VMA_ABORT_RESV,
+ VMA_END_RESV,
};
static long __vma_reservation_common(struct hstate *h,
struct vm_area_struct *vma, unsigned long addr,
@@ -1616,7 +1664,7 @@ static long __vma_reservation_common(struct hstate *h,
case VMA_COMMIT_RESV:
ret = region_add(resv, idx, idx + 1);
break;
- case VMA_ABORT_RESV:
+ case VMA_END_RESV:
region_abort(resv, idx, idx + 1);
ret = 0;
break;
@@ -1642,10 +1690,10 @@ static long vma_commit_reservation(struct hstate *h,
return __vma_reservation_common(h, vma, addr, VMA_COMMIT_RESV);
}
-static void vma_abort_reservation(struct hstate *h,
+static void vma_end_reservation(struct hstate *h,
struct vm_area_struct *vma, unsigned long addr)
{
- (void)__vma_reservation_common(h, vma, addr, VMA_ABORT_RESV);
+ (void)__vma_reservation_common(h, vma, addr, VMA_END_RESV);
}
static struct page *alloc_huge_page(struct vm_area_struct *vma,
@@ -1672,7 +1720,7 @@ static struct page *alloc_huge_page(struct vm_area_struct *vma,
return ERR_PTR(-ENOMEM);
if (chg || avoid_reserve)
if (hugepage_subpool_get_pages(spool, 1) < 0) {
- vma_abort_reservation(h, vma, addr);
+ vma_end_reservation(h, vma, addr);
return ERR_PTR(-ENOSPC);
}
@@ -1720,7 +1768,7 @@ out_uncharge_cgroup:
out_subpool_put:
if (chg || avoid_reserve)
hugepage_subpool_put_pages(spool, 1);
- vma_abort_reservation(h, vma, addr);
+ vma_end_reservation(h, vma, addr);
return ERR_PTR(-ENOSPC);
}
@@ -3367,7 +3415,7 @@ retry:
goto backout_unlocked;
}
/* Just decrements count, does not deallocate */
- vma_abort_reservation(h, vma, address);
+ vma_end_reservation(h, vma, address);
}
ptl = huge_pte_lockptr(h, mm, ptep);
@@ -3516,7 +3564,7 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
goto out_mutex;
}
/* Just decrements count, does not deallocate */
- vma_abort_reservation(h, vma, address);
+ vma_end_reservation(h, vma, address);
if (!(vma->vm_flags & VM_MAYSHARE))
pagecache_page = hugetlbfs_pagecache_page(h,
@@ -3872,7 +3920,7 @@ void hugetlb_unreserve_pages(struct inode *inode, long offset, long freed)
long gbl_reserve;
if (resv_map)
- chg = region_truncate(resv_map, offset);
+ chg = region_del(resv_map, offset, LONG_MAX);
spin_lock(&inode->i_lock);
inode->i_blocks -= (blocks_per_huge_page(h) * freed);
spin_unlock(&inode->i_lock);
--
2.1.0
--
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 v4 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
In-Reply-To: <1437502184-14269-1-git-send-email-mike.kravetz@oracle.com>
fallocate hole punch will want to remove a specific range of
pages. When pages are removed, their associated entries in
the region/reserve map will also be removed. This will break
an assumption in the region_chg/region_add calling sequence.
If a new region descriptor must be allocated, it is done as
part of the region_chg processing. In this way, region_add
can not fail because it does not need to attempt an allocation.
To prepare for fallocate hole punch, create a "cache" of
descriptors that can be used by region_add if necessary.
region_chg will ensure there are sufficient entries in the
cache. It will be necessary to track the number of in progress
add operations to know a sufficient number of descriptors
reside in the cache. A new routine region_abort is added to
adjust this in progress count when add operations are aborted.
vma_abort_reservation is also added for callers creating
reservations with vma_needs_reservation/vma_commit_reservation.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
include/linux/hugetlb.h | 3 +
mm/hugetlb.c | 170 ++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 154 insertions(+), 19 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index d891f94..667cf44 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -35,6 +35,9 @@ struct resv_map {
struct kref refs;
spinlock_t lock;
struct list_head regions;
+ long adds_in_progress;
+ struct list_head rgn_cache;
+ long rgn_cache_count;
};
extern struct resv_map *resv_map_alloc(void);
void resv_map_release(struct kref *ref);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 51ae41d..c3923a1 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -240,11 +240,14 @@ struct file_region {
/*
* Add the huge page range represented by [f, t) to the reserve
- * map. Existing regions will be expanded to accommodate the
- * specified range. We know only existing regions need to be
- * expanded, because region_add is only called after region_chg
- * with the same range. If a new file_region structure must
- * be allocated, it is done in region_chg.
+ * map. In the normal case, existing regions will be expanded
+ * to accommodate the specified range. Sufficient regions should
+ * exist for expansion due to the previous call to region_chg
+ * with the same range. However, it is possible that region_del
+ * could have been called after region_chg and modifed the map
+ * in such a way that no region exists to be expanded. In this
+ * case, pull a region descriptor from the cache associated with
+ * the map and use that for the new range.
*
* Return the number of new huge pages added to the map. This
* number is greater than or equal to zero.
@@ -261,6 +264,28 @@ static long region_add(struct resv_map *resv, long f, long t)
if (f <= rg->to)
break;
+ /*
+ * If no region exists which can be expanded to include the
+ * specified range, the list must have been modified by an
+ * interleving call to region_del(). Pull a region descriptor
+ * from the cache and use it for this range.
+ */
+ if (&rg->link == head || t < rg->from) {
+ VM_BUG_ON(resv->rgn_cache_count <= 0);
+
+ resv->rgn_cache_count--;
+ nrg = list_first_entry(&resv->rgn_cache, struct file_region,
+ link);
+ list_del(&nrg->link);
+
+ nrg->from = f;
+ nrg->to = t;
+ list_add(&nrg->link, rg->link.prev);
+
+ add += t - f;
+ goto out_locked;
+ }
+
/* Round our left edge to the current segment if it encloses us. */
if (f > rg->from)
f = rg->from;
@@ -294,6 +319,8 @@ static long region_add(struct resv_map *resv, long f, long t)
add += t - nrg->to; /* Added to end of region */
nrg->to = t;
+out_locked:
+ resv->adds_in_progress--;
spin_unlock(&resv->lock);
VM_BUG_ON(add < 0);
return add;
@@ -312,11 +339,16 @@ static long region_add(struct resv_map *resv, long f, long t)
* so that the subsequent region_add call will have all the
* regions it needs and will not fail.
*
+ * Upon entry, region_chg will also examine the cache of
+ * region descriptors associated with the map. If there
+ * not enough descriptors cached, one will be allocated
+ * for the in progress add operation.
+ *
* Returns the number of huge pages that need to be added
* to the existing reservation map for the range [f, t).
* This number is greater or equal to zero. -ENOMEM is
- * returned if a new file_region structure is needed and can
- * not be allocated.
+ * returned if a new file_region structure or cache entry
+ * is needed and can not be allocated.
*/
static long region_chg(struct resv_map *resv, long f, long t)
{
@@ -326,6 +358,31 @@ static long region_chg(struct resv_map *resv, long f, long t)
retry:
spin_lock(&resv->lock);
+retry_locked:
+ resv->adds_in_progress++;
+
+ /*
+ * Check for sufficient descriptors in the cache to accommodate
+ * the number of in progress add operations.
+ */
+ if (resv->adds_in_progress > resv->rgn_cache_count) {
+ struct file_region *trg;
+
+ VM_BUG_ON(resv->adds_in_progress - resv->rgn_cache_count > 1);
+ /* Must drop lock to allocate a new descriptor. */
+ resv->adds_in_progress--;
+ spin_unlock(&resv->lock);
+
+ trg = kmalloc(sizeof(*trg), GFP_KERNEL);
+ if (!trg)
+ return -ENOMEM;
+
+ spin_lock(&resv->lock);
+ list_add(&trg->link, &resv->rgn_cache);
+ resv->rgn_cache_count++;
+ goto retry_locked;
+ }
+
/* Locate the region we are before or in. */
list_for_each_entry(rg, head, link)
if (f <= rg->to)
@@ -336,6 +393,7 @@ retry:
* size such that we can guarantee to record the reservation. */
if (&rg->link == head || t < rg->from) {
if (!nrg) {
+ resv->adds_in_progress--;
spin_unlock(&resv->lock);
nrg = kmalloc(sizeof(*nrg), GFP_KERNEL);
if (!nrg)
@@ -385,6 +443,25 @@ out_nrg:
}
/*
+ * Abort the in progress add operation. The adds_in_progress field
+ * of the resv_map keeps track of the operations in progress between
+ * calls to region_chg and region_add. Operations are sometimes
+ * aborted after the call to region_chg. In such cases, region_abort
+ * is called to decrement the adds_in_progress counter.
+ *
+ * NOTE: The range arguments [f, t) are not needed or used in this
+ * routine. They are kept to make reading the calling code easier as
+ * arguments will match the associated region_chg call.
+ */
+static void region_abort(struct resv_map *resv, long f, long t)
+{
+ spin_lock(&resv->lock);
+ VM_BUG_ON(!resv->rgn_cache_count);
+ resv->adds_in_progress--;
+ spin_unlock(&resv->lock);
+}
+
+/*
* Truncate the reserve map at index 'end'. Modify/truncate any
* region which contains end. Delete any regions past end.
* Return the number of huge pages removed from the map.
@@ -544,22 +621,44 @@ static void set_vma_private_data(struct vm_area_struct *vma,
struct resv_map *resv_map_alloc(void)
{
struct resv_map *resv_map = kmalloc(sizeof(*resv_map), GFP_KERNEL);
- if (!resv_map)
+ struct file_region *rg = kmalloc(sizeof(*rg), GFP_KERNEL);
+
+ if (!resv_map || !rg) {
+ kfree(resv_map);
+ kfree(rg);
return NULL;
+ }
kref_init(&resv_map->refs);
spin_lock_init(&resv_map->lock);
INIT_LIST_HEAD(&resv_map->regions);
+ resv_map->adds_in_progress = 0;
+
+ INIT_LIST_HEAD(&resv_map->rgn_cache);
+ list_add(&rg->link, &resv_map->rgn_cache);
+ resv_map->rgn_cache_count = 1;
+
return resv_map;
}
void resv_map_release(struct kref *ref)
{
struct resv_map *resv_map = container_of(ref, struct resv_map, refs);
+ struct list_head *head = &resv_map->rgn_cache;
+ struct file_region *rg, *trg;
/* Clear out any active regions before we release the map. */
region_truncate(resv_map, 0);
+
+ /* ... and any entries left in the cache */
+ list_for_each_entry_safe(rg, trg, head, link) {
+ list_del(&rg->link);
+ kfree(rg);
+ }
+
+ VM_BUG_ON(resv_map->adds_in_progress);
+
kfree(resv_map);
}
@@ -1473,16 +1572,18 @@ static void return_unused_surplus_pages(struct hstate *h,
}
}
+
/*
- * vma_needs_reservation and vma_commit_reservation are used by the huge
- * page allocation routines to manage reservations.
+ * vma_needs_reservation, vma_commit_reservation and vma_abort_reservation
+ * are used by the huge page allocation routines to manage reservations.
*
* vma_needs_reservation is called to determine if the huge page at addr
* within the vma has an associated reservation. If a reservation is
* needed, the value 1 is returned. The caller is then responsible for
* managing the global reservation and subpool usage counts. After
* the huge page has been allocated, vma_commit_reservation is called
- * to add the page to the reservation map.
+ * to add the page to the reservation map. If the reservation must be
+ * aborted instead of committed, vma_abort_reservation is called.
*
* In the normal case, vma_commit_reservation returns the same value
* as the preceding vma_needs_reservation call. The only time this
@@ -1490,9 +1591,14 @@ static void return_unused_surplus_pages(struct hstate *h,
* is the responsibility of the caller to notice the difference and
* take appropriate action.
*/
+enum vma_resv_mode {
+ VMA_NEEDS_RESV,
+ VMA_COMMIT_RESV,
+ VMA_ABORT_RESV,
+};
static long __vma_reservation_common(struct hstate *h,
struct vm_area_struct *vma, unsigned long addr,
- bool commit)
+ enum vma_resv_mode mode)
{
struct resv_map *resv;
pgoff_t idx;
@@ -1503,10 +1609,20 @@ static long __vma_reservation_common(struct hstate *h,
return 1;
idx = vma_hugecache_offset(h, vma, addr);
- if (commit)
- ret = region_add(resv, idx, idx + 1);
- else
+ switch (mode) {
+ case VMA_NEEDS_RESV:
ret = region_chg(resv, idx, idx + 1);
+ break;
+ case VMA_COMMIT_RESV:
+ ret = region_add(resv, idx, idx + 1);
+ break;
+ case VMA_ABORT_RESV:
+ region_abort(resv, idx, idx + 1);
+ ret = 0;
+ break;
+ default:
+ BUG();
+ }
if (vma->vm_flags & VM_MAYSHARE)
return ret;
@@ -1517,13 +1633,19 @@ static long __vma_reservation_common(struct hstate *h,
static long vma_needs_reservation(struct hstate *h,
struct vm_area_struct *vma, unsigned long addr)
{
- return __vma_reservation_common(h, vma, addr, false);
+ return __vma_reservation_common(h, vma, addr, VMA_NEEDS_RESV);
}
static long vma_commit_reservation(struct hstate *h,
struct vm_area_struct *vma, unsigned long addr)
{
- return __vma_reservation_common(h, vma, addr, true);
+ return __vma_reservation_common(h, vma, addr, VMA_COMMIT_RESV);
+}
+
+static void vma_abort_reservation(struct hstate *h,
+ struct vm_area_struct *vma, unsigned long addr)
+{
+ (void)__vma_reservation_common(h, vma, addr, VMA_ABORT_RESV);
}
static struct page *alloc_huge_page(struct vm_area_struct *vma,
@@ -1549,8 +1671,10 @@ static struct page *alloc_huge_page(struct vm_area_struct *vma,
if (chg < 0)
return ERR_PTR(-ENOMEM);
if (chg || avoid_reserve)
- if (hugepage_subpool_get_pages(spool, 1) < 0)
+ if (hugepage_subpool_get_pages(spool, 1) < 0) {
+ vma_abort_reservation(h, vma, addr);
return ERR_PTR(-ENOSPC);
+ }
ret = hugetlb_cgroup_charge_cgroup(idx, pages_per_huge_page(h), &h_cg);
if (ret)
@@ -1596,6 +1720,7 @@ out_uncharge_cgroup:
out_subpool_put:
if (chg || avoid_reserve)
hugepage_subpool_put_pages(spool, 1);
+ vma_abort_reservation(h, vma, addr);
return ERR_PTR(-ENOSPC);
}
@@ -3236,11 +3361,14 @@ retry:
* any allocations necessary to record that reservation occur outside
* the spinlock.
*/
- if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED))
+ if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
if (vma_needs_reservation(h, vma, address) < 0) {
ret = VM_FAULT_OOM;
goto backout_unlocked;
}
+ /* Just decrements count, does not deallocate */
+ vma_abort_reservation(h, vma, address);
+ }
ptl = huge_pte_lockptr(h, mm, ptep);
spin_lock(ptl);
@@ -3387,6 +3515,8 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
ret = VM_FAULT_OOM;
goto out_mutex;
}
+ /* Just decrements count, does not deallocate */
+ vma_abort_reservation(h, vma, address);
if (!(vma->vm_flags & VM_MAYSHARE))
pagecache_page = hugetlbfs_pagecache_page(h,
@@ -3726,6 +3856,8 @@ int hugetlb_reserve_pages(struct inode *inode,
}
return 0;
out_err:
+ if (!vma || vma->vm_flags & VM_MAYSHARE)
+ region_abort(resv_map, from, to);
if (vma && is_vma_resv_set(vma, HPAGE_RESV_OWNER))
kref_put(&resv_map->refs, resv_map_release);
return ret;
--
2.1.0
--
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 v4 00/10] hugetlbfs: add fallocate support
From: Mike Kravetz @ 2015-07-21 18:09 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-api
Cc: Dave Hansen, Naoya Horiguchi, David Rientjes, Hugh Dickins,
Davidlohr Bueso, Aneesh Kumar, Hillf Danton, Christoph Hellwig,
Andrew Morton, Michal Hocko, Mike Kravetz
Changes in this revision address the minor comment and function name
issues brought up by Naoya Horiguchi. Patch set is also rebased on
current "mmotm/since-4.1". This revision does not introduce any
functional changes.
As suggested during the RFC process, tests have been proposed to
libhugetlbfs as described at:
http://librelist.com/browser//libhugetlbfs/2015/6/25/patch-tests-add-tests-for-fallocate-system-call/
fallocate(2) man page modifications are also necessary to specify
that fallocate for hugetlbfs only operates on whole pages. This
change will be submitted once the code has stabilized and been
proposed for merging.
hugetlbfs is used today by applications that want a high degree of
control over huge page usage. Often, large hugetlbfs files are used
to map a large number huge pages into the application processes.
The applications know when page ranges within these large files will
no longer be used, and ideally would like to release them back to
the subpool or global pools for other uses. The fallocate() system
call provides an interface for preallocation and hole punching within
files. This patch set adds fallocate functionality to hugetlbfs.
v4:
Renamed vma_abort_reservation() to vma_end_reservation().
v3:
Fixed issue with region_chg to recheck if there are sufficient
entries in the cache after acquiring lock.
v2:
Fixed leak in resv_map_release discovered by Hillf Danton.
Used LONG_MAX as indicator of truncate function for region_del.
v1:
Add a cache of region descriptors to the resv_map for use by
region_add in case hole punch deletes entries necessary for
a successful operation.
RFC v4:
Removed alloc_huge_page/hugetlb_reserve_pages race patches as already
in mmotm
Moved hugetlb_fix_reserve_counts in series as suggested by Naoya Horiguchi
Inline'ed hugetlb_fault_mutex routines as suggested by Davidlohr Bueso and
existing code changed to use new interfaces as suggested by Naoya
fallocate preallocation code cleaned up and made simpler
Modified alloc_huge_page to handle special case where allocation is
for a hole punched area with spool reserves
RFC v3:
Folded in patch for alloc_huge_page/hugetlb_reserve_pages race
in current code
fallocate allocation and hole punch is synchronized with page
faults via existing mutex table
hole punch uses existing hugetlb_vmtruncate_list instead of more
generic unmap_mapping_range for unmapping
Error handling for the case when region_del() fauils
RFC v2:
Addressed alignment and error handling issues noticed by Hillf Danton
New region_del() routine for region tracking/resv_map of ranges
Fixed several issues found during more extensive testing
Error handling in region_del() when kmalloc() fails stills needs
to be addressed
madvise remove support remains
v3 patch series:
Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>
Mike Kravetz (10):
mm/hugetlb: add cache of descriptors to resv_map for region_add
mm/hugetlb: add region_del() to delete a specific range of entries
mm/hugetlb: expose hugetlb fault mutex for use by fallocate
hugetlbfs: hugetlb_vmtruncate_list() needs to take a range to delete
hugetlbfs: truncate_hugepages() takes a range of pages
mm/hugetlb: vma_has_reserves() needs to handle fallocate hole punch
mm/hugetlb: alloc_huge_page handle areas hole punched by fallocate
hugetlbfs: New huge_add_to_page_cache helper routine
hugetlbfs: add hugetlbfs_fallocate()
mm: madvise allow remove operation for hugetlbfs
fs/hugetlbfs/inode.c | 281 ++++++++++++++++++++++++++++++--
include/linux/hugetlb.h | 17 +-
mm/hugetlb.c | 424 ++++++++++++++++++++++++++++++++++++++----------
mm/madvise.c | 2 +-
4 files changed, 621 insertions(+), 103 deletions(-)
--
2.1.0
--
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: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Ondřej Bílka @ 2015-07-21 18:00 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: Linus Torvalds, Andy Lutomirski, Ben Maurer, Ingo Molnar,
libc-alpha, Andrew Morton, linux-api, rostedt, Paul E. McKenney,
Florian Weimer, Josh Triplett, Lai Jiangshan, Paul Turner,
Andrew Hunter, Peter Zijlstra
In-Reply-To: <1350114812.1035.1437500726799.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>
On Tue, Jul 21, 2015 at 05:45:26PM +0000, Mathieu Desnoyers wrote:
> ----- On Jul 21, 2015, at 11:16 AM, Ondřej Bílka neleai@seznam.cz wrote:
>
> > On Tue, Jul 21, 2015 at 12:58:13PM +0000, Mathieu Desnoyers wrote:
> >> ----- On Jul 21, 2015, at 3:30 AM, Ondřej Bílka neleai@seznam.cz wrote:
> >>
> >> > On Tue, Jul 21, 2015 at 12:25:00AM +0000, Mathieu Desnoyers wrote:
> >> >> >> Does it solve the Wine problem? If Wine uses gs for something and
> >> >> >> calls a function that does this, Wine still goes boom, right?
> >> >> >
> >> >> > So the advantage of just making a global segment descriptor available
> >> >> > is that it's not *that* expensive to just save/restore segments. So
> >> >> > either wine could do it, or any library users would do it.
> >> >> >
> >> >> > But anyway, I'm not sure this is a good idea. The advantage of it is
> >> >> > that the kernel support really is _very_ minimal.
> >> >>
> >> >> Considering that we'd at least also want this feature on ARM and
> >> >> PowerPC 32/64, and that the gs segment selector approach clashes with
> >> >> existing apps (wine), I'm not sure that implementing a gs segment
> >> >> selector based approach to cpu number caching would lead to an overall
> >> >> decrease in complexity if it leads to performance similar to those of
> >> >> portable approaches.
> >> >>
> >> >> I'm perfectly fine with architecture-specific tweaks that lead to
> >> >> fast-path speedups, but if we have to bite the bullet and implement
> >> >> an approach based on TLS and registering a memory area at thread start
> >> >> through a system call on other architectures anyway, it might end up
> >> >> being less complex to add a new system call on x86 too, especially if
> >> >> fast path overhead is similar.
> >> >>
> >> >> But I'm inclined to think that some aspect of the question eludes me,
> >> >> especially given the amount of interest generated by the gs-segment
> >> >> selector approach. What am I missing ?
> >> >>
> >> > As I wrote before you don't have to bite bullet as I said before. It
> >> > suffices to create 128k element array with cpu for each tid, make that
> >> > mmapable file and userspace could get cpu with nearly same performance
> >> > without hacks.
> >>
> >> I don't see how this would be acceptable on memory-constrained embedded
> >> systems. They have multiple cores, and performance requirements, so
> >> having a fast getcpu would be useful there (e.g. telecom industry),
> >> but they clearly cannot afford a 512kB table per process just for that.
> >>
> > Which just means that you need more complicated api and implementation
> > for that but idea stays same. You would need syscalls
> > register/deregister_cpuid_idx that would give you index used instead
> > tid. A kernel would need to handle that many ids could be registered for
> > each thread and resize mmaped file in syscalls.
>
> I feel we're talking past each other here. What I propose is to implement
> a system call that registers a TLS area. It can be invoked at thread start.
> The kernel can then keep the current CPU number within that registered
> area up-to-date. This system call does not care how the TLS is implemented
> underneath.
>
> My understanding is that you are suggesting a way to speed up TLS accesses
> by creating a table indexed by TID. Although it might lead to interesting
> speed ups useful when reading the TLS, I don't see how you proposal is
> useful in addressing the problem of caching the current CPU number (other
> than possibly speeding up TLS accesses).
>
> Or am I missing something fundamental to your proposal ?
>
No, I still talk about getting cpu number. My first proposal is that
kernel allocates table of current cpu numbers accessed by tid. That
could process mmap and get cpu with cpu_tid_table[tid]. As you said that
size is problem I replied that you need to be more careful. Instead tid
you will use different id that you get with say register_cpucache, store
in tls variable and get cpu with cpu_cid_table[cid]. That decreases
space used to only threads that use this.
A tls speedup was side remark when you would implement per-cpu page then
you could speedup tls. As tls access speed and getting tid these are
equivalent as you could easily implement one with other.
^ permalink raw reply
* Re: [PATCH v8 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
From: Stephen Boyd @ 2015-07-21 17:59 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-arm-kernel, Greg Kroah-Hartman, Rob Herring, Mark Brown,
s.hauer, linux-api, linux-kernel, devicetree, linux-arm-msm, arnd,
pantelis.antoniou, mporter, stefan.wahren, wxt, Maxime Ripard
In-Reply-To: <55AE13D8.6020301@linaro.org>
On 07/21/2015 02:41 AM, Srinivas Kandagatla wrote:
> Thanks Stephen for review,
>
> On 20/07/15 22:11, Stephen Boyd wrote:
>> On 07/20/2015 07:43 AM, Srinivas Kandagatla wrote:
>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>> new file mode 100644
>>> index 0000000..bde5528
>>> --- /dev/null
>>> +++ b/drivers/nvmem/core.c
>>> @@ -0,0 +1,384 @@
>>>
>>> +
>>> +static int nvmem_add_cells(struct nvmem_device *nvmem,
>>> + const struct nvmem_config *cfg)
>>> +{
>>> + struct nvmem_cell **cells;
>>> + const struct nvmem_cell_info *info = cfg->cells;
>>> + int i, rval;
>>> +
>>> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
>>
>> kcalloc?
>
> Only reason for using kzalloc is to give the code more flexibility to
> free any pointer in the array in case of errors.
Still lost. The arrays are allocated down below in the for loop. This is
allocating a bunch of pointers so using kcalloc() here avoids problems
with overflows causing kzalloc() to allocate fewer pointers than
requested. I'm not suggesting we replace the for loop with a kcalloc,
just this single line.
>
>>
>>> + if (!cells)
>>> + return -ENOMEM;
>>> +
>>> + for (i = 0; i < cfg->ncells; i++) {
>>> + cells[i] = kzalloc(sizeof(**cells), GFP_KERNEL);
>>> + if (!cells[i]) {
>>> + rval = -ENOMEM;
>>> + goto err;
>>> + }
>>> +
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [RFC PATCH] getcpu_cache system call: caching current CPU number (x86)
From: Mathieu Desnoyers @ 2015-07-21 17:45 UTC (permalink / raw)
To: Ondřej Bílka
Cc: Linus Torvalds, Andy Lutomirski, Ben Maurer, Ingo Molnar,
libc-alpha, Andrew Morton, linux-api, rostedt, Paul E. McKenney,
Florian Weimer, Josh Triplett, Lai Jiangshan, Paul Turner,
Andrew Hunter, Peter Zijlstra
In-Reply-To: <20150721151613.GA12856@domone>
----- On Jul 21, 2015, at 11:16 AM, Ondřej Bílka neleai@seznam.cz wrote:
> On Tue, Jul 21, 2015 at 12:58:13PM +0000, Mathieu Desnoyers wrote:
>> ----- On Jul 21, 2015, at 3:30 AM, Ondřej Bílka neleai@seznam.cz wrote:
>>
>> > On Tue, Jul 21, 2015 at 12:25:00AM +0000, Mathieu Desnoyers wrote:
>> >> >> Does it solve the Wine problem? If Wine uses gs for something and
>> >> >> calls a function that does this, Wine still goes boom, right?
>> >> >
>> >> > So the advantage of just making a global segment descriptor available
>> >> > is that it's not *that* expensive to just save/restore segments. So
>> >> > either wine could do it, or any library users would do it.
>> >> >
>> >> > But anyway, I'm not sure this is a good idea. The advantage of it is
>> >> > that the kernel support really is _very_ minimal.
>> >>
>> >> Considering that we'd at least also want this feature on ARM and
>> >> PowerPC 32/64, and that the gs segment selector approach clashes with
>> >> existing apps (wine), I'm not sure that implementing a gs segment
>> >> selector based approach to cpu number caching would lead to an overall
>> >> decrease in complexity if it leads to performance similar to those of
>> >> portable approaches.
>> >>
>> >> I'm perfectly fine with architecture-specific tweaks that lead to
>> >> fast-path speedups, but if we have to bite the bullet and implement
>> >> an approach based on TLS and registering a memory area at thread start
>> >> through a system call on other architectures anyway, it might end up
>> >> being less complex to add a new system call on x86 too, especially if
>> >> fast path overhead is similar.
>> >>
>> >> But I'm inclined to think that some aspect of the question eludes me,
>> >> especially given the amount of interest generated by the gs-segment
>> >> selector approach. What am I missing ?
>> >>
>> > As I wrote before you don't have to bite bullet as I said before. It
>> > suffices to create 128k element array with cpu for each tid, make that
>> > mmapable file and userspace could get cpu with nearly same performance
>> > without hacks.
>>
>> I don't see how this would be acceptable on memory-constrained embedded
>> systems. They have multiple cores, and performance requirements, so
>> having a fast getcpu would be useful there (e.g. telecom industry),
>> but they clearly cannot afford a 512kB table per process just for that.
>>
> Which just means that you need more complicated api and implementation
> for that but idea stays same. You would need syscalls
> register/deregister_cpuid_idx that would give you index used instead
> tid. A kernel would need to handle that many ids could be registered for
> each thread and resize mmaped file in syscalls.
I feel we're talking past each other here. What I propose is to implement
a system call that registers a TLS area. It can be invoked at thread start.
The kernel can then keep the current CPU number within that registered
area up-to-date. This system call does not care how the TLS is implemented
underneath.
My understanding is that you are suggesting a way to speed up TLS accesses
by creating a table indexed by TID. Although it might lead to interesting
speed ups useful when reading the TLS, I don't see how you proposal is
useful in addressing the problem of caching the current CPU number (other
than possibly speeding up TLS accesses).
Or am I missing something fundamental to your proposal ?
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ 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