All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christophe Leroy <christophe.leroy@c-s.fr>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
Date: Wed, 23 Jan 2019 11:04:16 +0100	[thread overview]
Message-ID: <2e700e1c-5bd9-652e-b535-68a89dd703a1@c-s.fr> (raw)
In-Reply-To: <cover.1547195976.git.christophe.leroy@c-s.fr>



Le 12/01/2019 à 10:55, Christophe Leroy a écrit :
> The purpose of this serie is to activate CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
> 
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are
> leaked, making a number of attacks more difficult.

I ran null_syscall and context_switch benchmark selftests and the result 
is surprising. There is slight degradation in context_switch and a 
significant one on null_syscall:

Without the serie:

~# chrt -f 98 ./context_switch --no-altivec --no-vector --no-fp
55542
55562
55564
55562
55568
...

~# ./null_syscall
    2546.71 ns     336.17 cycles


With the serie:

~# chrt -f 98 ./context_switch --no-altivec --no-vector --no-fp
55138
55142
55152
55144
55142

~# ./null_syscall
    3479.54 ns     459.30 cycles

So 0,8% less context switches per second and 37% more time for one syscall ?


Any idea ?

Christophe


> 
> Changes since v12:
>   - Patch 1: Taken comment from Mike (re-introduced the 'panic' in case memblock allocation fails in setup_64.c
>   - Patch 1: Added alloc_stack() function in setup_32.c to also panic in case of allocation failure.
> 
> Changes since v11:
>   - Rebased on 81775f5563fa ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>   - Added a first patch to change memblock allocs to functions returning virtual addrs. This removes
>     the memset() which were the only remaining stuff in irq_ctx_init() and exc_lvl_ctx_init() at the end.
>   - dropping irq_ctx_init() and exc_lvl_ctx_init() in patch 5 (powerpc: Activate CONFIG_THREAD_INFO_IN_TASK)
>   - A few cosmetic changes in commit log and code.
> 
> Changes since v10:
>   - Rebased on 21622a0d2023 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>    ==> Fixed conflict in setup_32.S
> 
> Changes since v9:
>   - Rebased on 183cbf93be88 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>    ==> Fixed conflict on xmon
> 
> Changes since v8:
>   - Rebased on e589b79e40d9 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>    ==> Main impact was conflicts due to commit 9a8dd708d547 ("memblock: rename memblock_alloc{_nid,_try_nid} to memblock_phys_alloc*")
> 
> Changes since v7:
>   - Rebased on fb6c6ce7907d ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
> 
> Changes since v6:
>   - Fixed validate_sp() to exclude NULL sp in 'regain entire stack space' patch (early crash with CONFIG_KMEMLEAK)
> 
> Changes since v5:
>   - Fixed livepatch_sp setup by using end_of_stack() instead of hardcoding
>   - Fixed PPC_BPF_LOAD_CPU() macro
> 
> Changes since v4:
>   - Fixed a build failure on 32bits SMP when include/generated/asm-offsets.h is not
>   already existing, was due to spaces instead of a tab in the Makefile
> 
> Changes since RFC v3: (based on Nick's review)
>   - Renamed task_size.h to task_size_user64.h to better relate to what it contains.
>   - Handling of the isolation of thread_info cpu field inside CONFIG_SMP #ifdefs moved to a separate patch.
>   - Removed CURRENT_THREAD_INFO macro completely.
>   - Added a guard in asm/smp.h to avoid build failure before _TASK_CPU is defined.
>   - Added a patch at the end to rename 'tp' pointers to 'sp' pointers
>   - Renamed 'tp' into 'sp' pointers in preparation patch when relevant
>   - Fixed a few commit logs
>   - Fixed checkpatch report.
> 
> Changes since RFC v2:
>   - Removed the modification of names in asm-offsets
>   - Created a rule in arch/powerpc/Makefile to append the offset of current->cpu in CFLAGS
>   - Modified asm/smp.h to use the offset set in CFLAGS
>   - Squashed the renaming of THREAD_INFO to TASK_STACK in the preparation patch
>   - Moved the modification of current_pt_regs in the patch activating CONFIG_THREAD_INFO_IN_TASK
> 
> Changes since RFC v1:
>   - Removed the first patch which was modifying header inclusion order in timer
>   - Modified some names in asm-offsets to avoid conflicts when including asm-offsets in C files
>   - Modified asm/smp.h to avoid having to include linux/sched.h (using asm-offsets instead)
>   - Moved some changes from the activation patch to the preparation patch.
> 
> Christophe Leroy (10):
>    powerpc/irq: use memblock functions returning virtual address
>    book3s/64: avoid circular header inclusion in mmu-hash.h
>    powerpc: Only use task_struct 'cpu' field on SMP
>    powerpc: Prepare for moving thread_info into task_struct
>    powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
>    powerpc: regain entire stack space
>    powerpc: 'current_set' is now a table of task_struct pointers
>    powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
>    powerpc/64: Remove CURRENT_THREAD_INFO
>    powerpc: clean stack pointers naming
> 
>   arch/powerpc/Kconfig                           |   1 +
>   arch/powerpc/Makefile                          |   7 ++
>   arch/powerpc/include/asm/asm-prototypes.h      |   4 +-
>   arch/powerpc/include/asm/book3s/64/mmu-hash.h  |   2 +-
>   arch/powerpc/include/asm/exception-64s.h       |   4 +-
>   arch/powerpc/include/asm/irq.h                 |  18 ++--
>   arch/powerpc/include/asm/livepatch.h           |   6 +-
>   arch/powerpc/include/asm/processor.h           |  39 +--------
>   arch/powerpc/include/asm/ptrace.h              |   2 +-
>   arch/powerpc/include/asm/reg.h                 |   2 +-
>   arch/powerpc/include/asm/smp.h                 |  17 +++-
>   arch/powerpc/include/asm/task_size_user64.h    |  42 +++++++++
>   arch/powerpc/include/asm/thread_info.h         |  19 -----
>   arch/powerpc/kernel/asm-offsets.c              |  10 ++-
>   arch/powerpc/kernel/entry_32.S                 |  66 +++++---------
>   arch/powerpc/kernel/entry_64.S                 |  12 +--
>   arch/powerpc/kernel/epapr_hcalls.S             |   5 +-
>   arch/powerpc/kernel/exceptions-64e.S           |  13 +--
>   arch/powerpc/kernel/exceptions-64s.S           |   2 +-
>   arch/powerpc/kernel/head_32.S                  |  14 +--
>   arch/powerpc/kernel/head_40x.S                 |   4 +-
>   arch/powerpc/kernel/head_44x.S                 |   8 +-
>   arch/powerpc/kernel/head_64.S                  |   1 +
>   arch/powerpc/kernel/head_8xx.S                 |   2 +-
>   arch/powerpc/kernel/head_booke.h               |  12 +--
>   arch/powerpc/kernel/head_fsl_booke.S           |  16 ++--
>   arch/powerpc/kernel/idle_6xx.S                 |   8 +-
>   arch/powerpc/kernel/idle_book3e.S              |   2 +-
>   arch/powerpc/kernel/idle_e500.S                |   8 +-
>   arch/powerpc/kernel/idle_power4.S              |   2 +-
>   arch/powerpc/kernel/irq.c                      | 114 +++----------------------
>   arch/powerpc/kernel/kgdb.c                     |  28 ------
>   arch/powerpc/kernel/machine_kexec_64.c         |   6 +-
>   arch/powerpc/kernel/misc_32.S                  |  17 ++--
>   arch/powerpc/kernel/process.c                  |  40 ++++-----
>   arch/powerpc/kernel/setup-common.c             |   2 +-
>   arch/powerpc/kernel/setup_32.c                 |  25 +++---
>   arch/powerpc/kernel/setup_64.c                 |  51 +++--------
>   arch/powerpc/kernel/smp.c                      |  16 ++--
>   arch/powerpc/kernel/trace/ftrace_64_mprofile.S |   6 +-
>   arch/powerpc/kvm/book3s_hv_hmi.c               |   1 +
>   arch/powerpc/mm/hash_low_32.S                  |  14 ++-
>   arch/powerpc/net/bpf_jit32.h                   |   5 +-
>   arch/powerpc/sysdev/6xx-suspend.S              |   5 +-
>   arch/powerpc/xmon/xmon.c                       |   2 +-
>   45 files changed, 249 insertions(+), 431 deletions(-)
>   create mode 100644 arch/powerpc/include/asm/task_size_user64.h
> 

WARNING: multiple messages have this Message-ID (diff)
From: Christophe Leroy <christophe.leroy@c-s.fr>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Mike Rapoport <rppt@linux.ibm.com>
Subject: Re: [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
Date: Wed, 23 Jan 2019 11:04:16 +0100	[thread overview]
Message-ID: <2e700e1c-5bd9-652e-b535-68a89dd703a1@c-s.fr> (raw)
In-Reply-To: <cover.1547195976.git.christophe.leroy@c-s.fr>



Le 12/01/2019 à 10:55, Christophe Leroy a écrit :
> The purpose of this serie is to activate CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
> 
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are
> leaked, making a number of attacks more difficult.

I ran null_syscall and context_switch benchmark selftests and the result 
is surprising. There is slight degradation in context_switch and a 
significant one on null_syscall:

Without the serie:

~# chrt -f 98 ./context_switch --no-altivec --no-vector --no-fp
55542
55562
55564
55562
55568
...

~# ./null_syscall
    2546.71 ns     336.17 cycles


With the serie:

~# chrt -f 98 ./context_switch --no-altivec --no-vector --no-fp
55138
55142
55152
55144
55142

~# ./null_syscall
    3479.54 ns     459.30 cycles

So 0,8% less context switches per second and 37% more time for one syscall ?


Any idea ?

Christophe


> 
> Changes since v12:
>   - Patch 1: Taken comment from Mike (re-introduced the 'panic' in case memblock allocation fails in setup_64.c
>   - Patch 1: Added alloc_stack() function in setup_32.c to also panic in case of allocation failure.
> 
> Changes since v11:
>   - Rebased on 81775f5563fa ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>   - Added a first patch to change memblock allocs to functions returning virtual addrs. This removes
>     the memset() which were the only remaining stuff in irq_ctx_init() and exc_lvl_ctx_init() at the end.
>   - dropping irq_ctx_init() and exc_lvl_ctx_init() in patch 5 (powerpc: Activate CONFIG_THREAD_INFO_IN_TASK)
>   - A few cosmetic changes in commit log and code.
> 
> Changes since v10:
>   - Rebased on 21622a0d2023 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>    ==> Fixed conflict in setup_32.S
> 
> Changes since v9:
>   - Rebased on 183cbf93be88 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>    ==> Fixed conflict on xmon
> 
> Changes since v8:
>   - Rebased on e589b79e40d9 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>    ==> Main impact was conflicts due to commit 9a8dd708d547 ("memblock: rename memblock_alloc{_nid,_try_nid} to memblock_phys_alloc*")
> 
> Changes since v7:
>   - Rebased on fb6c6ce7907d ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
> 
> Changes since v6:
>   - Fixed validate_sp() to exclude NULL sp in 'regain entire stack space' patch (early crash with CONFIG_KMEMLEAK)
> 
> Changes since v5:
>   - Fixed livepatch_sp setup by using end_of_stack() instead of hardcoding
>   - Fixed PPC_BPF_LOAD_CPU() macro
> 
> Changes since v4:
>   - Fixed a build failure on 32bits SMP when include/generated/asm-offsets.h is not
>   already existing, was due to spaces instead of a tab in the Makefile
> 
> Changes since RFC v3: (based on Nick's review)
>   - Renamed task_size.h to task_size_user64.h to better relate to what it contains.
>   - Handling of the isolation of thread_info cpu field inside CONFIG_SMP #ifdefs moved to a separate patch.
>   - Removed CURRENT_THREAD_INFO macro completely.
>   - Added a guard in asm/smp.h to avoid build failure before _TASK_CPU is defined.
>   - Added a patch at the end to rename 'tp' pointers to 'sp' pointers
>   - Renamed 'tp' into 'sp' pointers in preparation patch when relevant
>   - Fixed a few commit logs
>   - Fixed checkpatch report.
> 
> Changes since RFC v2:
>   - Removed the modification of names in asm-offsets
>   - Created a rule in arch/powerpc/Makefile to append the offset of current->cpu in CFLAGS
>   - Modified asm/smp.h to use the offset set in CFLAGS
>   - Squashed the renaming of THREAD_INFO to TASK_STACK in the preparation patch
>   - Moved the modification of current_pt_regs in the patch activating CONFIG_THREAD_INFO_IN_TASK
> 
> Changes since RFC v1:
>   - Removed the first patch which was modifying header inclusion order in timer
>   - Modified some names in asm-offsets to avoid conflicts when including asm-offsets in C files
>   - Modified asm/smp.h to avoid having to include linux/sched.h (using asm-offsets instead)
>   - Moved some changes from the activation patch to the preparation patch.
> 
> Christophe Leroy (10):
>    powerpc/irq: use memblock functions returning virtual address
>    book3s/64: avoid circular header inclusion in mmu-hash.h
>    powerpc: Only use task_struct 'cpu' field on SMP
>    powerpc: Prepare for moving thread_info into task_struct
>    powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
>    powerpc: regain entire stack space
>    powerpc: 'current_set' is now a table of task_struct pointers
>    powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
>    powerpc/64: Remove CURRENT_THREAD_INFO
>    powerpc: clean stack pointers naming
> 
>   arch/powerpc/Kconfig                           |   1 +
>   arch/powerpc/Makefile                          |   7 ++
>   arch/powerpc/include/asm/asm-prototypes.h      |   4 +-
>   arch/powerpc/include/asm/book3s/64/mmu-hash.h  |   2 +-
>   arch/powerpc/include/asm/exception-64s.h       |   4 +-
>   arch/powerpc/include/asm/irq.h                 |  18 ++--
>   arch/powerpc/include/asm/livepatch.h           |   6 +-
>   arch/powerpc/include/asm/processor.h           |  39 +--------
>   arch/powerpc/include/asm/ptrace.h              |   2 +-
>   arch/powerpc/include/asm/reg.h                 |   2 +-
>   arch/powerpc/include/asm/smp.h                 |  17 +++-
>   arch/powerpc/include/asm/task_size_user64.h    |  42 +++++++++
>   arch/powerpc/include/asm/thread_info.h         |  19 -----
>   arch/powerpc/kernel/asm-offsets.c              |  10 ++-
>   arch/powerpc/kernel/entry_32.S                 |  66 +++++---------
>   arch/powerpc/kernel/entry_64.S                 |  12 +--
>   arch/powerpc/kernel/epapr_hcalls.S             |   5 +-
>   arch/powerpc/kernel/exceptions-64e.S           |  13 +--
>   arch/powerpc/kernel/exceptions-64s.S           |   2 +-
>   arch/powerpc/kernel/head_32.S                  |  14 +--
>   arch/powerpc/kernel/head_40x.S                 |   4 +-
>   arch/powerpc/kernel/head_44x.S                 |   8 +-
>   arch/powerpc/kernel/head_64.S                  |   1 +
>   arch/powerpc/kernel/head_8xx.S                 |   2 +-
>   arch/powerpc/kernel/head_booke.h               |  12 +--
>   arch/powerpc/kernel/head_fsl_booke.S           |  16 ++--
>   arch/powerpc/kernel/idle_6xx.S                 |   8 +-
>   arch/powerpc/kernel/idle_book3e.S              |   2 +-
>   arch/powerpc/kernel/idle_e500.S                |   8 +-
>   arch/powerpc/kernel/idle_power4.S              |   2 +-
>   arch/powerpc/kernel/irq.c                      | 114 +++----------------------
>   arch/powerpc/kernel/kgdb.c                     |  28 ------
>   arch/powerpc/kernel/machine_kexec_64.c         |   6 +-
>   arch/powerpc/kernel/misc_32.S                  |  17 ++--
>   arch/powerpc/kernel/process.c                  |  40 ++++-----
>   arch/powerpc/kernel/setup-common.c             |   2 +-
>   arch/powerpc/kernel/setup_32.c                 |  25 +++---
>   arch/powerpc/kernel/setup_64.c                 |  51 +++--------
>   arch/powerpc/kernel/smp.c                      |  16 ++--
>   arch/powerpc/kernel/trace/ftrace_64_mprofile.S |   6 +-
>   arch/powerpc/kvm/book3s_hv_hmi.c               |   1 +
>   arch/powerpc/mm/hash_low_32.S                  |  14 ++-
>   arch/powerpc/net/bpf_jit32.h                   |   5 +-
>   arch/powerpc/sysdev/6xx-suspend.S              |   5 +-
>   arch/powerpc/xmon/xmon.c                       |   2 +-
>   45 files changed, 249 insertions(+), 431 deletions(-)
>   create mode 100644 arch/powerpc/include/asm/task_size_user64.h
> 

  parent reply	other threads:[~2019-01-23 10:04 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-12  9:55 [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK Christophe Leroy
2019-01-12  9:55 ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 01/10] powerpc/irq: use memblock functions returning virtual address Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 02/10] book3s/64: avoid circular header inclusion in mmu-hash.h Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 03/10] powerpc: Only use task_struct 'cpu' field on SMP Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 04/10] powerpc: Prepare for moving thread_info into task_struct Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 05/10] powerpc: Activate CONFIG_THREAD_INFO_IN_TASK Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 06/10] powerpc: regain entire stack space Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 07/10] powerpc: 'current_set' is now a table of task_struct pointers Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 08/10] powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 09/10] powerpc/64: Remove CURRENT_THREAD_INFO Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-12  9:55 ` [PATCH v13 10/10] powerpc: clean stack pointers naming Christophe Leroy
2019-01-12  9:55   ` Christophe Leroy
2019-01-19 10:23 ` [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK Michael Ellerman
2019-01-19 10:23   ` Michael Ellerman
2019-01-19 17:21   ` LEROY Christophe
2019-01-19 17:21     ` LEROY Christophe
2019-01-23 23:10     ` Michael Ellerman
2019-01-23 23:10       ` Michael Ellerman
2019-01-22 19:42   ` Christophe Leroy
2019-01-22 19:42     ` Christophe Leroy
2019-01-24  0:59     ` Michael Ellerman
2019-01-24  0:59       ` Michael Ellerman
2019-01-24 15:08       ` Christophe Leroy
2019-01-24 15:08         ` Christophe Leroy
2019-01-23 10:04 ` Christophe Leroy [this message]
2019-01-23 10:04   ` Christophe Leroy
2019-01-24  1:06   ` Michael Ellerman
2019-01-24  1:06     ` Michael Ellerman
2019-01-24  9:43     ` Christophe Leroy
2019-01-24  9:43       ` Christophe Leroy
2019-01-24 15:01       ` Christophe Leroy
2019-01-24 15:58         ` Christophe Leroy
2019-01-25  7:00           ` Gabriel Paubert
2019-01-25  7:00             ` Gabriel Paubert

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=2e700e1c-5bd9-652e-b535-68a89dd703a1@c-s.fr \
    --to=christophe.leroy@c-s.fr \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=paulus@samba.org \
    --cc=rppt@linux.ibm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.