[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 8/8] ARM: make nr_irqs a constant

On 01/02/18 14:34, Andre Przywara wrote:


On 01/02/18 13:47, Julien Grall wrote:
Hi Andre,

On 01/02/18 13:43, Andre Przywara wrote:
On 30/01/18 14:36, Roger Pau Monné wrote:
On Wed, Jan 24, 2018 at 06:10:58PM +0000, Andre Przywara wrote:
On ARM the maximum number of IRQs is a constant, but we share it being
a variable to match x86. Since we are not supposed to alter it, let's
mark it as "const" to avoid accidental change.

Suggested-by: Julien Grall <julien.grall@xxxxxxxxxx>
Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>
   xen/arch/arm/irq.c        | 2 +-
   xen/include/asm-arm/irq.h | 2 +-
   2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 62103a20e3..d229cb6871 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -27,7 +27,7 @@
   #include <asm/gic.h>
   #include <asm/vgic.h>
   -unsigned int __read_mostly nr_irqs = NR_IRQS;
+const unsigned int __read_mostly nr_irqs = NR_IRQS;

Shouldn't you remove the __read_mostly attribute, so the symbol it's
placed at the .rodata section by the compiler?

Yes, makes sense, thanks for pointing this out!
const ... __read_mostly sounds somewhat redundant.

It looks like the compiler does the right thing anyway, as I can't find
nr_irqs in the ELF in any case. Both with and without __read_mostly it
results into the very same binary, actually even without the const.

Really? I was expecting const data to be in the section.rodata. Are you
suggesting this is landing in the section .data instead?

Well, for the local case (arm/irq.c) the compiler just does the right
thing and eliminates the variable completely :

00000000000005dc <request_irq>:
  5dc:   eb1f005f        cmp     x2, xzr
  5e0:   52807fe5        mov     w5, #0x3ff                // #1023
  5e4:   7a451002        ccmp    w0, w5, #0x2, ne
  5e8:   540003e8        b.hi    664 <request_irq+0x88>    // EINVAL

For common/domain.o it is as you guessed: in .data.read_mostly, with or
without this (original) patch. So __read_mostly trumps const.
Removing __read_mostly indeed puts it in .rodata.

Don't know why the resulting xen/xen.axf don't show any difference, but
the respective object files do.
xen-syms is an ELF binary.
xen is not an ELF.
xen.axf is not built anymore since Xen 4.9.

That might explain why you are not able to find it.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.