* VLC doesn't play videos anymore since the PowerPC fixes 5.1-3
From: Christian Zigotzky @ 2019-04-03 3:52 UTC (permalink / raw)
To: linuxppc-dev
Please test VLC with the RC3 of kernel 5.1.
The removing of the PowerPC fixes 5.1-3 has solved the VLC issue. Another user has already confirmed that [1]. This isn’t an April Fool‘s. ;-)
Thanks
[1] http://forum.hyperion-entertainment.com/viewtopic.php?f=58&t=4256&start=20#p47561
^ permalink raw reply
* Re: [PATCH] powerpc/xmon: add read-only mode
From: Christopher M Riedl @ 2019-04-03 3:38 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev; +Cc: andonnel
In-Reply-To: <14aa7671-fc0b-1cbc-da19-004299b304a2@c-s.fr>
> On March 29, 2019 at 3:41 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
>
>
>
> Le 29/03/2019 à 05:21, cmr a écrit :
> > Operations which write to memory should be restricted on secure systems
> > and optionally to avoid self-destructive behaviors.
> >
> > Add a config option, XMON_RO, to control default xmon behavior along
> > with kernel cmdline options xmon=ro and xmon=rw for explicit control.
> > The default is to enable read-only mode.
> >
> > The following xmon operations are affected:
> > memops:
> > disable memmove
> > disable memset
> > memex:
> > no-op'd mwrite
> > super_regs:
> > no-op'd write_spr
> > bpt_cmds:
> > disable
> > proc_call:
> > disable
> >
> > Signed-off-by: cmr <cmr@informatik.wtf>
>
> A Fully qualified name should be used.
What do you mean by fully-qualified here? PPC_XMON_RO? (PPC_)XMON_READONLY?
>
> > ---
> > arch/powerpc/Kconfig.debug | 7 +++++++
> > arch/powerpc/xmon/xmon.c | 24 ++++++++++++++++++++++++
> > 2 files changed, 31 insertions(+)
> >
> > diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> > index 4e00cb0a5464..33cc01adf4cb 100644
> > --- a/arch/powerpc/Kconfig.debug
> > +++ b/arch/powerpc/Kconfig.debug
> > @@ -117,6 +117,13 @@ config XMON_DISASSEMBLY
> > to say Y here, unless you're building for a memory-constrained
> > system.
> >
> > +config XMON_RO
> > + bool "Set xmon read-only mode"
> > + depends on XMON
> > + default y
>
> Should it really be always default y ?
> I would set default 'y' only when some security options are also set.
>
This is a good point, I based this on an internal Slack suggestion but giving this more thought, disabling read-only mode by default makes more sense. I'm not sure what security options could be set though?
^ permalink raw reply
* Re: [PATCH] powerpc/watchdog: Use hrtimers for per-CPU heartbeat
From: Ravi Bangoria @ 2019-04-03 3:42 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: Ravikumar Bangoria, linuxppc-dev
In-Reply-To: <20190402112521.24888-1-npiggin@gmail.com>
On 4/2/19 4:55 PM, Nicholas Piggin wrote:
> Using a jiffies timer creates a dependency on the tick_do_timer_cpu
> incrementing jiffies. If that CPU has locked up and jiffies is not
> incrementing, the watchdog heartbeat timer for all CPUs stops and
> creates false positives and confusing warnings on local CPUs, and
> also causes the SMP detector to stop, so the root cause is never
> detected.
>
> Fix this by using hrtimer based timers for the watchdog heartbeat,
> like the generic kernel hardlockup detector.
>
> Reported-by: Ravikumar Bangoria <ravi.bangoria@in.ibm.com>
Reported-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Thanks,
Ravi
^ permalink raw reply
* Re: [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere
From: Michael Ellerman @ 2019-04-03 2:47 UTC (permalink / raw)
To: Arnd Bergmann, Andrew Morton
Cc: Rich Felker, linux-ia64, linux-sh, Heiko Carstens, linux-mips,
James E . J . Bottomley, Max Filippov, Paul Mackerras, sparclinux,
linux-s390, Helge Deller, Russell King, Geert Uytterhoeven,
Catalin Marinas, James Hogan, Firoz Khan, Matt Turner, Fenghua Yu,
Arnd Bergmann, Will Deacon, linux-m68k, Ivan Kokshaysky,
linux-arm-kernel, Richard Henderson, Jens Axboe, Michal Simek,
Tony Luck, linux-parisc, linux-kernel, Ralf Baechle, Paul Burton,
linux-alpha, Martin Schwidefsky, linuxppc-dev, David S . Miller
In-Reply-To: <20190325144737.703921-1-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> writes:
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index b18abb0c3dae..00f5a63c8d9a 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -505,3 +505,7 @@
> 421 32 rt_sigtimedwait_time64 sys_rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
> 422 32 futex_time64 sys_futex sys_futex
> 423 32 sched_rr_get_interval_time64 sys_sched_rr_get_interval sys_sched_rr_get_interval
> +424 common pidfd_send_signal sys_pidfd_send_signal
> +425 common io_uring_setup sys_io_uring_setup
> +426 common io_uring_enter sys_io_uring_enter
> +427 common io_uring_register sys_io_uring_register
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
Lightly tested.
The pidfd_test selftest passes.
Ran the io_uring example from fio, which prints lots of:
IOPS=209952, IOS/call=32/32, inflight=117 (117), Cachehit=0.00%
IOPS=209952, IOS/call=32/32, inflight=116 (116), Cachehit=0.00%
IOPS=209920, IOS/call=32/32, inflight=115 (115), Cachehit=0.00%
IOPS=209952, IOS/call=32/32, inflight=115 (115), Cachehit=0.00%
IOPS=209920, IOS/call=32/32, inflight=115 (115), Cachehit=0.00%
IOPS=209952, IOS/call=32/32, inflight=115 (115), Cachehit=0.00%
IOPS=210016, IOS/call=32/32, inflight=114 (114), Cachehit=0.00%
IOPS=210016, IOS/call=32/32, inflight=113 (113), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=113 (113), Cachehit=0.00%
IOPS=210016, IOS/call=32/32, inflight=113 (113), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=112 (112), Cachehit=0.00%
IOPS=210016, IOS/call=32/32, inflight=110 (110), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=105 (105), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=104 (104), Cachehit=0.00%
IOPS=210080, IOS/call=32/32, inflight=102 (102), Cachehit=0.00%
IOPS=210112, IOS/call=32/32, inflight=100 (100), Cachehit=0.00%
IOPS=210080, IOS/call=32/32, inflight=97 (97), Cachehit=0.00%
IOPS=210112, IOS/call=32/32, inflight=97 (97), Cachehit=0.00%
IOPS=210112, IOS/call=32/31, inflight=126 (126), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=126 (126), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=125 (125), Cachehit=0.00%
IOPS=210016, IOS/call=32/32, inflight=119 (119), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=117 (117), Cachehit=0.00%
IOPS=210016, IOS/call=32/32, inflight=114 (114), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=111 (111), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=108 (108), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=107 (107), Cachehit=0.00%
IOPS=210048, IOS/call=32/32, inflight=105 (105), Cachehit=0.00%
Which is good I think?
cheers
^ permalink raw reply
* Re: [PATCH 2/2] arch: add pidfd and io_uring syscalls everywhere
From: Michael Ellerman @ 2019-04-03 1:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rich Felker, linux-ia64, Linux-sh list, Heiko Carstens,
linux-mips, James E . J . Bottomley, Max Filippov, Paul Mackerras,
sparclinux, linux-s390, Helge Deller, Russell King,
Geert Uytterhoeven, Catalin Marinas, James Hogan, Firoz Khan,
Matt Turner, Fenghua Yu, Will Deacon, linux-m68k, Ivan Kokshaysky,
Linux ARM, Richard Henderson, Michal Simek, Tony Luck,
Parisc List, Linux Kernel Mailing List, Ralf Baechle, Paul Burton,
alpha, Martin Schwidefsky, Andrew Morton, linuxppc-dev,
David S . Miller
In-Reply-To: <CAK8P3a14of-jJA2q7rki3gHk6gwE-0TCkzHuXZ1+TkemopgfRA@mail.gmail.com>
Arnd Bergmann <arnd@arndb.de> writes:
> On Sun, Mar 31, 2019 at 5:47 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Arnd Bergmann <arnd@arndb.de> writes:
>> > Add the io_uring and pidfd_send_signal system calls to all architectures.
>> >
>> > These system calls are designed to handle both native and compat tasks,
>> > so all entries are the same across architectures, only arm-compat and
>> > the generic tale still use an old format.
>> >
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > ---
>> > arch/alpha/kernel/syscalls/syscall.tbl | 4 ++++
>> > arch/arm/tools/syscall.tbl | 4 ++++
>> > arch/arm64/include/asm/unistd.h | 2 +-
>> > arch/arm64/include/asm/unistd32.h | 8 ++++++++
>> > arch/ia64/kernel/syscalls/syscall.tbl | 4 ++++
>> > arch/m68k/kernel/syscalls/syscall.tbl | 4 ++++
>> > arch/microblaze/kernel/syscalls/syscall.tbl | 4 ++++
>> > arch/mips/kernel/syscalls/syscall_n32.tbl | 4 ++++
>> > arch/mips/kernel/syscalls/syscall_n64.tbl | 4 ++++
>> > arch/mips/kernel/syscalls/syscall_o32.tbl | 4 ++++
>> > arch/parisc/kernel/syscalls/syscall.tbl | 4 ++++
>> > arch/powerpc/kernel/syscalls/syscall.tbl | 4 ++++
>>
>> Have you done any testing?
>>
>> I'd rather not wire up syscalls that have never been tested at all on
>> powerpc.
>
> No, I have not. I did review the system calls carefully and added the first
> patch to fix the bug on x86 compat mode before adding the same bug
> on the other compat architectures though ;-)
>
> Generally, my feeling is that adding system calls is not fundamentally
> different from adding other ABIs, and we should really do it at
> the same time across all architectures, rather than waiting for each
> maintainer to get around to reviewing and testing the new calls
> first. This is not a problem on powerpc, but a lot of other architectures
> are less active, which is how we have always ended up with
> different sets of system calls across architectures.
Well it's still something of a problem on powerpc. No one has
volunteered to test io_uring on powerpc, so at this stage it will go in
completely untested.
If there was a selftest in the tree I'd be a bit happier, because at
least then our CI would start testing it as soon as the syscalls were
wired up in linux-next.
And yeah obviously I should test it, but I don't have infinite time
unfortunately.
> The problem here is that this makes it harder for the C library to
> know when a system call is guaranteed to be available. glibc
> still needs a feature test for newly added syscalls to see if they
> are working (they might be backported to an older kernel, or
> disabled), but whenever the minimum kernel version is increased,
> it makes sense to drop those checks and assume non-optional
> system calls will work if they were part of that minimum version.
But that's the thing, if we just wire them up untested they may not
actually work. And then you have the far worse situation where the
syscall exists in kernel version x but does not actually work properly.
See the mess we have with pkeys for example.
> In the future, I'd hope that any new system calls get added
> right away on all architectures when they land (it was a bit
> tricky this time, because I still did a bunch of reworks that
> conflicted with the new calls). Bugs will happen of course, but
> I think adding them sooner makes it more likely to catch those
> bugs early on so we have a chance to fix them properly,
> and need fewer arch specific workarounds (ideally none)
> for system calls.
For syscalls that have a selftest in the tree, and don't rely on
anything arch specific I agree.
I'm a bit more wary of things that are not easily tested and have the
potential to work differently across arches.
cheers
^ permalink raw reply
* Re: [PATCH stable v4.14 13/32] powerpc/fsl: Add barrier_nospec implementation for NXP PowerPC Book3E
From: Michael Ellerman @ 2019-04-03 0:53 UTC (permalink / raw)
To: Joakim Tjernlund, stable@vger.kernel.org,
gregkh@linuxfoundation.org
Cc: diana.craciun@nxp.com, linuxppc-dev@ozlabs.org, msuchanek@suse.de
In-Reply-To: <b4501def553a2352a4f219637245d3bbcaf9700d.camel@infinera.com>
Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
> On Tue, 2019-04-02 at 17:19 +1100, Michael Ellerman wrote:
>> Joakim Tjernlund <Joakim.Tjernlund@infinera.com> writes:
...
>>
>> > Can I compile it away?
>>
>> You can't actually, but you can disable it at runtime with
>> "nospectre_v1" on the kernel command line.
>>
>> We could make it a user selectable compile time option if you really
>> want it to be.
>
> I think yes. Considering that these patches are fairly untested and the impact
> in the wild unknown. Requiring systems to change their boot config over night is
> too fast.
OK. Just to be clear, you're actually using 4.14 on an NXP board and
would actually use this option? I don't want to add another option just
for a theoretical use case.
cheers
^ permalink raw reply
* [PATCH] powerpc: config: skiroot: Add (back) MLX5 ethernet support
From: Joel Stanley @ 2019-04-03 0:49 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Carol L Soto, Stewart Smith
It turns out that some defconfig changes and kernel config option
changes meant we accidentally dropped Ethernet support for Mellanox CLX5
cards.
Reported-by: Carol L Soto <clsoto@us.ibm.com>
Suggested-by: Carol L Soto <clsoto@us.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
arch/powerpc/configs/skiroot_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
index 5ba131c30f6b..6038b9347d9e 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -163,6 +163,8 @@ CONFIG_S2IO=m
CONFIG_MLX4_EN=m
# CONFIG_MLX4_CORE_GEN2 is not set
CONFIG_MLX5_CORE=m
+CONFIG_MLX5_CORE_EN=y
+# CONFIG_MLX5_EN_RXNFC is not set
# CONFIG_NET_VENDOR_MICREL is not set
# CONFIG_NET_VENDOR_MICROSEMI is not set
CONFIG_MYRI10GE=m
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/6] mm: change locked_vm's type from unsigned long to atomic64_t
From: Davidlohr Bueso @ 2019-04-02 23:43 UTC (permalink / raw)
To: Andrew Morton
Cc: kvm, Alan Tull, Alexey Kardashevskiy, linux-fpga, linux-kernel,
kvm-ppc, Daniel Jordan, linux-mm, Alex Williamson, Moritz Fischer,
Christoph Lameter, linuxppc-dev, Wu Hao
In-Reply-To: <20190402150424.5cf64e19deeafa58fc6c1a9f@linux-foundation.org>
On Tue, 02 Apr 2019, Andrew Morton wrote:
>Also, we didn't remove any down_write(mmap_sem)s from core code so I'm
>thinking that the benefit of removing a few mmap_sem-takings from a few
>obscure drivers (sorry ;)) is pretty small.
afaik porting the remaining incorrect users of locked_vm to pinned_vm was
the next step before this one, which made converting locked_vm to atomic
hardly worth it. Daniel?
Thanks,
Davidlohr
^ permalink raw reply
* Re: [PATCH 0/4] Enabling secure boot on PowerNV systems
From: Claudio Carvalho @ 2019-04-02 23:31 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-efi, Ard Biesheuvel, Nayna Jain, Linux Kernel Mailing List,
Matthew Garret, linuxppc-dev, Paul Mackerras, Jeremy Kerr,
linux-integrity
In-Reply-To: <CACdnJusBm93zwDqTXTx_QYsg1-aGUAAHm_qq8Lcx3TvGTxdmbg@mail.gmail.com>
On 4/2/19 6:51 PM, Matthew Garrett wrote:
> On Tue, Apr 2, 2019 at 2:11 PM Claudio Carvalho <cclaudio@linux.ibm.com> wrote:
>> We want to use the efivarfs for compatibility with existing userspace
>> tools. We will track and match any EFI changes that affect us.
> So you implement the full PK/KEK/db/dbx/dbt infrastructure, and
> updates are signed in the same way?
For the first version, our firmware will implement a simplistic PK, KEK and
db infrastructure (without dbx and dbt) where only the Setup and User modes
will be supported.
PK, KEK and db updates will be signed the same way, that is, using
userspace tooling like efitools in PowerNV. As for the authentication
descriptors, only the EFI_VARIABLE_AUTHENTICATION_2 descriptor will be
supported.
>> Our use case is restricted to secure boot - this is not going to be a
>> general purpose EFI variable implementation.
> In that case we might be better off with a generic interface for this
> purpose that we can expose on all platforms that implement a secure
> boot key hierarchy. Having an efivarfs that doesn't allow the creation
> of arbitrary attributes may break other existing userland
> expectations.
>
For what it's worth, gsmi uses the efivars infrastructure for EFI-like
variables.
What might a generic interface look like? It would have to work for
existing secure boot solutions - including EFI - which would seem to imply
changes to userspace tools.
Claudio
^ permalink raw reply
* Re: [PATCH v1 2/4] soc/fsl/guts: Add definition for LX2160A
From: Li Yang @ 2019-04-02 23:05 UTC (permalink / raw)
To: Vabhav Sharma
Cc: mark.rutland@arm.com, kstewart@linuxfoundation.org,
ulf.hansson@linaro.org, linux-kernel-owner@vger.kernel.org,
catalin.marinas@arm.com, mturquette@baylibre.com,
will.deacon@arm.com, adrian.hunter@intel.com,
yamada.masahiro@socionext.com, linux-clk@vger.kernel.org,
linux@armlinux.org.uk, viresh.kumar@linaro.org, Yinbo Zhu,
devicetree@vger.kernel.org, arnd@arndb.de,
linux-pm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
oss@buserror.net, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org, sboyd@kernel.org,
gregkh@linuxfoundation.org, rjw@rjwysocki.net,
linux-kernel@vger.kernel.org, sudeep.holla@arm.com,
shawnguo@kernel.org
In-Reply-To: <1551132245-6089-3-git-send-email-vabhav.sharma@nxp.com>
On Tue, Feb 26, 2019 at 4:12 AM Vabhav Sharma <vabhav.sharma@nxp.com> wrote:
>
> Adding compatible string "lx2160a-dcfg" to
> initialize guts driver for lx2160 and SoC die
> attribute definition for LX2160A
Applied to branch next. Thanks.
Regards,
Leo
>
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>
> Acked-by: Li Yang <leoyang.li@nxp.com>
> ---
> drivers/soc/fsl/guts.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
> index 302e0c8..bcab1ee 100644
> --- a/drivers/soc/fsl/guts.c
> +++ b/drivers/soc/fsl/guts.c
> @@ -100,6 +100,11 @@ static const struct fsl_soc_die_attr fsl_soc_die[] = {
> .svr = 0x87000000,
> .mask = 0xfff70000,
> },
> + /* Die: LX2160A, SoC: LX2160A/LX2120A/LX2080A */
> + { .die = "LX2160A",
> + .svr = 0x87360000,
> + .mask = 0xff3f0000,
> + },
> { },
> };
>
> @@ -222,6 +227,7 @@ static const struct of_device_id fsl_guts_of_match[] = {
> { .compatible = "fsl,ls1088a-dcfg", },
> { .compatible = "fsl,ls1012a-dcfg", },
> { .compatible = "fsl,ls1046a-dcfg", },
> + { .compatible = "fsl,lx2160a-dcfg", },
> {}
> };
> MODULE_DEVICE_TABLE(of, fsl_guts_of_match);
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH] soc/fsl/qe: Fix an error code in qe_pin_request()
From: Li Yang @ 2019-04-02 23:02 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linuxppc-dev, kernel-janitors, Qiang Zhao
In-Reply-To: <20190328141841.GN32590@kadam>
On Thu, Mar 28, 2019 at 9:21 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> We forgot to set "err" on this error path.
>
> Fixes: 1a2d397a6eb5 ("gpio/powerpc: Eliminate duplication of of_get_named_gpio_flags()")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to fix branch. Thanks.
Regards,
Leo
> ---
> drivers/soc/fsl/qe/gpio.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/qe/gpio.c b/drivers/soc/fsl/qe/gpio.c
> index 819bed0f5667..51b3a47b5a55 100644
> --- a/drivers/soc/fsl/qe/gpio.c
> +++ b/drivers/soc/fsl/qe/gpio.c
> @@ -179,8 +179,10 @@ struct qe_pin *qe_pin_request(struct device_node *np, int index)
> if (err < 0)
> goto err0;
> gc = gpio_to_chip(err);
> - if (WARN_ON(!gc))
> + if (WARN_ON(!gc)) {
> + err = -ENODEV;
> goto err0;
> + }
>
> if (!of_device_is_compatible(gc->of_node, "fsl,mpc8323-qe-pario-bank")) {
> pr_debug("%s: tried to get a non-qe pin\n", __func__);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 0/4] Enabling secure boot on PowerNV systems
From: Matthew Garrett @ 2019-04-02 21:51 UTC (permalink / raw)
To: Claudio Carvalho
Cc: linux-efi, Ard Biesheuvel, Nayna Jain, Linux Kernel Mailing List,
Matthew Garret, linuxppc-dev, Paul Mackerras, Jeremy Kerr,
linux-integrity
In-Reply-To: <4ce5e057-0702-b0d5-7bb2-cea5b22e2efa@linux.ibm.com>
On Tue, Apr 2, 2019 at 2:11 PM Claudio Carvalho <cclaudio@linux.ibm.com> wrote:
> We want to use the efivarfs for compatibility with existing userspace
> tools. We will track and match any EFI changes that affect us.
So you implement the full PK/KEK/db/dbx/dbt infrastructure, and
updates are signed in the same way?
> Our use case is restricted to secure boot - this is not going to be a
> general purpose EFI variable implementation.
In that case we might be better off with a generic interface for this
purpose that we can expose on all platforms that implement a secure
boot key hierarchy. Having an efivarfs that doesn't allow the creation
of arbitrary attributes may break other existing userland
expectations.
^ permalink raw reply
* Re: [PATCH 0/4] Enabling secure boot on PowerNV systems
From: Matthew Garrett @ 2019-04-02 19:36 UTC (permalink / raw)
To: Claudio Carvalho
Cc: linux-efi, Ard Biesheuvel, Nayna Jain, Linux Kernel Mailing List,
Matthew Garret, linuxppc-dev, Paul Mackerras, Jeremy Kerr,
linux-integrity
In-Reply-To: <20190402181505.25037-1-cclaudio@linux.ibm.com>
On Tue, Apr 2, 2019 at 11:15 AM Claudio Carvalho <cclaudio@linux.ibm.com> wrote:
> 1. Enable efivarfs by selecting CONFIG_EFI in the CONFIG_OPAL_SECVAR
> introduced in this patch set. With CONFIG_EFIVAR_FS, userspace tools can
> be used to manage the secure variables.
efivarfs has some pretty significant behavioural semantics that
directly reflect the EFI specification. Using it to expose non-EFI
variable data feels like it's going to increase fragility - there's a
risk that we'll change things in a way that makes sense for the EFI
spec but breaks your use case. Is the desire to use efivarfs to
maintain consistency with existing userland tooling, or just to avoid
having a separate filesystem?
^ permalink raw reply
* [PATCH 0/6] convert locked_vm from unsigned long to atomic64_t
From: Daniel Jordan @ 2019-04-02 20:41 UTC (permalink / raw)
To: akpm
Cc: Davidlohr Bueso, kvm, Alan Tull, Alexey Kardashevskiy, linux-fpga,
linux-kernel, kvm-ppc, daniel.m.jordan, linux-mm, Alex Williamson,
Moritz Fischer, Christoph Lameter, linuxppc-dev, Wu Hao
Hi,
From patch 1:
Taking and dropping mmap_sem to modify a single counter, locked_vm, is
overkill when the counter could be synchronized separately.
Make mmap_sem a little less coarse by changing locked_vm to an atomic,
the 64-bit variety to avoid issues with overflow on 32-bit systems.
This is a more conservative alternative to [1] with no user-visible
effects. Thanks to Alexey Kardashevskiy for pointing out the racy
atomics and to Alex Williamson, Christoph Lameter, Ira Weiny, and Jason
Gunthorpe for their comments on [1].
Davidlohr Bueso recently did a similar conversion for pinned_vm[2].
Testing
1. passes LTP mlock[all], munlock[all], fork, mmap, and mremap tests in an
x86 kvm guest
2. a VFIO-enabled x86 kvm guest shows the same VmLck in
/proc/pid/status before and after this change
3. cross-compiles on powerpc
The series is based on v5.1-rc3. Please consider for 5.2.
Daniel
[1] https://lore.kernel.org/linux-mm/20190211224437.25267-1-daniel.m.jordan@oracle.com/
[2] https://lore.kernel.org/linux-mm/20190206175920.31082-1-dave@stgolabs.net/
Daniel Jordan (6):
mm: change locked_vm's type from unsigned long to atomic64_t
vfio/type1: drop mmap_sem now that locked_vm is atomic
vfio/spapr_tce: drop mmap_sem now that locked_vm is atomic
fpga/dlf/afu: drop mmap_sem now that locked_vm is atomic
powerpc/mmu: drop mmap_sem now that locked_vm is atomic
kvm/book3s: drop mmap_sem now that locked_vm is atomic
arch/powerpc/kvm/book3s_64_vio.c | 34 ++++++++++--------------
arch/powerpc/mm/mmu_context_iommu.c | 28 +++++++++-----------
drivers/fpga/dfl-afu-dma-region.c | 40 ++++++++++++-----------------
drivers/vfio/vfio_iommu_spapr_tce.c | 37 ++++++++++++--------------
drivers/vfio/vfio_iommu_type1.c | 31 +++++++++-------------
fs/proc/task_mmu.c | 2 +-
include/linux/mm_types.h | 2 +-
kernel/fork.c | 2 +-
mm/debug.c | 5 ++--
mm/mlock.c | 4 +--
mm/mmap.c | 18 ++++++-------
mm/mremap.c | 6 ++---
12 files changed, 89 insertions(+), 120 deletions(-)
base-commit: 79a3aaa7b82e3106be97842dedfd8429248896e6
--
2.21.0
^ permalink raw reply
* Re: [PATCH 1/6] mm: change locked_vm's type from unsigned long to atomic64_t
From: Andrew Morton @ 2019-04-02 22:04 UTC (permalink / raw)
To: Daniel Jordan
Cc: Davidlohr Bueso, kvm, Alan Tull, Alexey Kardashevskiy, linux-fpga,
linux-kernel, kvm-ppc, linux-mm, Alex Williamson, Moritz Fischer,
Christoph Lameter, linuxppc-dev, Wu Hao
In-Reply-To: <20190402204158.27582-2-daniel.m.jordan@oracle.com>
On Tue, 2 Apr 2019 16:41:53 -0400 Daniel Jordan <daniel.m.jordan@oracle.com> wrote:
> Taking and dropping mmap_sem to modify a single counter, locked_vm, is
> overkill when the counter could be synchronized separately.
>
> Make mmap_sem a little less coarse by changing locked_vm to an atomic,
> the 64-bit variety to avoid issues with overflow on 32-bit systems.
>
> ...
>
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -59,32 +59,34 @@ static unsigned long kvmppc_stt_pages(unsigned long tce_pages)
> static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
> {
> long ret = 0;
> + s64 locked_vm;
>
> if (!current || !current->mm)
> return ret; /* process exited */
>
> down_write(¤t->mm->mmap_sem);
>
> + locked_vm = atomic64_read(¤t->mm->locked_vm);
> if (inc) {
> unsigned long locked, lock_limit;
>
> - locked = current->mm->locked_vm + stt_pages;
> + locked = locked_vm + stt_pages;
> lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> if (locked > lock_limit && !capable(CAP_IPC_LOCK))
> ret = -ENOMEM;
> else
> - current->mm->locked_vm += stt_pages;
> + atomic64_add(stt_pages, ¤t->mm->locked_vm);
> } else {
> - if (WARN_ON_ONCE(stt_pages > current->mm->locked_vm))
> - stt_pages = current->mm->locked_vm;
> + if (WARN_ON_ONCE(stt_pages > locked_vm))
> + stt_pages = locked_vm;
>
> - current->mm->locked_vm -= stt_pages;
> + atomic64_sub(stt_pages, ¤t->mm->locked_vm);
> }
With the current code, current->mm->locked_vm cannot go negative.
After the patch, it can go negative. If someone else decreased
current->mm->locked_vm between this function's atomic64_read() and
atomic64_sub().
I guess this is a can't-happen in this case because the racing code
which performed the modification would have taken it negative anyway.
But this all makes me rather queazy.
Also, we didn't remove any down_write(mmap_sem)s from core code so I'm
thinking that the benefit of removing a few mmap_sem-takings from a few
obscure drivers (sorry ;)) is pretty small.
Also, the argument for switching 32-bit arches to a 64-bit counter was
suspiciously vague. What overflow issues? Or are we just being lazy?
^ permalink raw reply
* [PATCH 6/6] kvm/book3s: drop mmap_sem now that locked_vm is atomic
From: Daniel Jordan @ 2019-04-02 20:41 UTC (permalink / raw)
To: akpm
Cc: Davidlohr Bueso, Alexey Kardashevskiy, linux-kernel, kvm-ppc,
daniel.m.jordan, linux-mm, Christoph Lameter, linuxppc-dev
In-Reply-To: <20190402204158.27582-1-daniel.m.jordan@oracle.com>
With locked_vm now an atomic, there is no need to take mmap_sem as
writer. Delete and refactor accordingly.
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: <linux-mm@kvack.org>
Cc: <kvm-ppc@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-kernel@vger.kernel.org>
---
arch/powerpc/kvm/book3s_64_vio.c | 34 +++++++++++---------------------
1 file changed, 12 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index e7fdb6d10eeb..8e034c3a5d25 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -56,7 +56,7 @@ static unsigned long kvmppc_stt_pages(unsigned long tce_pages)
return tce_pages + ALIGN(stt_bytes, PAGE_SIZE) / PAGE_SIZE;
}
-static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
+static long kvmppc_account_memlimit(unsigned long pages, bool inc)
{
long ret = 0;
s64 locked_vm;
@@ -64,33 +64,23 @@ static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
if (!current || !current->mm)
return ret; /* process exited */
- down_write(¤t->mm->mmap_sem);
-
- locked_vm = atomic64_read(¤t->mm->locked_vm);
if (inc) {
- unsigned long locked, lock_limit;
+ unsigned long lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
- locked = locked_vm + stt_pages;
- lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
- if (locked > lock_limit && !capable(CAP_IPC_LOCK))
+ locked_vm = atomic64_add_return(pages, ¤t->mm->locked_vm);
+ if (locked_vm > lock_limit && !capable(CAP_IPC_LOCK)) {
ret = -ENOMEM;
- else
- atomic64_add(stt_pages, ¤t->mm->locked_vm);
+ atomic64_sub(pages, ¤t->mm->locked_vm);
+ }
} else {
- if (WARN_ON_ONCE(stt_pages > locked_vm))
- stt_pages = locked_vm;
-
- atomic64_sub(stt_pages, ¤t->mm->locked_vm);
+ locked_vm = atomic64_sub_return(pages, ¤t->mm->locked_vm);
+ WARN_ON_ONCE(locked_vm < 0);
}
- pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%ld %ld/%ld%s\n", current->pid,
- inc ? '+' : '-',
- stt_pages << PAGE_SHIFT,
- atomic64_read(¤t->mm->locked_vm) << PAGE_SHIFT,
- rlimit(RLIMIT_MEMLOCK),
- ret ? " - exceeded" : "");
-
- up_write(¤t->mm->mmap_sem);
+ pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%lu %lld/%lu%s\n", current->pid,
+ inc ? '+' : '-', pages << PAGE_SHIFT,
+ locked_vm << PAGE_SHIFT,
+ rlimit(RLIMIT_MEMLOCK), ret ? " - exceeded" : "");
return ret;
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH 0/4] Enabling secure boot on PowerNV systems
From: Claudio Carvalho @ 2019-04-02 21:11 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-efi, Ard Biesheuvel, Nayna Jain, Linux Kernel Mailing List,
Matthew Garret, linuxppc-dev, Paul Mackerras, Jeremy Kerr,
linux-integrity
In-Reply-To: <CACdnJuumhkqTb4+1=QBiLmbW4xd3wW=MZu6Tj_KdaoTMhCN+Tg@mail.gmail.com>
On 4/2/19 4:36 PM, Matthew Garrett wrote:
> On Tue, Apr 2, 2019 at 11:15 AM Claudio Carvalho <cclaudio@linux.ibm.com> wrote:
>> 1. Enable efivarfs by selecting CONFIG_EFI in the CONFIG_OPAL_SECVAR
>> introduced in this patch set. With CONFIG_EFIVAR_FS, userspace tools can
>> be used to manage the secure variables.
> efivarfs has some pretty significant behavioural semantics that
> directly reflect the EFI specification. Using it to expose non-EFI
> variable data feels like it's going to increase fragility - there's a
> risk that we'll change things in a way that makes sense for the EFI
> spec but breaks your use case. Is the desire to use efivarfs to
> maintain consistency with existing userland tooling, or just to avoid
> having a separate filesystem?
>
We want to use the efivarfs for compatibility with existing userspace
tools. We will track and match any EFI changes that affect us.
Our use case is restricted to secure boot - this is not going to be a
general purpose EFI variable implementation.
Claudio
^ permalink raw reply
* [PATCH 5/6] powerpc/mmu: drop mmap_sem now that locked_vm is atomic
From: Daniel Jordan @ 2019-04-02 20:41 UTC (permalink / raw)
To: akpm
Cc: Davidlohr Bueso, Alexey Kardashevskiy, linux-kernel,
daniel.m.jordan, linux-mm, Paul Mackerras, Christoph Lameter,
linuxppc-dev
In-Reply-To: <20190402204158.27582-1-daniel.m.jordan@oracle.com>
With locked_vm now an atomic, there is no need to take mmap_sem as
writer. Delete and refactor accordingly.
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: <linux-mm@kvack.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-kernel@vger.kernel.org>
---
arch/powerpc/mm/mmu_context_iommu.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index 8038ac24a312..a4ef22b67c07 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -54,34 +54,29 @@ struct mm_iommu_table_group_mem_t {
static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
unsigned long npages, bool incr)
{
- long ret = 0, locked, lock_limit;
+ long ret = 0;
+ unsigned long lock_limit;
s64 locked_vm;
if (!npages)
return 0;
- down_write(&mm->mmap_sem);
- locked_vm = atomic64_read(&mm->locked_vm);
if (incr) {
- locked = locked_vm + npages;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
- if (locked > lock_limit && !capable(CAP_IPC_LOCK))
+ locked_vm = atomic64_add_return(npages, &mm->locked_vm);
+ if (locked_vm > lock_limit && !capable(CAP_IPC_LOCK)) {
ret = -ENOMEM;
- else
- atomic64_add(npages, &mm->locked_vm);
+ atomic64_sub(npages, &mm->locked_vm);
+ }
} else {
- if (WARN_ON_ONCE(npages > locked_vm))
- npages = locked_vm;
- atomic64_sub(npages, &mm->locked_vm);
+ locked_vm = atomic64_sub_return(npages, &mm->locked_vm);
+ WARN_ON_ONCE(locked_vm < 0);
}
- pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%ld %ld/%ld\n",
- current ? current->pid : 0,
- incr ? '+' : '-',
- npages << PAGE_SHIFT,
- atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
+ pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%lu %lld/%lu\n",
+ current ? current->pid : 0, incr ? '+' : '-',
+ npages << PAGE_SHIFT, locked_vm << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
- up_write(&mm->mmap_sem);
return ret;
}
--
2.21.0
^ permalink raw reply related
* [PATCH v3 5/5] Lib: sort.h: remove the size argument from the swap function
From: Andrey Abramov @ 2019-04-02 20:55 UTC (permalink / raw)
To: vgupta, benh, paulus, Michael Ellerman, tglx, mingo, bp, hpa, x86,
mark, jlbec, richard, dedekind1, adrian.hunter, gregkh,
naveen.n.rao, jpoimboe, Dave Chinner, darrick.wong,
ard.biesheuvel, George Spelvin, linux-snps-arc,
Linux Kernel Mailing List, linuxppc-dev, ocfs2-devel, linux-mtd,
sfr
Cc: jannh, peterz, Rasmus Villemoes, yamada.masahiro, Morton Andrew,
Andy Shevchenko, jslaby
In-Reply-To: <824481554237711@sas1-23a37bc8251c.qloud-c.yandex.net>
Removes size argument from the swap function because:
1) It wasn't used.
2) Custom swap function knows what kind of objects it swaps,
so it already knows their sizes.
Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
Reviewed by: George Spelvin <lkml@sdf.org>
---
arch/x86/kernel/unwind_orc.c | 2 +-
include/linux/sort.h | 2 +-
kernel/jump_label.c | 2 +-
lib/extable.c | 2 +-
lib/sort.c | 7 +++----
5 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 89be1be1790c..dc410b567189 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -176,7 +176,7 @@ static struct orc_entry *orc_find(unsigned long ip)
return orc_ftrace_find(ip);
}
-static void orc_sort_swap(void *_a, void *_b, int size)
+static void orc_sort_swap(void *_a, void *_b)
{
struct orc_entry *orc_a, *orc_b;
struct orc_entry orc_tmp;
diff --git a/include/linux/sort.h b/include/linux/sort.h
index 2b99a5dd073d..13bb4635b5f1 100644
--- a/include/linux/sort.h
+++ b/include/linux/sort.h
@@ -6,6 +6,6 @@
void sort(void *base, size_t num, size_t size,
int (*cmp)(const void *, const void *),
- void (*swap)(void *, void *, int));
+ void (*swap)(void *, void *));
#endif
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index bad96b476eb6..6b1187b8a060 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -45,7 +45,7 @@ static int jump_label_cmp(const void *a, const void *b)
return 0;
}
-static void jump_label_swap(void *a, void *b, int size)
+static void jump_label_swap(void *a, void *b)
{
long delta = (unsigned long)a - (unsigned long)b;
struct jump_entry *jea = a;
diff --git a/lib/extable.c b/lib/extable.c
index f54996fdd0b8..0515a94538ca 100644
--- a/lib/extable.c
+++ b/lib/extable.c
@@ -28,7 +28,7 @@ static inline unsigned long ex_to_insn(const struct exception_table_entry *x)
#ifndef ARCH_HAS_RELATIVE_EXTABLE
#define swap_ex NULL
#else
-static void swap_ex(void *a, void *b, int size)
+static void swap_ex(void *a, void *b)
{
struct exception_table_entry *x = a, *y = b, tmp;
int delta = b - a;
diff --git a/lib/sort.c b/lib/sort.c
index 50855ea8c262..8704750e6bde 100644
--- a/lib/sort.c
+++ b/lib/sort.c
@@ -114,7 +114,7 @@ static void swap_bytes(void *a, void *b, size_t n)
} while (n);
}
-typedef void (*swap_func_t)(void *a, void *b, int size);
+typedef void (*swap_func_t)(void *a, void *b);
/*
* The values are arbitrary as long as they can't be confused with
@@ -138,7 +138,7 @@ static void do_swap(void *a, void *b, size_t size, swap_func_t swap_func)
else if (swap_func == SWAP_BYTES)
swap_bytes(a, b, size);
else
- swap_func(a, b, (int)size);
+ swap_func(a, b);
}
/**
@@ -186,8 +186,7 @@ static size_t parent(size_t i, unsigned int lsbit, size_t size)
* it less suitable for kernel use.
*/
void sort(void *base, size_t num, size_t size,
- int (*cmp_func)(const void *, const void *),
- void (*swap_func)(void *, void *, int size))
+ int (*cmp_func)(const void *, const void *), swap_func_t swap_func)
{
/* pre-scale counters for performance */
size_t n = num * size, a = (num/2) * size;
--
2.21.0
^ permalink raw reply related
* [PATCH v3 4/5] ubifs: find.c: replace swap function with built-in one
From: Andrey Abramov @ 2019-04-02 20:52 UTC (permalink / raw)
To: vgupta, benh, paulus, Michael Ellerman, tglx, mingo, bp, hpa, x86,
mark, jlbec, richard, dedekind1, adrian.hunter, gregkh,
naveen.n.rao, jpoimboe, Dave Chinner, darrick.wong,
ard.biesheuvel, George Spelvin, linux-snps-arc,
Linux Kernel Mailing List, linuxppc-dev, ocfs2-devel, linux-mtd,
sfr
Cc: gustavo
In-Reply-To: <824481554237711@sas1-23a37bc8251c.qloud-c.yandex.net>
Replace swap_dirty_idx function with built-in one,
because swap_dirty_idx does only a simple byte to byte swap.
Since Spectre mitigations have made indirect function calls more
expensive, and the default simple byte copies swap is implemented
without them, an "optimized" custom swap function is now
a waste of time as well as code.
Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
Reviewed by: George Spelvin <lkml@sdf.org>
---
v2->v3: nothing changed
fs/ubifs/find.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/fs/ubifs/find.c b/fs/ubifs/find.c
index f9646835b026..5deaae7fcead 100644
--- a/fs/ubifs/find.c
+++ b/fs/ubifs/find.c
@@ -747,12 +747,6 @@ static int cmp_dirty_idx(const struct ubifs_lprops **a,
return lpa->dirty + lpa->free - lpb->dirty - lpb->free;
}
-static void swap_dirty_idx(struct ubifs_lprops **a, struct ubifs_lprops **b,
- int size)
-{
- swap(*a, *b);
-}
-
/**
* ubifs_save_dirty_idx_lnums - save an array of the most dirty index LEB nos.
* @c: the UBIFS file-system description object
@@ -772,8 +766,7 @@ int ubifs_save_dirty_idx_lnums(struct ubifs_info *c)
sizeof(void *) * c->dirty_idx.cnt);
/* Sort it so that the dirtiest is now at the end */
sort(c->dirty_idx.arr, c->dirty_idx.cnt, sizeof(void *),
- (int (*)(const void *, const void *))cmp_dirty_idx,
- (void (*)(void *, void *, int))swap_dirty_idx);
+ (int (*)(const void *, const void *))cmp_dirty_idx, NULL);
dbg_find("found %d dirty index LEBs", c->dirty_idx.cnt);
if (c->dirty_idx.cnt)
dbg_find("dirtiest index LEB is %d with dirty %d and free %d",
--
2.21.0
^ permalink raw reply related
* [PATCH 1/6] mm: change locked_vm's type from unsigned long to atomic64_t
From: Daniel Jordan @ 2019-04-02 20:41 UTC (permalink / raw)
To: akpm
Cc: Davidlohr Bueso, kvm, Alan Tull, Alexey Kardashevskiy, linux-fpga,
linux-kernel, kvm-ppc, daniel.m.jordan, linux-mm, Alex Williamson,
Moritz Fischer, Christoph Lameter, linuxppc-dev, Wu Hao
In-Reply-To: <20190402204158.27582-1-daniel.m.jordan@oracle.com>
Taking and dropping mmap_sem to modify a single counter, locked_vm, is
overkill when the counter could be synchronized separately.
Make mmap_sem a little less coarse by changing locked_vm to an atomic,
the 64-bit variety to avoid issues with overflow on 32-bit systems.
Signed-off-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Alan Tull <atull@kernel.org>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Christoph Lameter <cl@linux.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Moritz Fischer <mdf@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>
Cc: Wu Hao <hao.wu@intel.com>
Cc: <linux-mm@kvack.org>
Cc: <kvm@vger.kernel.org>
Cc: <kvm-ppc@vger.kernel.org>
Cc: <linuxppc-dev@lists.ozlabs.org>
Cc: <linux-fpga@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>
---
arch/powerpc/kvm/book3s_64_vio.c | 14 ++++++++------
arch/powerpc/mm/mmu_context_iommu.c | 15 ++++++++-------
drivers/fpga/dfl-afu-dma-region.c | 18 ++++++++++--------
drivers/vfio/vfio_iommu_spapr_tce.c | 17 +++++++++--------
drivers/vfio/vfio_iommu_type1.c | 10 ++++++----
fs/proc/task_mmu.c | 2 +-
include/linux/mm_types.h | 2 +-
kernel/fork.c | 2 +-
mm/debug.c | 5 +++--
mm/mlock.c | 4 ++--
mm/mmap.c | 18 +++++++++---------
mm/mremap.c | 6 +++---
12 files changed, 61 insertions(+), 52 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index f02b04973710..e7fdb6d10eeb 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -59,32 +59,34 @@ static unsigned long kvmppc_stt_pages(unsigned long tce_pages)
static long kvmppc_account_memlimit(unsigned long stt_pages, bool inc)
{
long ret = 0;
+ s64 locked_vm;
if (!current || !current->mm)
return ret; /* process exited */
down_write(¤t->mm->mmap_sem);
+ locked_vm = atomic64_read(¤t->mm->locked_vm);
if (inc) {
unsigned long locked, lock_limit;
- locked = current->mm->locked_vm + stt_pages;
+ locked = locked_vm + stt_pages;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
if (locked > lock_limit && !capable(CAP_IPC_LOCK))
ret = -ENOMEM;
else
- current->mm->locked_vm += stt_pages;
+ atomic64_add(stt_pages, ¤t->mm->locked_vm);
} else {
- if (WARN_ON_ONCE(stt_pages > current->mm->locked_vm))
- stt_pages = current->mm->locked_vm;
+ if (WARN_ON_ONCE(stt_pages > locked_vm))
+ stt_pages = locked_vm;
- current->mm->locked_vm -= stt_pages;
+ atomic64_sub(stt_pages, ¤t->mm->locked_vm);
}
pr_debug("[%d] RLIMIT_MEMLOCK KVM %c%ld %ld/%ld%s\n", current->pid,
inc ? '+' : '-',
stt_pages << PAGE_SHIFT,
- current->mm->locked_vm << PAGE_SHIFT,
+ atomic64_read(¤t->mm->locked_vm) << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK),
ret ? " - exceeded" : "");
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index e7a9c4f6bfca..8038ac24a312 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -55,30 +55,31 @@ static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
unsigned long npages, bool incr)
{
long ret = 0, locked, lock_limit;
+ s64 locked_vm;
if (!npages)
return 0;
down_write(&mm->mmap_sem);
-
+ locked_vm = atomic64_read(&mm->locked_vm);
if (incr) {
- locked = mm->locked_vm + npages;
+ locked = locked_vm + npages;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
if (locked > lock_limit && !capable(CAP_IPC_LOCK))
ret = -ENOMEM;
else
- mm->locked_vm += npages;
+ atomic64_add(npages, &mm->locked_vm);
} else {
- if (WARN_ON_ONCE(npages > mm->locked_vm))
- npages = mm->locked_vm;
- mm->locked_vm -= npages;
+ if (WARN_ON_ONCE(npages > locked_vm))
+ npages = locked_vm;
+ atomic64_sub(npages, &mm->locked_vm);
}
pr_debug("[%d] RLIMIT_MEMLOCK HASH64 %c%ld %ld/%ld\n",
current ? current->pid : 0,
incr ? '+' : '-',
npages << PAGE_SHIFT,
- mm->locked_vm << PAGE_SHIFT,
+ atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
up_write(&mm->mmap_sem);
diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
index e18a786fc943..08132fd9b6b7 100644
--- a/drivers/fpga/dfl-afu-dma-region.c
+++ b/drivers/fpga/dfl-afu-dma-region.c
@@ -45,6 +45,7 @@ void afu_dma_region_init(struct dfl_feature_platform_data *pdata)
static int afu_dma_adjust_locked_vm(struct device *dev, long npages, bool incr)
{
unsigned long locked, lock_limit;
+ s64 locked_vm;
int ret = 0;
/* the task is exiting. */
@@ -53,24 +54,25 @@ static int afu_dma_adjust_locked_vm(struct device *dev, long npages, bool incr)
down_write(¤t->mm->mmap_sem);
+ locked_vm = atomic64_read(¤t->mm->locked_vm);
if (incr) {
- locked = current->mm->locked_vm + npages;
+ locked = locked_vm + npages;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
if (locked > lock_limit && !capable(CAP_IPC_LOCK))
ret = -ENOMEM;
else
- current->mm->locked_vm += npages;
+ atomic64_add(npages, ¤t->mm->locked_vm);
} else {
- if (WARN_ON_ONCE(npages > current->mm->locked_vm))
- npages = current->mm->locked_vm;
- current->mm->locked_vm -= npages;
+ if (WARN_ON_ONCE(npages > locked_vm))
+ npages = locked_vm;
+ atomic64_sub(npages, ¤t->mm->locked_vm);
}
- dev_dbg(dev, "[%d] RLIMIT_MEMLOCK %c%ld %ld/%ld%s\n", current->pid,
+ dev_dbg(dev, "[%d] RLIMIT_MEMLOCK %c%ld %lld/%lu%s\n", current->pid,
incr ? '+' : '-', npages << PAGE_SHIFT,
- current->mm->locked_vm << PAGE_SHIFT, rlimit(RLIMIT_MEMLOCK),
- ret ? "- exceeded" : "");
+ (s64)atomic64_read(¤t->mm->locked_vm) << PAGE_SHIFT,
+ rlimit(RLIMIT_MEMLOCK), ret ? "- exceeded" : "");
up_write(¤t->mm->mmap_sem);
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 8dbb270998f4..e7d787e5d839 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -36,7 +36,8 @@ static void tce_iommu_detach_group(void *iommu_data,
static long try_increment_locked_vm(struct mm_struct *mm, long npages)
{
- long ret = 0, locked, lock_limit;
+ long ret = 0, lock_limit;
+ s64 locked;
if (WARN_ON_ONCE(!mm))
return -EPERM;
@@ -45,16 +46,16 @@ static long try_increment_locked_vm(struct mm_struct *mm, long npages)
return 0;
down_write(&mm->mmap_sem);
- locked = mm->locked_vm + npages;
+ locked = atomic64_read(&mm->locked_vm) + npages;
lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
if (locked > lock_limit && !capable(CAP_IPC_LOCK))
ret = -ENOMEM;
else
- mm->locked_vm += npages;
+ atomic64_add(npages, &mm->locked_vm);
pr_debug("[%d] RLIMIT_MEMLOCK +%ld %ld/%ld%s\n", current->pid,
npages << PAGE_SHIFT,
- mm->locked_vm << PAGE_SHIFT,
+ atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK),
ret ? " - exceeded" : "");
@@ -69,12 +70,12 @@ static void decrement_locked_vm(struct mm_struct *mm, long npages)
return;
down_write(&mm->mmap_sem);
- if (WARN_ON_ONCE(npages > mm->locked_vm))
- npages = mm->locked_vm;
- mm->locked_vm -= npages;
+ if (WARN_ON_ONCE(npages > atomic64_read(&mm->locked_vm)))
+ npages = atomic64_read(&mm->locked_vm);
+ atomic64_sub(npages, &mm->locked_vm);
pr_debug("[%d] RLIMIT_MEMLOCK -%ld %ld/%ld\n", current->pid,
npages << PAGE_SHIFT,
- mm->locked_vm << PAGE_SHIFT,
+ atomic64_read(&mm->locked_vm) << PAGE_SHIFT,
rlimit(RLIMIT_MEMLOCK));
up_write(&mm->mmap_sem);
}
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 73652e21efec..5b2878697286 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -270,18 +270,19 @@ static int vfio_lock_acct(struct vfio_dma *dma, long npage, bool async)
if (!ret) {
if (npage > 0) {
if (!dma->lock_cap) {
+ s64 locked_vm = atomic64_read(&mm->locked_vm);
unsigned long limit;
limit = task_rlimit(dma->task,
RLIMIT_MEMLOCK) >> PAGE_SHIFT;
- if (mm->locked_vm + npage > limit)
+ if (locked_vm + npage > limit)
ret = -ENOMEM;
}
}
if (!ret)
- mm->locked_vm += npage;
+ atomic64_add(npage, &mm->locked_vm);
up_write(&mm->mmap_sem);
}
@@ -401,6 +402,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
long ret, pinned = 0, lock_acct = 0;
bool rsvd;
dma_addr_t iova = vaddr - dma->vaddr + dma->iova;
+ atomic64_t *locked_vm = ¤t->mm->locked_vm;
/* This code path is only user initiated */
if (!current->mm)
@@ -418,7 +420,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
* pages are already counted against the user.
*/
if (!rsvd && !vfio_find_vpfn(dma, iova)) {
- if (!dma->lock_cap && current->mm->locked_vm + 1 > limit) {
+ if (!dma->lock_cap && atomic64_read(locked_vm) + 1 > limit) {
put_pfn(*pfn_base, dma->prot);
pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
limit << PAGE_SHIFT);
@@ -445,7 +447,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
if (!rsvd && !vfio_find_vpfn(dma, iova)) {
if (!dma->lock_cap &&
- current->mm->locked_vm + lock_acct + 1 > limit) {
+ atomic64_read(locked_vm) + lock_acct + 1 > limit) {
put_pfn(pfn, dma->prot);
pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
__func__, limit << PAGE_SHIFT);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 92a91e7816d8..61da4b24d0e0 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -58,7 +58,7 @@ void task_mem(struct seq_file *m, struct mm_struct *mm)
swap = get_mm_counter(mm, MM_SWAPENTS);
SEQ_PUT_DEC("VmPeak:\t", hiwater_vm);
SEQ_PUT_DEC(" kB\nVmSize:\t", total_vm);
- SEQ_PUT_DEC(" kB\nVmLck:\t", mm->locked_vm);
+ SEQ_PUT_DEC(" kB\nVmLck:\t", atomic64_read(&mm->locked_vm));
SEQ_PUT_DEC(" kB\nVmPin:\t", atomic64_read(&mm->pinned_vm));
SEQ_PUT_DEC(" kB\nVmHWM:\t", hiwater_rss);
SEQ_PUT_DEC(" kB\nVmRSS:\t", total_rss);
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 7eade9132f02..5059b99a0827 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -410,7 +410,7 @@ struct mm_struct {
unsigned long hiwater_vm; /* High-water virtual memory usage */
unsigned long total_vm; /* Total pages mapped */
- unsigned long locked_vm; /* Pages that have PG_mlocked set */
+ atomic64_t locked_vm; /* Pages that have PG_mlocked set */
atomic64_t pinned_vm; /* Refcount permanently increased */
unsigned long data_vm; /* VM_WRITE & ~VM_SHARED & ~VM_STACK */
unsigned long exec_vm; /* VM_EXEC & ~VM_WRITE & ~VM_STACK */
diff --git a/kernel/fork.c b/kernel/fork.c
index 9dcd18aa210b..56be8cdc7b4a 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -979,7 +979,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
mm->core_state = NULL;
mm_pgtables_bytes_init(mm);
mm->map_count = 0;
- mm->locked_vm = 0;
+ atomic64_set(&mm->locked_vm, 0);
atomic64_set(&mm->pinned_vm, 0);
memset(&mm->rss_stat, 0, sizeof(mm->rss_stat));
spin_lock_init(&mm->page_table_lock);
diff --git a/mm/debug.c b/mm/debug.c
index eee9c221280c..b9cd71927d3c 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -136,7 +136,7 @@ void dump_mm(const struct mm_struct *mm)
#endif
"mmap_base %lu mmap_legacy_base %lu highest_vm_end %lu\n"
"pgd %px mm_users %d mm_count %d pgtables_bytes %lu map_count %d\n"
- "hiwater_rss %lx hiwater_vm %lx total_vm %lx locked_vm %lx\n"
+ "hiwater_rss %lx hiwater_vm %lx total_vm %lx locked_vm %llx\n"
"pinned_vm %llx data_vm %lx exec_vm %lx stack_vm %lx\n"
"start_code %lx end_code %lx start_data %lx end_data %lx\n"
"start_brk %lx brk %lx start_stack %lx\n"
@@ -167,7 +167,8 @@ void dump_mm(const struct mm_struct *mm)
atomic_read(&mm->mm_count),
mm_pgtables_bytes(mm),
mm->map_count,
- mm->hiwater_rss, mm->hiwater_vm, mm->total_vm, mm->locked_vm,
+ mm->hiwater_rss, mm->hiwater_vm, mm->total_vm,
+ (u64)atomic64_read(&mm->locked_vm),
(u64)atomic64_read(&mm->pinned_vm),
mm->data_vm, mm->exec_vm, mm->stack_vm,
mm->start_code, mm->end_code, mm->start_data, mm->end_data,
diff --git a/mm/mlock.c b/mm/mlock.c
index 080f3b36415b..e492a155c51a 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -562,7 +562,7 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
nr_pages = -nr_pages;
else if (old_flags & VM_LOCKED)
nr_pages = 0;
- mm->locked_vm += nr_pages;
+ atomic64_add(nr_pages, &mm->locked_vm);
/*
* vm_flags is protected by the mmap_sem held in write mode.
@@ -687,7 +687,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla
if (down_write_killable(¤t->mm->mmap_sem))
return -EINTR;
- locked += current->mm->locked_vm;
+ locked += atomic64_read(¤t->mm->locked_vm);
if ((locked > lock_limit) && (!capable(CAP_IPC_LOCK))) {
/*
* It is possible that the regions requested intersect with
diff --git a/mm/mmap.c b/mm/mmap.c
index 41eb48d9b527..03576c1d530c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1339,7 +1339,7 @@ static inline int mlock_future_check(struct mm_struct *mm,
/* mlock MCL_FUTURE? */
if (flags & VM_LOCKED) {
locked = len >> PAGE_SHIFT;
- locked += mm->locked_vm;
+ locked += atomic64_read(&mm->locked_vm);
lock_limit = rlimit(RLIMIT_MEMLOCK);
lock_limit >>= PAGE_SHIFT;
if (locked > lock_limit && !capable(CAP_IPC_LOCK))
@@ -1825,7 +1825,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
vma == get_gate_vma(current->mm))
vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
else
- mm->locked_vm += (len >> PAGE_SHIFT);
+ atomic64_add(len >> PAGE_SHIFT, &mm->locked_vm);
}
if (file)
@@ -2301,7 +2301,7 @@ static int acct_stack_growth(struct vm_area_struct *vma,
if (vma->vm_flags & VM_LOCKED) {
unsigned long locked;
unsigned long limit;
- locked = mm->locked_vm + grow;
+ locked = atomic64_read(&mm->locked_vm) + grow;
limit = rlimit(RLIMIT_MEMLOCK);
limit >>= PAGE_SHIFT;
if (locked > limit && !capable(CAP_IPC_LOCK))
@@ -2395,7 +2395,7 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
*/
spin_lock(&mm->page_table_lock);
if (vma->vm_flags & VM_LOCKED)
- mm->locked_vm += grow;
+ atomic64_add(grow, &mm->locked_vm);
vm_stat_account(mm, vma->vm_flags, grow);
anon_vma_interval_tree_pre_update_vma(vma);
vma->vm_end = address;
@@ -2475,7 +2475,7 @@ int expand_downwards(struct vm_area_struct *vma,
*/
spin_lock(&mm->page_table_lock);
if (vma->vm_flags & VM_LOCKED)
- mm->locked_vm += grow;
+ atomic64_add(grow, &mm->locked_vm);
vm_stat_account(mm, vma->vm_flags, grow);
anon_vma_interval_tree_pre_update_vma(vma);
vma->vm_start = address;
@@ -2796,11 +2796,11 @@ int __do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
/*
* unlock any mlock()ed ranges before detaching vmas
*/
- if (mm->locked_vm) {
+ if (atomic64_read(&mm->locked_vm)) {
struct vm_area_struct *tmp = vma;
while (tmp && tmp->vm_start < end) {
if (tmp->vm_flags & VM_LOCKED) {
- mm->locked_vm -= vma_pages(tmp);
+ atomic64_sub(vma_pages(tmp), &mm->locked_vm);
munlock_vma_pages_all(tmp);
}
@@ -3043,7 +3043,7 @@ static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long fla
mm->total_vm += len >> PAGE_SHIFT;
mm->data_vm += len >> PAGE_SHIFT;
if (flags & VM_LOCKED)
- mm->locked_vm += (len >> PAGE_SHIFT);
+ atomic64_add(len >> PAGE_SHIFT, &mm->locked_vm);
vma->vm_flags |= VM_SOFTDIRTY;
return 0;
}
@@ -3115,7 +3115,7 @@ void exit_mmap(struct mm_struct *mm)
up_write(&mm->mmap_sem);
}
- if (mm->locked_vm) {
+ if (atomic64_read(&mm->locked_vm)) {
vma = mm->mmap;
while (vma) {
if (vma->vm_flags & VM_LOCKED)
diff --git a/mm/mremap.c b/mm/mremap.c
index e3edef6b7a12..9a4046bb2875 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -422,7 +422,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
}
if (vm_flags & VM_LOCKED) {
- mm->locked_vm += new_len >> PAGE_SHIFT;
+ atomic64_add(new_len >> PAGE_SHIFT, &mm->locked_vm);
*locked = true;
}
@@ -473,7 +473,7 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
if (vma->vm_flags & VM_LOCKED) {
unsigned long locked, lock_limit;
- locked = mm->locked_vm << PAGE_SHIFT;
+ locked = atomic64_read(&mm->locked_vm) << PAGE_SHIFT;
lock_limit = rlimit(RLIMIT_MEMLOCK);
locked += new_len - old_len;
if (locked > lock_limit && !capable(CAP_IPC_LOCK))
@@ -679,7 +679,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
vm_stat_account(mm, vma->vm_flags, pages);
if (vma->vm_flags & VM_LOCKED) {
- mm->locked_vm += pages;
+ atomic64_add(pages, &mm->locked_vm);
locked = true;
new_addr = addr;
}
--
2.21.0
^ permalink raw reply related
* [PATCH v3 3/5] ocfs2: dir, refcounttree, xattr: replace swap functions with built-in one
From: Andrey Abramov @ 2019-04-02 20:49 UTC (permalink / raw)
To: vgupta, benh, paulus, Michael Ellerman, tglx, mingo, bp, hpa, x86,
mark, jlbec, richard, dedekind1, adrian.hunter, gregkh,
naveen.n.rao, jpoimboe, Dave Chinner, darrick.wong,
ard.biesheuvel, George Spelvin, linux-snps-arc,
Linux Kernel Mailing List, linuxppc-dev, ocfs2-devel, linux-mtd,
sfr
Cc: amir73il, yuehaibing, ge.changwei, ashish.samant, piaojun,
jiangyiwen
In-Reply-To: <824481554237711@sas1-23a37bc8251c.qloud-c.yandex.net>
Replace dx_leaf_sort_swap, swap_refcount_rec and swap_xe functions
with built-in one, because they do only a simple byte to byte swap.
Since Spectre mitigations have made indirect function calls more
expensive, and the default simple byte copies swap is implemented
without them, an "optimized" custom swap function is now
a waste of time as well as code.
Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
Reviewed by: George Spelvin <lkml@sdf.org>
---
v2->v3: nothing changed
fs/ocfs2/dir.c | 13 +------------
fs/ocfs2/refcounttree.c | 13 +++----------
fs/ocfs2/xattr.c | 15 +++------------
3 files changed, 7 insertions(+), 34 deletions(-)
diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c
index c121abbdfc7d..4b86b181df0a 100644
--- a/fs/ocfs2/dir.c
+++ b/fs/ocfs2/dir.c
@@ -3529,16 +3529,6 @@ static int dx_leaf_sort_cmp(const void *a, const void *b)
return 0;
}
-static void dx_leaf_sort_swap(void *a, void *b, int size)
-{
- struct ocfs2_dx_entry *entry1 = a;
- struct ocfs2_dx_entry *entry2 = b;
-
- BUG_ON(size != sizeof(*entry1));
-
- swap(*entry1, *entry2);
-}
-
static int ocfs2_dx_leaf_same_major(struct ocfs2_dx_leaf *dx_leaf)
{
struct ocfs2_dx_entry_list *dl_list = &dx_leaf->dl_list;
@@ -3799,8 +3789,7 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
* This block is changing anyway, so we can sort it in place.
*/
sort(dx_leaf->dl_list.de_entries, num_used,
- sizeof(struct ocfs2_dx_entry), dx_leaf_sort_cmp,
- dx_leaf_sort_swap);
+ sizeof(struct ocfs2_dx_entry), dx_leaf_sort_cmp, NULL);
ocfs2_journal_dirty(handle, dx_leaf_bh);
diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 1dc9a08e8bdc..7bbc94d23a0c 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -1400,13 +1400,6 @@ static int cmp_refcount_rec_by_cpos(const void *a, const void *b)
return 0;
}
-static void swap_refcount_rec(void *a, void *b, int size)
-{
- struct ocfs2_refcount_rec *l = a, *r = b;
-
- swap(*l, *r);
-}
-
/*
* The refcount cpos are ordered by their 64bit cpos,
* But we will use the low 32 bit to be the e_cpos in the b-tree.
@@ -1482,7 +1475,7 @@ static int ocfs2_divide_leaf_refcount_block(struct buffer_head *ref_leaf_bh,
*/
sort(&rl->rl_recs, le16_to_cpu(rl->rl_used),
sizeof(struct ocfs2_refcount_rec),
- cmp_refcount_rec_by_low_cpos, swap_refcount_rec);
+ cmp_refcount_rec_by_low_cpos, NULL);
ret = ocfs2_find_refcount_split_pos(rl, &cpos, &split_index);
if (ret) {
@@ -1507,11 +1500,11 @@ static int ocfs2_divide_leaf_refcount_block(struct buffer_head *ref_leaf_bh,
sort(&rl->rl_recs, le16_to_cpu(rl->rl_used),
sizeof(struct ocfs2_refcount_rec),
- cmp_refcount_rec_by_cpos, swap_refcount_rec);
+ cmp_refcount_rec_by_cpos, NULL);
sort(&new_rl->rl_recs, le16_to_cpu(new_rl->rl_used),
sizeof(struct ocfs2_refcount_rec),
- cmp_refcount_rec_by_cpos, swap_refcount_rec);
+ cmp_refcount_rec_by_cpos, NULL);
*split_cpos = cpos;
return 0;
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index 3a24ce3deb01..b3e6f42baf78 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -4175,15 +4175,6 @@ static int cmp_xe(const void *a, const void *b)
return 0;
}
-static void swap_xe(void *a, void *b, int size)
-{
- struct ocfs2_xattr_entry *l = a, *r = b, tmp;
-
- tmp = *l;
- memcpy(l, r, sizeof(struct ocfs2_xattr_entry));
- memcpy(r, &tmp, sizeof(struct ocfs2_xattr_entry));
-}
-
/*
* When the ocfs2_xattr_block is filled up, new bucket will be created
* and all the xattr entries will be moved to the new bucket.
@@ -4249,7 +4240,7 @@ static void ocfs2_cp_xattr_block_to_bucket(struct inode *inode,
trace_ocfs2_cp_xattr_block_to_bucket_end(offset, size, off_change);
sort(target + offset, count, sizeof(struct ocfs2_xattr_entry),
- cmp_xe, swap_xe);
+ cmp_xe, NULL);
}
/*
@@ -4444,7 +4435,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
*/
sort(entries, le16_to_cpu(xh->xh_count),
sizeof(struct ocfs2_xattr_entry),
- cmp_xe_offset, swap_xe);
+ cmp_xe_offset, NULL);
/* Move all name/values to the end of the bucket. */
xe = xh->xh_entries;
@@ -4486,7 +4477,7 @@ static int ocfs2_defrag_xattr_bucket(struct inode *inode,
/* sort the entries by their name_hash. */
sort(entries, le16_to_cpu(xh->xh_count),
sizeof(struct ocfs2_xattr_entry),
- cmp_xe, swap_xe);
+ cmp_xe, NULL);
buf = bucket_buf;
for (i = 0; i < bucket->bu_blocks; i++, buf += blocksize)
--
2.21.0
^ permalink raw reply related
* [PATCH v3 2/5] powerpc: module_[32|64].c: replace swap function with built-in one
From: Andrey Abramov @ 2019-04-02 20:47 UTC (permalink / raw)
To: vgupta, benh, paulus, Michael Ellerman, tglx, mingo, bp, hpa, x86,
mark, jlbec, richard, dedekind1, adrian.hunter, gregkh,
naveen.n.rao, jpoimboe, Dave Chinner, darrick.wong,
ard.biesheuvel, George Spelvin, linux-snps-arc,
Linux Kernel Mailing List, linuxppc-dev, ocfs2-devel, linux-mtd,
sfr
Cc: malat, yamada.masahiro, npiggin
In-Reply-To: <824481554237711@sas1-23a37bc8251c.qloud-c.yandex.net>
Replace relaswap with built-in one, because relaswap
does a simple byte to byte swap.
Since Spectre mitigations have made indirect function calls more
expensive, and the default simple byte copies swap is implemented
without them, an "optimized" custom swap function is now
a waste of time as well as code.
Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
Reviewed by: George Spelvin <lkml@sdf.org>
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
---
v2->v3: nothing changed
arch/powerpc/kernel/module_32.c | 17 +----------------
arch/powerpc/kernel/module_64.c | 17 +----------------
2 files changed, 2 insertions(+), 32 deletions(-)
diff --git a/arch/powerpc/kernel/module_32.c b/arch/powerpc/kernel/module_32.c
index 88d83771f462..c311e8575d10 100644
--- a/arch/powerpc/kernel/module_32.c
+++ b/arch/powerpc/kernel/module_32.c
@@ -79,21 +79,6 @@ static int relacmp(const void *_x, const void *_y)
return 0;
}
-static void relaswap(void *_x, void *_y, int size)
-{
- uint32_t *x, *y, tmp;
- int i;
-
- y = (uint32_t *)_x;
- x = (uint32_t *)_y;
-
- for (i = 0; i < sizeof(Elf32_Rela) / sizeof(uint32_t); i++) {
- tmp = x[i];
- x[i] = y[i];
- y[i] = tmp;
- }
-}
-
/* Get the potential trampolines size required of the init and
non-init sections */
static unsigned long get_plt_size(const Elf32_Ehdr *hdr,
@@ -130,7 +115,7 @@ static unsigned long get_plt_size(const Elf32_Ehdr *hdr,
*/
sort((void *)hdr + sechdrs[i].sh_offset,
sechdrs[i].sh_size / sizeof(Elf32_Rela),
- sizeof(Elf32_Rela), relacmp, relaswap);
+ sizeof(Elf32_Rela), relacmp, NULL);
ret += count_relocs((void *)hdr
+ sechdrs[i].sh_offset,
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 8661eea78503..0c833d7f36f1 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -231,21 +231,6 @@ static int relacmp(const void *_x, const void *_y)
return 0;
}
-static void relaswap(void *_x, void *_y, int size)
-{
- uint64_t *x, *y, tmp;
- int i;
-
- y = (uint64_t *)_x;
- x = (uint64_t *)_y;
-
- for (i = 0; i < sizeof(Elf64_Rela) / sizeof(uint64_t); i++) {
- tmp = x[i];
- x[i] = y[i];
- y[i] = tmp;
- }
-}
-
/* Get size of potential trampolines required. */
static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
const Elf64_Shdr *sechdrs)
@@ -269,7 +254,7 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
*/
sort((void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size / sizeof(Elf64_Rela),
- sizeof(Elf64_Rela), relacmp, relaswap);
+ sizeof(Elf64_Rela), relacmp, NULL);
relocs += count_relocs((void *)sechdrs[i].sh_addr,
sechdrs[i].sh_size
--
2.21.0
^ permalink raw reply related
* [PATCH v3 1/5] arch/arc: unwind.c: replace swap function with built-in one
From: Andrey Abramov @ 2019-04-02 20:45 UTC (permalink / raw)
To: vgupta, benh, paulus, Michael Ellerman, tglx, mingo, bp, hpa, x86,
mark, jlbec, richard, dedekind1, adrian.hunter, gregkh,
naveen.n.rao, jpoimboe, Dave Chinner, darrick.wong,
ard.biesheuvel, George Spelvin, linux-snps-arc,
Linux Kernel Mailing List, linuxppc-dev, ocfs2-devel, linux-mtd,
sfr
Cc: mhocko, rppt
In-Reply-To: <824481554237711@sas1-23a37bc8251c.qloud-c.yandex.net>
Replace swap_eh_frame_hdr_table_entries with built-in one, because
swap_eh_frame_hdr_table_entries does a simple byte to byte swap.
Since Spectre mitigations have made indirect function calls more
expensive, and the default simple byte copies swap is implemented
without them, an "optimized" custom swap function is now
a waste of time as well as code.
Signed-off-by: Andrey Abramov <st5pub@yandex.ru>
Reviewed by: George Spelvin <lkml@sdf.org>
Acked-by: Vineet Gupta <vgupta@synopsys.com>
---
v2->v3: nothing changed
arch/arc/kernel/unwind.c | 20 ++------------------
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c
index 271e9fafa479..7610fe84afea 100644
--- a/arch/arc/kernel/unwind.c
+++ b/arch/arc/kernel/unwind.c
@@ -248,20 +248,6 @@ static int cmp_eh_frame_hdr_table_entries(const void *p1, const void *p2)
return (e1->start > e2->start) - (e1->start < e2->start);
}
-static void swap_eh_frame_hdr_table_entries(void *p1, void *p2, int size)
-{
- struct eh_frame_hdr_table_entry *e1 = p1;
- struct eh_frame_hdr_table_entry *e2 = p2;
- unsigned long v;
-
- v = e1->start;
- e1->start = e2->start;
- e2->start = v;
- v = e1->fde;
- e1->fde = e2->fde;
- e2->fde = v;
-}
-
static void init_unwind_hdr(struct unwind_table *table,
void *(*alloc) (unsigned long))
{
@@ -354,10 +340,8 @@ static void init_unwind_hdr(struct unwind_table *table,
}
WARN_ON(n != header->fde_count);
- sort(header->table,
- n,
- sizeof(*header->table),
- cmp_eh_frame_hdr_table_entries, swap_eh_frame_hdr_table_entries);
+ sort(header->table, n,
+ sizeof(*header->table), cmp_eh_frame_hdr_table_entries, NULL);
table->hdrsz = hdrSize;
smp_wmb();
--
2.21.0
^ permalink raw reply related
* [PATCH v3 0/5] simple sort swap function improvements
From: Andrey Abramov @ 2019-04-02 20:41 UTC (permalink / raw)
To: vgupta, benh, paulus, Michael Ellerman, tglx, mingo, bp, hpa, x86,
mark, jlbec, richard, dedekind1, adrian.hunter, gregkh,
naveen.n.rao, jpoimboe, Dave Chinner, darrick.wong,
ard.biesheuvel, George Spelvin, linux-snps-arc,
Linux Kernel Mailing List, linuxppc-dev, ocfs2-devel, linux-mtd,
sfr
Cc: mhocko, gustavo, peterz, amir73il, Rasmus Villemoes, kamalesh,
piaojun, yamada.masahiro, jiang.biao2, jslaby, yuehaibing, rppt,
ge.changwei, keescook, jannh, ashish.samant, npiggin, jiangyiwen,
Andy Shevchenko, lchen, malat, Morton Andrew
This is the logical continuation of the "lib/sort & lib/list_sort:
faster and smaller" series by George Spelvin (added to linux-next
recently).
Since Spectre mitigations have made indirect function calls more
expensive, and the previous patch series implements the default
simple byte copies without them, an "optimized" custom swap
function is now a waste of time as well as code.
Patches 1 to 4 replace trivial swap functions with the built-in
(which is now much faster) and are grouped by subsystem.
Being pure code deletion patches, they are sure to bring joy to
Linus's heart.
Having reviewed all call sites, only three non-trivial swap
functions remain: arch/x86/kernel/unwind_orc.c,
kernel/jump_label.c and lib/extable.c.
Patch #5 removes size argument from the swap function because:
1) It wasn't used.
2) Custom swap function knows what kind of objects it swaps,
so it already knows their sizes.
v1->v2: Only commit messages have changed to better explain
the purpose of commits. (Thanks to George Spelvin and Greg KH)
v2->v3: Patch #5 now completely removes the size argument
Andrey Abramov (5):
arch/arc: unwind.c: replace swap function with built-in one
powerpc: module_[32|64].c: replace swap function with built-in one
ocfs2: dir,refcounttree,xattr: replace swap functions with built-in
one
ubifs: find.c: replace swap function with built-in one
Lib: sort.h: remove the size argument from the swap function
arch/arc/kernel/unwind.c | 20 ++------------------
arch/powerpc/kernel/module_32.c | 17 +----------------
arch/powerpc/kernel/module_64.c | 17 +----------------
arch/x86/kernel/unwind_orc.c | 2 +-
fs/ocfs2/dir.c | 13 +------------
fs/ocfs2/refcounttree.c | 13 +++----------
fs/ocfs2/xattr.c | 15 +++------------
fs/ubifs/find.c | 9 +--------
include/linux/sort.h | 2 +-
kernel/jump_label.c | 2 +-
lib/extable.c | 2 +-
lib/sort.c | 7 +++----
12 files changed, 19 insertions(+), 100 deletions(-)
--
2.21.0
^ 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