Re: correct section for cpu_tss_rw?
On Tue, 2 Jan 2018, Nick Desaulniers wrote:
> (emailing the folks listed from running `./scripts/get_maintainer.pl
> -f` on arch/x86/kernel/process.c, arch/x86/include/asm/processor.h,
> and include/linux/percpu-defs.h)
>
> Clang emits the following warning:
>
> arch/x86/kernel/process.c:50:11: warning: section does not match
> previous declaration [-Wsection]
> __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
> ^
> ./include/linux/percpu-defs.h:144:2: note: expanded from macro
> 'DEFINE_PER_CPU_SHARED_ALIGNED'
> DEFINE_PER_CPU_SECTION(type, name, PER_CPU_SHARED_ALIGNED_SECTION) \
> ^
> ./include/linux/percpu-defs.h:104:2: note: expanded from macro
> 'DEFINE_PER_CPU_SECTION'
> __PCPU_ATTRS(sec) PER_CPU_DEF_ATTRIBUTES\
> ^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
> __percpu __attribute__((section(PER_CPU_BASE_SECTION sec))) \
> ^
> ./arch/x86/include/asm/processor.h:365:1: note: previous attribute is here
> DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw);
> ^
> ./include/linux/percpu-defs.h:159:2: note: expanded from macro
> 'DECLARE_PER_CPU_PAGE_ALIGNED'
> DECLARE_PER_CPU_SECTION(type, name, "..page_aligned") \
> ^
> ./include/linux/percpu-defs.h:101:9: note: expanded from macro
> 'DECLARE_PER_CPU_SECTION'
> extern __PCPU_ATTRS(sec) __typeof__(type) name
>^
> ./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
> __percpu __attribute__((section(PER_CPU_BASE_SECTION sec))) \
> ^
>
> it seems that from commit c482feefe1a ("x86/entry/64: Make
> cpu_entry_area.tss read-only") that cpu_tss_rw is declared but then
> defined in two different sections. (Though, it looks like this issue
> predates that commit).
>
> It seems that cpu_tss_rw is defined as SHARED_ALIGNED, but then
> declared as PAGE_ALIGNED. Should be an easy fix (that I'm happy to
> author), but what section *should* cpu_tss_rw be in (SHARED_ALIGNED or
> PAGE_ALIGNED)? That affects whether I fix the declaration or
> definition (and thus the .h or the .c file).
>
> >From the comment in arch/x86/kernel/process.c#50:
> 43 /*
> 44 * per-CPU TSS segments. Threads are completely 'soft' on Linux,
> 45 * no more per-task TSS's. The TSS size is kept cacheline-aligned
> 46 * so they are allowed to end up in the .data..cacheline_aligned
> 47 * section. Since TSS's are completely CPU-local, we want them
> 48 * on exact cacheline boundaries, to eliminate cacheline
> ping-pong.
> 49 */
> 50 __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
>
> I suspect that cache-line alignment is stricter than page alignment,
> so the declaration should be fixed, but I was not sure and wanted to
> check?
It must be page aligned.
Thanks,
tglx
correct section for cpu_tss_rw?
(emailing the folks listed from running `./scripts/get_maintainer.pl
-f` on arch/x86/kernel/process.c, arch/x86/include/asm/processor.h,
and include/linux/percpu-defs.h)
Clang emits the following warning:
arch/x86/kernel/process.c:50:11: warning: section does not match
previous declaration [-Wsection]
__visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
^
./include/linux/percpu-defs.h:144:2: note: expanded from macro
'DEFINE_PER_CPU_SHARED_ALIGNED'
DEFINE_PER_CPU_SECTION(type, name, PER_CPU_SHARED_ALIGNED_SECTION) \
^
./include/linux/percpu-defs.h:104:2: note: expanded from macro
'DEFINE_PER_CPU_SECTION'
__PCPU_ATTRS(sec) PER_CPU_DEF_ATTRIBUTES\
^
./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
__percpu __attribute__((section(PER_CPU_BASE_SECTION sec))) \
^
./arch/x86/include/asm/processor.h:365:1: note: previous attribute is here
DECLARE_PER_CPU_PAGE_ALIGNED(struct tss_struct, cpu_tss_rw);
^
./include/linux/percpu-defs.h:159:2: note: expanded from macro
'DECLARE_PER_CPU_PAGE_ALIGNED'
DECLARE_PER_CPU_SECTION(type, name, "..page_aligned") \
^
./include/linux/percpu-defs.h:101:9: note: expanded from macro
'DECLARE_PER_CPU_SECTION'
extern __PCPU_ATTRS(sec) __typeof__(type) name
^
./include/linux/percpu-defs.h:49:26: note: expanded from macro '__PCPU_ATTRS'
__percpu __attribute__((section(PER_CPU_BASE_SECTION sec))) \
^
it seems that from commit c482feefe1a ("x86/entry/64: Make
cpu_entry_area.tss read-only") that cpu_tss_rw is declared but then
defined in two different sections. (Though, it looks like this issue
predates that commit).
In include/linux/percpu-defs.h, there's two pairs of macros:
DECLARE_PER_CPU_SHARED_ALIGNED
DEFINE_PER_CPU_SHARED_ALIGNED
DECLARE_PER_CPU_PAGE_ALIGNED
DEFINE_PER_CPU_PAGE_ALIGNED
It seems that cpu_tss_rw is defined as SHARED_ALIGNED, but then
declared as PAGE_ALIGNED. Should be an easy fix (that I'm happy to
author), but what section *should* cpu_tss_rw be in (SHARED_ALIGNED or
PAGE_ALIGNED)? That affects whether I fix the declaration or
definition (and thus the .h or the .c file).
>From the comment in arch/x86/kernel/process.c#50:
43 /*
44 * per-CPU TSS segments. Threads are completely 'soft' on Linux,
45 * no more per-task TSS's. The TSS size is kept cacheline-aligned
46 * so they are allowed to end up in the .data..cacheline_aligned
47 * section. Since TSS's are completely CPU-local, we want them
48 * on exact cacheline boundaries, to eliminate cacheline
ping-pong.
49 */
50 __visible DEFINE_PER_CPU_SHARED_ALIGNED(struct tss_struct, cpu_tss_rw) = {
I suspect that cache-line alignment is stricter than page alignment,
so the declaration should be fixed, but I was not sure and wanted to
check?
--
Thanks,
~Nick Desaulniers
