lkml.org 
[lkml]   [2021]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2 5/7] sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y
    From
    Date


    Le 30/09/2021 à 15:12, Ard Biesheuvel a écrit :
    > On Thu, 30 Sept 2021 at 15:09, Christophe Leroy
    > <christophe.leroy@csgroup.eu> wrote:
    >>
    >>
    >>
    >> Le 30/09/2021 à 14:58, Ard Biesheuvel a écrit :
    >>> THREAD_INFO_IN_TASK moved the CPU field out of thread_info, but this
    >>> causes some issues on architectures that define raw_smp_processor_id()
    >>> in terms of this field, due to the fact that #include'ing linux/sched.h
    >>> to get at struct task_struct is problematic in terms of circular
    >>> dependencies.
    >>>
    >>> Given that thread_info and task_struct are the same data structure
    >>> anyway when THREAD_INFO_IN_TASK=y, let's move it back so that having
    >>> access to the type definition of struct thread_info is sufficient to
    >>> reference the CPU number of the current task.
    >>>
    >>> Note that this requires THREAD_INFO_IN_TASK's definition of the
    >>> task_thread_info() helper to be updated, as task_cpu() takes a
    >>> pointer-to-const, whereas task_thread_info() (which is used to generate
    >>> lvalues as well), needs a non-const pointer. So make it a macro instead.
    >>>
    >>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
    >>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
    >>> Acked-by: Mark Rutland <mark.rutland@arm.com>
    >>> Acked-by: Michael Ellerman <mpe@ellerman.id.au>
    >>> ---
    >>> arch/arm64/kernel/asm-offsets.c | 1 -
    >>> arch/arm64/kernel/head.S | 2 +-
    >>> arch/powerpc/kernel/asm-offsets.c | 2 +-
    >>> arch/powerpc/kernel/smp.c | 2 +-
    >>> include/linux/sched.h | 13 +------------
    >>> kernel/sched/sched.h | 4 ----
    >>> 6 files changed, 4 insertions(+), 20 deletions(-)
    >>>
    >>> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
    >>> index cee9f3e9f906..0bfc048221af 100644
    >>> --- a/arch/arm64/kernel/asm-offsets.c
    >>> +++ b/arch/arm64/kernel/asm-offsets.c
    >>> @@ -27,7 +27,6 @@
    >>> int main(void)
    >>> {
    >>> DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
    >>> - DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
    >>> BLANK();
    >>> DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
    >>> DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
    >>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
    >>> index 17962452e31d..6a98f1a38c29 100644
    >>> --- a/arch/arm64/kernel/head.S
    >>> +++ b/arch/arm64/kernel/head.S
    >>> @@ -412,7 +412,7 @@ SYM_FUNC_END(__create_page_tables)
    >>> scs_load \tsk
    >>>
    >>> adr_l \tmp1, __per_cpu_offset
    >>> - ldr w\tmp2, [\tsk, #TSK_CPU]
    >>> + ldr w\tmp2, [\tsk, #TSK_TI_CPU]
    >>
    >> Why do you need to change the name ?
    >>
    >> For powerpc64, you leave TASK_CPU.
    >>
    >
    > Because arm64 has a clear idiom here, where TSK_TI_ is used for
    > thread_info fields accessed via a task_struct pointer. Also, it only
    > occurs once in the code.
    >
    > Power does not seem to have this idiom, and TASK_CPU is used in many
    > more places, so I don't think it makes sense to change its name.

    In the old days it was called TI_CPU, was changed by commit f7354ccac844
    ("powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU") after
    commit ed1cd6deb013 ("powerpc: Activate CONFIG_THREAD_INFO_IN_TASK")

    I don't have a strong opinion about it but we have:

    $ git grep thread_info arch/powerpc/kernel/asm-offsets.c
    arch/powerpc/kernel/asm-offsets.c:#include <asm/thread_info.h>
    arch/powerpc/kernel/asm-offsets.c: OFFSET(TI_livepatch_sp,
    thread_info, livepatch_sp);
    arch/powerpc/kernel/asm-offsets.c: OFFSET(TI_LOCAL_FLAGS,
    thread_info, local_flags);
    arch/powerpc/kernel/asm-offsets.c:
    offsetof(struct task_struct, thread_info));


    >
    >
    >>> ldr \tmp1, [\tmp1, \tmp2, lsl #3]
    >>> set_this_cpu_offset \tmp1
    >>> .endm
    >>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
    >>> index e563d3222d69..e37e4546034e 100644
    >>> --- a/arch/powerpc/kernel/asm-offsets.c
    >>> +++ b/arch/powerpc/kernel/asm-offsets.c
    >>> @@ -93,7 +93,7 @@ int main(void)
    >>> #endif /* CONFIG_PPC64 */
    >>> OFFSET(TASK_STACK, task_struct, stack);
    >>> #ifdef CONFIG_SMP
    >>> - OFFSET(TASK_CPU, task_struct, cpu);
    >>> + OFFSET(TASK_CPU, task_struct, thread_info.cpu);
    >>> #endif
    >>>
    >>> #ifdef CONFIG_LIVEPATCH
    >>
    >> ...

    \
     
     \ /
      Last update: 2021-09-30 15:23    [W:3.563 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site