mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jason Baron <jbaron@akamai.com>
To: David Daney <ddaney@caviumnetworks.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 14:34:52 -0500	[thread overview]
Message-ID: <542488db-5c59-afa5-6d1d-a437c87bc613@akamai.com> (raw)
In-Reply-To: <d10e986c-7f6a-3935-88e2-ba39708f79ad@caviumnetworks.com>



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

  reply	other threads:[~2017-02-28 19:35 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27 18:49 Jason Baron
2017-02-27 18:57 ` David Daney
2017-02-27 19:18   ` Jason Baron
2017-02-27 19:59     ` David Daney
2017-02-27 21:06       ` Steven Rostedt
2017-02-27 21:41         ` David Daney
2017-02-27 22:09           ` Steven Rostedt
2017-02-27 22:21             ` David Daney
2017-02-27 22:36               ` Steven Rostedt
2017-02-27 22:45                 ` David Daney
2017-02-27 22:50                   ` Jason Baron
2017-02-27 23:34                     ` David Daney
2017-02-28  4:55                     ` Sachin Sant
2017-02-28 16:21                       ` Steven Rostedt
2017-02-28 18:16                         ` David Daney
2017-02-28 18:39                           ` Jason Baron
2017-02-28 19:05                             ` David Daney
2017-02-28 19:22                               ` David Daney
2017-02-28 19:34                                 ` Jason Baron [this message]
2017-02-28 20:15                                   ` David Daney
2017-02-28 22:41                                     ` Jason Baron
2017-03-01  6:34                                       ` Michael Ellerman
2017-03-01 16:40                                         ` David Daney
2017-03-01 20:02                                           ` Jason Baron
2017-03-01 21:12                                             ` David Daney
2017-03-01  7:12                                       ` Sachin Sant
2017-02-27 22:58                   ` Steven Rostedt
2017-03-06  2:16               ` [PATCH] MIPS: jump_lable: Give __jump_table elements an entsize kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=542488db-5c59-afa5-6d1d-a437c87bc613@akamai.com \
    --to=jbaron@akamai.com \
    --cc=anton@samba.org \
    --cc=cmetcalf@mellanox.com \
    --cc=ddaney@caviumnetworks.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=linux@armlinux.org.uk \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=rabin@rab.in \
    --cc=ralf@linux-mips.org \
    --cc=rostedt@goodmis.org \
    --cc=sachinp@linux.vnet.ibm.com \
    --cc=zlu@ezchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®