2010-08-08 19:58:05

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH] MIPS: remove RELOC_HIDE on __pa_symbol

remove unneccessary use of RELOC_HIDE(). It does simple addition of ptr and
offset and in this case (offset 0) does practically nothing. It does NOT do
anything with linker relocation.

Signed-off-by: Namhyung Kim <[email protected]>
---
arch/mips/include/asm/page.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index a16beaf..f7e2684 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -150,7 +150,7 @@ typedef struct { unsigned long pgprot; } pgprot_t;
((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
#endif
#define __va(x) ((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
-#define __pa_symbol(x) __pa(RELOC_HIDE((unsigned long)(x), 0))
+#define __pa_symbol(x) __pa(x)

#define pfn_to_kaddr(pfn) __va((pfn) << PAGE_SHIFT)

--
1.7.0.4


2010-08-09 05:15:13

by David Daney

[permalink] [raw]
Subject: Re: [PATCH] MIPS: remove RELOC_HIDE on __pa_symbol

On 08/08/2010 12:57 PM, Namhyung Kim wrote:
> remove unneccessary use of RELOC_HIDE(). It does simple addition of ptr and
> offset and in this case (offset 0) does practically nothing. It does NOT do
> anything with linker relocation.
>

Maybe you could explain in more detail the problems you are having with
the current definition of __pa_symbol(). I would be hesitant to change
this bit of black magic unless there is a concrete problem you are
trying to solve.

David Daney


> Signed-off-by: Namhyung Kim<[email protected]>
> ---
> arch/mips/include/asm/page.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
> index a16beaf..f7e2684 100644
> --- a/arch/mips/include/asm/page.h
> +++ b/arch/mips/include/asm/page.h
> @@ -150,7 +150,7 @@ typedef struct { unsigned long pgprot; } pgprot_t;
> ((unsigned long)(x) - PAGE_OFFSET + PHYS_OFFSET)
> #endif
> #define __va(x) ((void *)((unsigned long)(x) + PAGE_OFFSET - PHYS_OFFSET))
> -#define __pa_symbol(x) __pa(RELOC_HIDE((unsigned long)(x), 0))
> +#define __pa_symbol(x) __pa(x)
>
> #define pfn_to_kaddr(pfn) __va((pfn)<< PAGE_SHIFT)
>

2010-08-09 12:21:58

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH] MIPS: remove RELOC_HIDE on __pa_symbol

On Sun, Aug 08, 2010 at 10:15:04PM -0700, David Daney wrote:

> On 08/08/2010 12:57 PM, Namhyung Kim wrote:
> >remove unneccessary use of RELOC_HIDE(). It does simple addition of ptr and
> >offset and in this case (offset 0) does practically nothing. It does NOT do
> >anything with linker relocation.
> >
>
> Maybe you could explain in more detail the problems you are having
> with the current definition of __pa_symbol(). I would be hesitant
> to change this bit of black magic unless there is a concrete problem
> you are trying to solve.

RELOC_HIDE was originally added by 6007b903dfe5f1d13e0c711ac2894bdd4a61b1ad
(lmo) rsp. 8431fd094d625b94d364fe393076ccef88e6ce18 (kernel.org). A
discussion can be found in lkml posting
[email protected]> which is
archived at

http://lists.linuxcoding.com/kernel/2006-q3/msg17360.html

I felt this was dubious by the time it was added and probably should go?

Ralf

2010-08-10 03:07:26

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] MIPS: remove RELOC_HIDE on __pa_symbol

2010-08-09 (월), 13:21 +0100, Ralf Baechle:
> On Sun, Aug 08, 2010 at 10:15:04PM -0700, David Daney wrote:
> > Maybe you could explain in more detail the problems you are having
> > with the current definition of __pa_symbol(). I would be hesitant
> > to change this bit of black magic unless there is a concrete problem
> > you are trying to solve.
>
> RELOC_HIDE was originally added by 6007b903dfe5f1d13e0c711ac2894bdd4a61b1ad
> (lmo) rsp. 8431fd094d625b94d364fe393076ccef88e6ce18 (kernel.org). A
> discussion can be found in lkml posting
> [email protected]> which is
> archived at
>
> http://lists.linuxcoding.com/kernel/2006-q3/msg17360.html
>
> I felt this was dubious by the time it was added and probably should go?
>
> Ralf

Hi,

I've sent basically same patch to x86 folks [1] and they said there is a
possiblility of miscompilation on gcc 3. I am not sure the same goes
here on mips but it might be safer to keep it. Sorry for the noise ;-(

[1] http://lkml.org/lkml/2010/8/8/138


--
Regards,
Namhyung Kim

2010-08-17 15:00:44

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH] MIPS: remove RELOC_HIDE on __pa_symbol

On Tue, Aug 10, 2010 at 12:07:08PM +0900, Namhyung Kim wrote:

> I've sent basically same patch to x86 folks [1] and they said there is a
> possiblility of miscompilation on gcc 3. I am not sure the same goes
> here on mips but it might be safer to keep it. Sorry for the noise ;-(
>
> [1] http://lkml.org/lkml/2010/8/8/138

So in a distant future when GCC 3.x will finally be retired we will be
able to apply this patch, sigh. I'll drop your patch for the time being
and add a comment to the code.

Thanks!

Ralf

2010-08-17 15:05:49

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] MIPS: remove RELOC_HIDE on __pa_symbol

2010-08-17 (화), 16:00 +0100, Ralf Baechle:
> On Tue, Aug 10, 2010 at 12:07:08PM +0900, Namhyung Kim wrote:
>
> > I've sent basically same patch to x86 folks [1] and they said there is a
> > possiblility of miscompilation on gcc 3. I am not sure the same goes
> > here on mips but it might be safer to keep it. Sorry for the noise ;-(
> >
> > [1] http://lkml.org/lkml/2010/8/8/138
>
> So in a distant future when GCC 3.x will finally be retired we will be
> able to apply this patch, sigh. I'll drop your patch for the time being
> and add a comment to the code.
>
> Thanks!
>
> Ralf

FYI, the exact version introduced -f[no-]strict-overlow was gcc 4.2.

--
Regards,
Namhyung Kim