[1/4] arm/percpu: Move {get, set}_processor_id() into smp.h
diff mbox series

Message ID 20190726210854.6408-2-andrew.cooper3@citrix.com
State New
Headers show
Series
  • xen/percpu: Cleanup
Related show

Commit Message

Andrew Cooper July 26, 2019, 9:08 p.m. UTC
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(-)

Comments

Julien Grall July 26, 2019, 10:30 p.m. UTC | #1
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,
Andrew Cooper July 26, 2019, 10:37 p.m. UTC | #2
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
Julien Grall July 27, 2019, 12:41 p.m. UTC | #3
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,

Patch
diff mbox series

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())
 
 /*