Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes

From: David Daney <ddaney@caviumnetworks.com>
To: David Daney <ddaney@caviumnetworks.com>,
	Jason Baron <jbaron@akamai.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Sachin Sant <sachinp@linux.vnet.ibm.com>
Cc: linux-mips@linux-mips.org, Chris Metcalf <cmetcalf@mellanox.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Russell King <linux@armlinux.org.uk>,
	Ralf Baechle <ralf@linux-mips.org>,
	LKML <linux-kernel@vger.kernel.org>, Rabin Vincent <rabin@rab.in>,
	Paul Mackerras <paulus@samba.org>,
	Anton Blanchard <anton@samba.org>,
	linuxppc-dev@lists.ozlabs.org, Ingo Molnar <mingo@kernel.org>,
	linux-arm-kernel@lists.infradead.org, Zhigang Lu <zlu@ezchip.com>
Subject: Re: [PATCH] jump_label: align jump_entry table to at least 4-bytes
Date: Tue, 28 Feb 2017 11:22:18 -0800	[thread overview]
Message-ID: <d10e986c-7f6a-3935-88e2-ba39708f79ad@caviumnetworks.com> (raw)
In-Reply-To: <999e2c3f-698c-703c-67a9-26aea3c97dc0@caviumnetworks.com>

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.

>
>>
>> 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@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel