| Message ID | 20190726210854.6408-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/percpu: Cleanup | expand |
Hi Andrew, Thank you for looking into this. On 7/26/19 10:08 PM, Andrew Cooper wrote: > For cleanup purposes, it is necessary for asm/percpu.h to not use > DECLARE_PER_CPU() itself. asm/smp.h is arguably a better place for it to > live anyway. I have noticed that a lot of arch helpers called by common code are defined in different headers. The most offenders are in mm.h and page.h. Looking at x86, the two helpers are defined in current.h. So I think it would make sense to define them at the same places. I don't have a particular preference between smp.h and current.h, just want some consistency :). FWIW, Xen build nicely when the helpers are moved in current.h: diff --git a/xen/include/asm-arm/current.h b/xen/include/asm-arm/current.h index c4af66fbb9..23fa6009d8 100644 --- a/xen/include/asm-arm/current.h +++ b/xen/include/asm-arm/current.h @@ -39,6 +39,13 @@ static inline struct cpu_info *get_cpu_info(void) #define reset_stack_and_jump(fn) switch_stack_and_jump(get_cpu_info(), fn) +DECLARE_PER_CPU(unsigned int, cpu_id); +#define get_processor_id() (this_cpu(cpu_id)) +#define set_processor_id(id) do { \ + WRITE_SYSREG(__per_cpu_offset[id], TPIDR_EL2); \ + this_cpu(cpu_id) = (id); \ +} while(0) + #endif #endif /* __ARM_CURRENT_H__ */ diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h index 9584b830d4..011016347b 100644 --- a/xen/include/asm-arm/percpu.h +++ b/xen/include/asm-arm/percpu.h @@ -27,12 +27,6 @@ void percpu_init_areas(void); #define DECLARE_PER_CPU(type, name) extern __typeof__(type) per_cpu__##name -DECLARE_PER_CPU(unsigned int, cpu_id); -#define get_processor_id() (this_cpu(cpu_id)) -#define set_processor_id(id) do { \ - WRITE_SYSREG(__per_cpu_offset[id], TPIDR_EL2); \ - this_cpu(cpu_id) = (id); \ -} while(0) #endif #endif /* __ARM_PERCPU_H__ */ Cheers,
On 26/07/2019 23:30, Julien Grall wrote: > Hi Andrew, > > Thank you for looking into this. > > On 7/26/19 10:08 PM, Andrew Cooper wrote: >> For cleanup purposes, it is necessary for asm/percpu.h to not use >> DECLARE_PER_CPU() itself. asm/smp.h is arguably a better place for it to >> live anyway. > I have noticed that a lot of arch helpers called by common code > are defined in different headers. The most offenders are in mm.h > and page.h. Those are massive rats nests. At least we're slowly making progress on the outskirts. Lonterm, I'd like to get to a position where the only time you'd ever include an asm/ header is either from the common header itself, or for headers unique to the current architecture. I expect this to simplify things massively. > > Looking at x86, the two helpers are defined in current.h. So I > think it would make sense to define them at the same places. I > don't have a particular preference between smp.h and current.h, > just want some consistency :). > > FWIW, Xen build nicely when the helpers are moved in current.h: I did consider current.h, but couldn't decide and flipped a coin. I'm perfectly happy to put it here if that's what you'd prefer. ~Andrew
Hi Andrew, On 7/26/19 11:37 PM, Andrew Cooper wrote: > On 26/07/2019 23:30, Julien Grall wrote: >> Hi Andrew, >> >> Thank you for looking into this. >> >> On 7/26/19 10:08 PM, Andrew Cooper wrote: >>> For cleanup purposes, it is necessary for asm/percpu.h to not use >>> DECLARE_PER_CPU() itself. asm/smp.h is arguably a better place for it to >>> live anyway. >> I have noticed that a lot of arch helpers called by common code >> are defined in different headers. The most offenders are in mm.h >> and page.h. > > Those are massive rats nests. At least we're slowly making progress on > the outskirts. > > Lonterm, I'd like to get to a position where the only time you'd ever > include an asm/ header is either from the common header itself, or for > headers unique to the current architecture. I expect this to simplify > things massively. > >> >> Looking at x86, the two helpers are defined in current.h. So I >> think it would make sense to define them at the same places. I >> don't have a particular preference between smp.h and current.h, >> just want some consistency :). >> >> FWIW, Xen build nicely when the helpers are moved in current.h: > > I did consider current.h, but couldn't decide and flipped a coin. > > I'm perfectly happy to put it here if that's what you'd prefer. I would prefer consistency with x86 so, current.h :). Feel free to re-use the diff I provided and add my acked-by. Cheers,
diff --git a/xen/include/asm-arm/percpu.h b/xen/include/asm-arm/percpu.h index 264120b192..5ce81a1707 100644 --- a/xen/include/asm-arm/percpu.h +++ b/xen/include/asm-arm/percpu.h @@ -25,12 +25,6 @@ void percpu_init_areas(void); #define DECLARE_PER_CPU(type, name) extern __typeof__(type) per_cpu__##name -DECLARE_PER_CPU(unsigned int, cpu_id); -#define get_processor_id() (this_cpu(cpu_id)) -#define set_processor_id(id) do { \ - WRITE_SYSREG(__per_cpu_offset[id], TPIDR_EL2); \ - this_cpu(cpu_id) = (id); \ -} while(0) #endif #endif /* __ARM_PERCPU_H__ */ diff --git a/xen/include/asm-arm/smp.h b/xen/include/asm-arm/smp.h index fdbcefa241..7d4edfa0a0 100644 --- a/xen/include/asm-arm/smp.h +++ b/xen/include/asm-arm/smp.h @@ -7,11 +7,19 @@ #include <asm/current.h> #endif +DECLARE_PER_CPU(unsigned int, cpu_id); DECLARE_PER_CPU(cpumask_var_t, cpu_sibling_mask); DECLARE_PER_CPU(cpumask_var_t, cpu_core_mask); #define cpu_is_offline(cpu) unlikely(!cpu_online(cpu)) +#define get_processor_id() this_cpu(cpu_id) +#define set_processor_id(id) \ +do { \ + WRITE_SYSREG(__per_cpu_offset[(id)], TPIDR_EL2); \ + this_cpu(cpu_id) = (id); \ +} while ( 0 ) + #define raw_smp_processor_id() (get_processor_id()) /*
For cleanup purposes, it is necessary for asm/percpu.h to not use DECLARE_PER_CPU() itself. asm/smp.h is arguably a better place for it to live anyway. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> --- xen/include/asm-arm/percpu.h | 6 ------ xen/include/asm-arm/smp.h | 8 ++++++++ 2 files changed, 8 insertions(+), 6 deletions(-)