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

[Andre Przywara <andre.przywara@linaro.org>]

Hi,

On 01/02/18 14:39, Julien Grall wrote:
> 
> 
> On 01/02/18 14:34, Andre Przywara wrote:
>> Hi,
> 
> Hi,
> 
>> 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@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.
>>>
>>> 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.

Indeed, I missed that part. The date stayed always at "Jun  7  2017" ;-)
So on real surprise that it didn't change.

Cheers,
Andre.

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


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