linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
	Michael Ellerman <mpe@ellerman.id.au>,
	x86@kernel.org
Cc: linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, Kees Cook <keescook@chromium.org>,
	linux-kernel@vger.kernel.org
Subject: Re: remove the last set_fs() in common code, and remove it for x86 and powerpc
Date: Tue, 18 Aug 2020 19:46:22 +0200	[thread overview]
Message-ID: <319d15b1-cb4a-a7b4-3082-12bb30eb5143@csgroup.eu> (raw)
In-Reply-To: <20200817073212.830069-1-hch@lst.de>



Le 17/08/2020 à 09:32, Christoph Hellwig a écrit :
> Hi all,
> 
> this series removes the last set_fs() used to force a kernel address
> space for the uaccess code in the kernel read/write/splice code, and then
> stops implementing the address space overrides entirely for x86 and
> powerpc.
> 
> The file system part has been posted a few times, and the read/write side
> has been pretty much unchanced.  For splice this series drops the
> conversion of the seq_file and sysctl code to the iter ops, and thus loses
> the splice support for them.  The reasons for that is that it caused a lot
> of churn for not much use - splice for these small files really isn't much
> of a win, even if existing userspace uses it.  All callers I found do the
> proper fallback, but if this turns out to be an issue the conversion can
> be resurrected.

I like this series.

I gave it a go on my powerpc mpc832x. I tested it on top of my newest 
series that reworks the 32 bits signal handlers (see 
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=196278) 
with the microbenchmark test used is that series.

With KUAP activated, on top of signal32 rework, performance is boosted 
as system time for the microbenchmark goes from 1.73s down to 1.56s, 
that is 10% quicker

Surprisingly, with the kernel as is today without my signal's series, 
your series degrades performance slightly (from 2.55s to 2.64s ie 3.5% 
slower).


I also observe, in both cases, a degradation on

	dd if=/dev/zero of=/dev/null count=1M

Without your series, it runs in 5.29 seconds.
With your series, it runs in 5.82 seconds, that is 10% more time.

Christophe


> 
> Besides x86 and powerpc I plan to eventually convert all other
> architectures, although this will be a slow process, starting with the
> easier ones once the infrastructure is merged.  The process to convert
> architectures is roughtly:
> 
>   - ensure there is no set_fs(KERNEL_DS) left in arch specific code
>   - implement __get_kernel_nofault and __put_kernel_nofault
>   - remove the arch specific address limitation functionality
> 
> Diffstat:
>   arch/Kconfig                           |    3
>   arch/alpha/Kconfig                     |    1
>   arch/arc/Kconfig                       |    1
>   arch/arm/Kconfig                       |    1
>   arch/arm64/Kconfig                     |    1
>   arch/c6x/Kconfig                       |    1
>   arch/csky/Kconfig                      |    1
>   arch/h8300/Kconfig                     |    1
>   arch/hexagon/Kconfig                   |    1
>   arch/ia64/Kconfig                      |    1
>   arch/m68k/Kconfig                      |    1
>   arch/microblaze/Kconfig                |    1
>   arch/mips/Kconfig                      |    1
>   arch/nds32/Kconfig                     |    1
>   arch/nios2/Kconfig                     |    1
>   arch/openrisc/Kconfig                  |    1
>   arch/parisc/Kconfig                    |    1
>   arch/powerpc/include/asm/processor.h   |    7 -
>   arch/powerpc/include/asm/thread_info.h |    5 -
>   arch/powerpc/include/asm/uaccess.h     |   78 ++++++++-----------
>   arch/powerpc/kernel/signal.c           |    3
>   arch/powerpc/lib/sstep.c               |    6 -
>   arch/riscv/Kconfig                     |    1
>   arch/s390/Kconfig                      |    1
>   arch/sh/Kconfig                        |    1
>   arch/sparc/Kconfig                     |    1
>   arch/um/Kconfig                        |    1
>   arch/x86/ia32/ia32_aout.c              |    1
>   arch/x86/include/asm/page_32_types.h   |   11 ++
>   arch/x86/include/asm/page_64_types.h   |   38 +++++++++
>   arch/x86/include/asm/processor.h       |   60 ---------------
>   arch/x86/include/asm/thread_info.h     |    2
>   arch/x86/include/asm/uaccess.h         |   26 ------
>   arch/x86/kernel/asm-offsets.c          |    3
>   arch/x86/lib/getuser.S                 |   28 ++++---
>   arch/x86/lib/putuser.S                 |   21 +++--
>   arch/xtensa/Kconfig                    |    1
>   drivers/char/mem.c                     |   16 ----
>   drivers/misc/lkdtm/bugs.c              |    2
>   drivers/misc/lkdtm/core.c              |    4 +
>   drivers/misc/lkdtm/usercopy.c          |    2
>   fs/read_write.c                        |   69 ++++++++++-------
>   fs/splice.c                            |  130 +++------------------------------
>   include/linux/fs.h                     |    2
>   include/linux/uaccess.h                |   18 ++++
>   lib/test_bitmap.c                      |   10 ++
>   46 files changed, 235 insertions(+), 332 deletions(-)
> 

  parent reply	other threads:[~2020-08-18 17:46 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-17  7:32 remove the last set_fs() in common code, and remove it for x86 and powerpc Christoph Hellwig
2020-08-17  7:32 ` [PATCH 01/11] mem: remove duplicate ops for /dev/zero and /dev/null Christoph Hellwig
2020-08-18 19:33   ` Kees Cook
2020-08-17  7:32 ` [PATCH 02/11] fs: don't allow kernel reads and writes without iter ops Christoph Hellwig
2020-08-18 19:34   ` Kees Cook
2020-08-17  7:32 ` [PATCH 03/11] fs: don't allow splice read/write without explicit ops Christoph Hellwig
2020-08-18 19:39   ` Kees Cook
2020-08-18 19:54     ` Christoph Hellwig
2020-08-18 19:58       ` Kees Cook
2020-08-18 20:07         ` Christoph Hellwig
2020-08-17  7:32 ` [PATCH 04/11] uaccess: add infrastructure for kernel builds with set_fs() Christoph Hellwig
2020-08-18 19:40   ` Kees Cook
2020-08-17  7:32 ` [PATCH 05/11] test_bitmap: skip user bitmap tests for !CONFIG_SET_FS Christoph Hellwig
2020-08-17  7:50   ` Christophe Leroy
2020-08-17  7:52     ` Christoph Hellwig
2020-08-18 19:43   ` Kees Cook
2020-08-17  7:32 ` [PATCH 06/11] lkdtm: disable set_fs-based " Christoph Hellwig
2020-08-18 19:32   ` Kees Cook
2020-08-17  7:32 ` [PATCH 07/11] x86: move PAGE_OFFSET, TASK_SIZE & friends to page_{32,64}_types.h Christoph Hellwig
2020-08-18 19:27   ` Kees Cook
2020-08-17  7:32 ` [PATCH 08/11] x86: make TASK_SIZE_MAX usable from assembly code Christoph Hellwig
2020-08-18 19:44   ` Kees Cook
2020-08-18 19:55     ` Christoph Hellwig
2020-08-18 19:59       ` Kees Cook
2020-08-18 20:00         ` Christoph Hellwig
2020-08-18 20:08           ` Kees Cook
2020-08-17  7:32 ` [PATCH 09/11] x86: remove address space overrides using set_fs() Christoph Hellwig
2020-08-17  8:23   ` David Laight
2020-08-27  9:37     ` 'Christoph Hellwig'
2020-08-18 19:46   ` Kees Cook
2020-08-17  7:32 ` [PATCH 10/11] powerpc: use non-set_fs based maccess routines Christoph Hellwig
2020-08-17 15:47   ` Christophe Leroy
2020-08-17  7:32 ` [PATCH 11/11] powerpc: remove address space overrides using set_fs() Christoph Hellwig
2020-08-17  7:39 ` remove the last set_fs() in common code, and remove it for x86 and powerpc Christoph Hellwig
2020-08-18 17:46 ` Christophe Leroy [this message]
2020-08-18 18:05   ` Christoph Hellwig
2020-08-18 18:23     ` Christophe Leroy
2020-08-19  7:16       ` Christophe Leroy
2020-08-19  7:22         ` iter and normal ops on /dev/zero & co, was " Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=319d15b1-cb4a-a7b4-3082-12bb30eb5143@csgroup.eu \
    --to=christophe.leroy@csgroup.eu \
    --cc=hch@lst.de \
    --cc=keescook@chromium.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=viro@zeniv.linux.org.uk \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).