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

From: Andre Przywara <andre.przywara@linaro.org>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: xen-devel@lists.xenproject.org,
	Julien Grall <julien.grall@linaro.org>,
	Stefano Stabellini <sstabellini@kernel.org>
Subject: Re: [PATCH v3 8/8] ARM: make nr_irqs a constant
Date: Thu, 1 Feb 2018 14:39:46 +0000	[thread overview]
Message-ID: <6684fb54-8fa8-7641-f67b-1f9470521f9c@linaro.org> (raw)
In-Reply-To: <20180201135718.cgbmsxiwa4og73wk@MacBook-Pro-de-Roger.local>

Hi,

On 01/02/18 13:57, Roger Pau Monné wrote:
> On Thu, Feb 01, 2018 at 01:43:09PM +0000, Andre Przywara wrote:
>> Hi,
>>
>> 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@linaro.org>
>>>> Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
>>>> ---
>>>>  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.
>> But I will include the change anyway.
> 
> Hm, that's kind of weird. nr_irqs seems to be used in ARM code. How
> did you assert that the symbol is not there?

Well, I was looking at xen/xen.axf, which is obviously an artefact from
some experiments last summer, as it didn't get rebuild ;-)
So ignore me, the differences are there in xen-syms. And __read_mostly
overrides const, so it *is* helpful to remove it.

Thanks!
Andre.

> 
> This is what I do on x86:
> 
> # nm xen/xen-syms | grep ' nr_irqs$'
> ffff82d0804324f0 D nr_irqs
> 
> Which matches what I would expect from the x86 build.
> 
> Roger.
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel