Message-ID: <542488db-5c59-afa5-6d1d-a437c87bc613@akamai.com>
Date:   Tue, 28 Feb 2017 14:34:52 -0500
From:   Jason Baron <jbaron@...mai.com>
To:     David Daney <ddaney@...iumnetworks.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sachin Sant <sachinp@...ux.vnet.ibm.com>
Cc:     linux-mips@...ux-mips.org, Chris Metcalf <cmetcalf@...lanox.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Russell King <linux@...linux.org.uk>,
        Ralf Baechle <ralf@...ux-mips.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Rabin Vincent <rabin@....in>,
        Paul Mackerras <paulus@...ba.org>,
        Anton Blanchard <anton@...ba.org>,
        linuxppc-dev@...ts.ozlabs.org, Ingo Molnar <mingo@...nel.org>,
        linux-arm-kernel@...ts.infradead.org, Zhigang Lu <zlu@...hip.com>
Subject: Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes



On 02/28/2017 02:22 PM, David Daney wrote:
> On 02/28/2017 11:05 AM, David Daney wrote:
>> On 02/28/2017 10:39 AM, Jason Baron wrote:
>>>
> [...]
>>>> I suspect that the alignment of the __jump_table section in the .ko
>>>> files is not correct, and you are seeing some sort of problem due to
>>>> that.
>>>>
>>>>
>>>
>>> Hi,
>>>
>>> Yes, if you look at the trace that Sachin sent the module being loaded
>>> that does the WARN_ON() is nfsd.ko.
>>>
>>> That module from Sachin's trace has:
>>>
>>>   [31] __jump_table      PROGBITS        0000000000000000 03fd77 0000c0
>>> 18 WAM  0   0  1
>>
>> The problem is then the section alignment (last column) for power.
>>
>> On mips with no patches applied, we get:
>>
>>   [17] __jump_table      PROGBITS        0000000000000000 00d2c0 000048
>> 00  WA  0   0  8
>>
>> Look, proper alignment!
>>
>> The question I have is why do the power ".llong" and ".long" assembler
>> directives not force section alignment?  Is there an alternative that
>> could be used that would result in the proper alignment?  Would ".word"
>> work?
>>
>> If not, then I would say patch only power with your balign thing. 8-byte
>> alignment for 64-bit kernel, 4-byte alignment for 32-bit kernel
>>
> 
> I think the proper fix is either:
> 
> A) Modify scripts/module-common.lds to force __jump_table alignment for
> all architectures.
> 
> B) Add arch/powerpc/kernel/module.lds to force __jump_table alignment
> for powerpc only.
> 
> David.
> 
>

Ok, I can try adding it to the linger script.

FWIW, here is my before and after with the .balign thing for the nfsd.ko
module on powperc (using a cross-compiler):

before:

  [31] __jump_table      PROGBITS        0000000000000000 03ee3e 0000f0
00  WA  0   0  1

after:

 [31] __jump_table      PROGBITS        0000000000000000 03ee40 0000f0
00  WA  0   0  4

Thanks,

-Jason


> 
>>
>>>
>>> So its not the size but rather the start offset '03fd77', that is the
>>> problem here. That is what the WARN_ON triggers on, that the start of
>>> the table is not 4-byte aligned.
>>>
>>> Using a ppc cross-compiler and the ENTSIZE patch that line does not
>>> change, however if I use the initial patch posted in this thread, the
>>> start does align to 4-bytes and thus the warning goes away, as Sachin
>>> verified. In fact, without the patch I found several modules that don't
>>> start at the proper alignment, however with the patch that started this
>>> thread they were all properly aligned.
>>>
>>> In terms of the '.balign' causing holes, we originally added the
>>> '_ASM_ALIGN' to x86 for precisely this reason. See commit:
>>> ef64789 jump label: Add _ASM_ALIGN for x86 and x86_64 and discussion.
>>>
>>> In addition, we have a lot of runtime with the .balign in the tree and
>>> I'm not aware of any holes in the table. I think the code would blow up
>>> pretty badly if there were.
>>>
>>> A number of arches were already using the '.balign', and the patch I
>>> proposed simply added it to remaining ones, now that we added a
>>> WARN_ON() to catch this condition.
>>>
>>> Thanks,
>>>
>>> -Jason
>>>
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Powered by blists - more mailing lists