Messages in this thread Patch in this message | | | Subject | Re: [RFC PATCH v1 1/9] timer: fix circular header dependency | From | Christophe Leroy <> | Date | Tue, 25 Sep 2018 17:48:41 +0000 |
| |
On 09/25/2018 12:15 PM, Christophe LEROY wrote: > > > Le 25/09/2018 à 14:11, Mark Rutland a écrit : >> On Tue, Sep 25, 2018 at 07:34:15AM +0200, Christophe LEROY wrote: >>> >>> >>> Le 24/09/2018 à 17:52, Christophe Leroy a écrit : >>>> When switching powerpc to CONFIG_THREAD_INFO_IN_TASK, include/sched.h >>>> has to be included in asm/smp.h for the following change, in order >>>> to avoid uncomplete definition of task_struct: >>>> >>>> -#define raw_smp_processor_id() (current_thread_info()->cpu) >>>> +#define raw_smp_processor_id() (current->cpu) >>>> >>>> But this generates the following compilation error, due to circular >>>> header dependency. >>>> >>>> CC kernel/time/alarmtimer.o >>>> In file included from ./arch/powerpc/include/asm/smp.h:31, >>>> from ./include/linux/smp.h:64, >>>> from ./include/linux/percpu.h:7, >>>> from ./include/linux/hrtimer.h:22, >>>> from kernel/time/alarmtimer.c:19: >>>> ./include/linux/sched.h:558:19: error: field 'dl_timer' has >>>> incomplete type >>>> struct hrtimer dl_timer; >>>> ^~~~~~~~ >>>> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has >>>> incomplete type >>>> struct hrtimer inactive_timer; >>>> ^~~~~~~~~~~~~~ >>>> make[1]: *** [kernel/time/alarmtimer.o] Error 1 >>>> make: *** [kernel/time/alarmtimer.o] Error 2 >>>> >>>> CC fs/timerfd.o >>>> In file included from ./arch/powerpc/include/asm/smp.h:31, >>>> from ./include/linux/smp.h:64, >>>> from ./include/linux/percpu.h:7, >>>> from ./include/linux/hrtimer.h:22, >>>> from ./include/linux/alarmtimer.h:6, >>>> from fs/timerfd.c:12: >>>> ./include/linux/sched.h:558:19: error: field 'dl_timer' has >>>> incomplete type >>>> struct hrtimer dl_timer; >>>> ^~~~~~~~ >>>> ./include/linux/sched.h:567:17: error: field 'inactive_timer' has >>>> incomplete type >>>> struct hrtimer inactive_timer; >>>> ^~~~~~~~~~~~~~ >>>> make[1]: *** [fs/timerfd.o] Error 1 >>>> make: *** [fs/timerfd.o] Error 2 >>>> >>>> This patch fixes it by including linux/hrtimer.h after linux/sched.h >>>> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >>>> --- >>>> Should it be fixed in powerpc instead ? In that case, how ? Any >>>> idea ? >>> >>> >>> Looks like there are several other places where the problem occurs, >>> so it >>> has to be fixed in the powerpc headers instead. >>> >>> Seems like RISC arch faced the same issue, and fixed it the following >>> way: >>> >>> /* >>> * This is particularly ugly: it appears we can't actually get the >>> definition >>> * of task_struct here, but we need access to the CPU this task is >>> running >>> on. >>> * Instead of using C we're using asm-offsets.h to get the current >>> processor >>> * ID. >>> */ >>> #define raw_smp_processor_id() (*((int*)((char*)get_current() + >>> TASK_TI_CPU))) >>> >>> >>> Unless someone has a better idea, I'll fixed it that way. >> >> To solve this on arm64 we made the cpu number a percpu variable; see >> commit: >> >> 57c82954e77fa12c ("arm64: make cpu number a percpu variable") > > Thanks, I'll have a look > >> IIUC, you could do that by placing it in paca_struct. > > By the way, it is already in PACA struct for PPC64. > The issue is with PPC32, up to now it was in current_thread_info: > > #ifdef CONFIG_PPC64 > #define raw_smp_processor_id() (local_paca->paca_index) > #define hard_smp_processor_id() (get_paca()->hw_cpu_id) > #else > /* 32-bit */ > extern int smp_hw_index[]; > > #define raw_smp_processor_id() (current_thread_info()->cpu) > #define hard_smp_processor_id() (smp_hw_index[smp_processor_id()]) > > [..] > > #endif >
It seems not that easy. Adding the following leads to the below failure. I'm having a hard time identifying and fixing the issue. Looks again like a circular thing, because linux/percpu-defs.h is where raw_cpu_ptr() is defined.
--- a/arch/powerpc/include/asm/smp.h +++ b/arch/powerpc/include/asm/smp.h @@ -83,7 +83,14 @@ int is_cpu_dead(unsigned int cpu); /* 32-bit */ extern int smp_hw_index[];
-#define raw_smp_processor_id() (current_thread_info()->cpu) +DECLARE_PER_CPU_READ_MOSTLY(unsigned int, cpu_number); + +/* + * We don't use this_cpu_read(cpu_number) as that has implicit writes to + * preempt_count, and associated (compiler) barriers, that we'd like to avoid + * the expense of. If we're preemptible, the value can be stale at use anyway. + */ +#define raw_smp_processor_id() (*this_cpu_ptr(&cpu_number)) #define hard_smp_processor_id() (smp_hw_index[smp_processor_id()])
static inline int get_hard_smp_processor_id(int cpu)
CC arch/powerpc/kernel/asm-offsets.s In file included from ././include/linux/compiler_types.h:64:0, from <command-line>:0: ./include/linux/percpu-rwsem.h: In function 'percpu_down_read_preempt_disable': ./include/linux/percpu-defs.h:254:27: error: implicit declaration of function 'raw_cpu_ptr' [-Werror=implicit-function-declaration] #define this_cpu_ptr(ptr) raw_cpu_ptr(ptr) ^ ./include/linux/compiler-gcc.h:58:26: note: in definition of macro 'RELOC_HIDE' (typeof(ptr)) (__ptr + (off)); \ ^ ./include/asm-generic/percpu.h:44:31: note: in expansion of macro 'SHIFT_PERCPU_PTR' #define arch_raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset) ^ ./include/asm-generic/percpu.h:31:25: note: in expansion of macro 'per_cpu_offset' #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id()) ^ ./arch/powerpc/include/asm/smp.h:93:35: note: in expansion of macro 'this_cpu_ptr' #define raw_smp_processor_id() (*this_cpu_ptr(&cpu_number)) ^ ./include/asm-generic/percpu.h:31:40: note: in expansion of macro 'raw_smp_processor_id' #define __my_cpu_offset per_cpu_offset(raw_smp_processor_id()) ^ ./include/asm-generic/percpu.h:44:53: note: in expansion of macro '__my_cpu_offset' #define arch_raw_cpu_ptr(ptr) SHIFT_PERCPU_PTR(ptr, __my_cpu_offset) ^ ./include/linux/percpu-defs.h:244:2: note: in expansion of macro 'arch_raw_cpu_ptr' arch_raw_cpu_ptr(ptr); \ ^ ./include/asm-generic/percpu.h:76:3: note: in expansion of macro 'raw_cpu_ptr' *raw_cpu_ptr(&(pcp)) op val; \ ^ ./include/asm-generic/percpu.h:225:34: note: in expansion of macro 'raw_cpu_generic_to_op' #define raw_cpu_add_1(pcp, val) raw_cpu_generic_to_op(pcp, val, +=) ^ ./include/linux/percpu-defs.h:379:11: note: in expansion of macro 'raw_cpu_add_1' case 1: stem##1(variable, __VA_ARGS__);break; \ ^ ./include/linux/percpu-defs.h:424:32: note: in expansion of macro '__pcpu_size_call' #define raw_cpu_add(pcp, val) __pcpu_size_call(raw_cpu_add_, pcp, val) ^ ./include/linux/percpu-defs.h:460:2: note: in expansion of macro 'raw_cpu_add' raw_cpu_add(pcp, val); \ ^ ./include/linux/percpu-defs.h:499:30: note: in expansion of macro '__this_cpu_add' #define __this_cpu_inc(pcp) __this_cpu_add(pcp, 1) ^ ./include/linux/percpu-rwsem.h:47:2: note: in expansion of macro '__this_cpu_inc' __this_cpu_inc(*sem->read_count); ^ ./arch/powerpc/include/asm/smp.h:93:34: error: invalid type argument of unary '*' (have 'int') #define raw_smp_processor_id() (*this_cpu_ptr(&cpu_number)) ^
[...]
| |