Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1685551 > unrolled thread

[PATCH] mm: Mark create_huge_pmd() inline to prevent build failure

Back to article view | Back to linux.kernel

Contents

  [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-12 09:00 +0200
    Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Arnd Bergmann <arnd@arndb.de> - 2017-07-12 09:30 +0200
      Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-12 09:40 +0200
        Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Arnd Bergmann <arnd@arndb.de> - 2017-07-12 10:10 +0200
    Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Dan Williams <dan.j.williams@intel.com> - 2017-07-13 02:30 +0200
      Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Dan Williams <dan.j.williams@intel.com> - 2017-07-13 06:10 +0200
      Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Geert Uytterhoeven <geert@linux-m68k.org> - 2017-07-13 09:10 +0200
        Re: [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure Dan Williams <dan.j.williams@intel.com> - 2017-07-13 10:00 +0200

#1685551 — [PATCH] mm: Mark create_huge_pmd() inline to prevent build failure

With gcc 4.1.2:

    mm/memory.o: In function `create_huge_pmd':
    memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'

Converting transparent_hugepage_enabled() from a macro to a static
inline function reduced the ability of the compiler to remove unused
code.

Fix this by marking create_huge_pmd() inline.

Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Interestingly, create_huge_pmd() is emitted in the assembler output, but
never called.
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index cbb57194687e393a..0e517be91a89e162 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3591,7 +3591,7 @@ static int do_numa_page(struct vm_fault *vmf)
 	return 0;
 }
 
-static int create_huge_pmd(struct vm_fault *vmf)
+static inline int create_huge_pmd(struct vm_fault *vmf)
 {
 	if (vma_is_anonymous(vmf->vma))
 		return do_huge_pmd_anonymous_page(vmf);
-- 
2.7.4

[toc] | [next] | [standalone]

#1685565

On Wed, Jul 12, 2017 at 8:57 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> With gcc 4.1.2:
>
>     mm/memory.o: In function `create_huge_pmd':
>     memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'
>
> Converting transparent_hugepage_enabled() from a macro to a static
> inline function reduced the ability of the compiler to remove unused
> code.
>
> Fix this by marking create_huge_pmd() inline.
>
> Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

> ---
> Interestingly, create_huge_pmd() is emitted in the assembler output, but
> never called.

I've never seen this before either. I know that early gcc-4 compilers
would do this
when a function is referenced from an unused function pointer, but not with
a compile-time constant evaluation. I guess that transparent_hugepage_enabled
is just slightly more complex than it gcc-4.1 can handle here.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1685574

Hi Arnd,

On Wed, Jul 12, 2017 at 9:22 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Jul 12, 2017 at 8:57 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> With gcc 4.1.2:
>>
>>     mm/memory.o: In function `create_huge_pmd':
>>     memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'
>>
>> Converting transparent_hugepage_enabled() from a macro to a static
>> inline function reduced the ability of the compiler to remove unused
>> code.
>>
>> Fix this by marking create_huge_pmd() inline.
>>
>> Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks!

>> ---
>> Interestingly, create_huge_pmd() is emitted in the assembler output, but
>> never called.
>
> I've never seen this before either. I know that early gcc-4 compilers
> would do this
> when a function is referenced from an unused function pointer, but not with
> a compile-time constant evaluation. I guess that transparent_hugepage_enabled
> is just slightly more complex than it gcc-4.1 can handle here.

You did mention seeing it with mips-gcc-4.1 in the thread "[RFC] minimum gcc
version for kernel: raise to gcc-4.3 or 4.6?", but didn't provide any further
details. Finally I started seeing it myself for m68k ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1685590

On Wed, Jul 12, 2017 at 9:37 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

> You did mention seeing it with mips-gcc-4.1 in the thread "[RFC] minimum gcc
> version for kernel: raise to gcc-4.3 or 4.6?", but didn't provide any further
> details. Finally I started seeing it myself for m68k ;-)

Ah right, I misremembered that then.

     Arnd

[toc] | [prev] | [next] | [standalone]

#1686151

On Tue, Jul 11, 2017 at 11:57 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> With gcc 4.1.2:
>
>     mm/memory.o: In function `create_huge_pmd':
>     memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'
>
> Converting transparent_hugepage_enabled() from a macro to a static
> inline function reduced the ability of the compiler to remove unused
> code.
>
> Fix this by marking create_huge_pmd() inline.
>
> Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Interestingly, create_huge_pmd() is emitted in the assembler output, but
> never called.
> ---
>  mm/memory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index cbb57194687e393a..0e517be91a89e162 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3591,7 +3591,7 @@ static int do_numa_page(struct vm_fault *vmf)
>         return 0;
>  }
>
> -static int create_huge_pmd(struct vm_fault *vmf)
> +static inline int create_huge_pmd(struct vm_fault *vmf)
>  {

This seems fragile, what if the kernel decides to ignore the inline
hint? If it must be inlined to avoid compile errors then it should be
__always_inline, right?

I also wonder if it's enough to just specify __always_inline to
transparent_hugepage_enabled(), i.e. in case the compiler is making an
uninlined copy of transparent_hugepage_enabled() in mm/memory.c.

[toc] | [prev] | [next] | [standalone]

#1686230

On Wed, Jul 12, 2017 at 5:29 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Jul 11, 2017 at 11:57 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> With gcc 4.1.2:
>>
>>     mm/memory.o: In function `create_huge_pmd':
>>     memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'
>>
>> Converting transparent_hugepage_enabled() from a macro to a static
>> inline function reduced the ability of the compiler to remove unused
>> code.
>>
>> Fix this by marking create_huge_pmd() inline.
>>
>> Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> Interestingly, create_huge_pmd() is emitted in the assembler output, but
>> never called.
>> ---
>>  mm/memory.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index cbb57194687e393a..0e517be91a89e162 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -3591,7 +3591,7 @@ static int do_numa_page(struct vm_fault *vmf)
>>         return 0;
>>  }
>>
>> -static int create_huge_pmd(struct vm_fault *vmf)
>> +static inline int create_huge_pmd(struct vm_fault *vmf)
>>  {
>
> This seems fragile, what if the kernel decides

...of course I meant *compiler* instead of 'kernel' here

>  to ignore the inline
> hint? If it must be inlined to avoid compile errors then it should be
> __always_inline, right?
>
> I also wonder if it's enough to just specify __always_inline to
> transparent_hugepage_enabled(), i.e. in case the compiler is making an
> uninlined copy of transparent_hugepage_enabled() in mm/memory.c.
>

[toc] | [prev] | [next] | [standalone]

#1686305

Hi Dan,

On Thu, Jul 13, 2017 at 2:29 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Jul 11, 2017 at 11:57 PM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> With gcc 4.1.2:
>>
>>     mm/memory.o: In function `create_huge_pmd':
>>     memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'
>>
>> Converting transparent_hugepage_enabled() from a macro to a static
>> inline function reduced the ability of the compiler to remove unused
>> code.
>>
>> Fix this by marking create_huge_pmd() inline.
>>
>> Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> Interestingly, create_huge_pmd() is emitted in the assembler output, but
>> never called.
>> ---
>>  mm/memory.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index cbb57194687e393a..0e517be91a89e162 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -3591,7 +3591,7 @@ static int do_numa_page(struct vm_fault *vmf)
>>         return 0;
>>  }
>>
>> -static int create_huge_pmd(struct vm_fault *vmf)
>> +static inline int create_huge_pmd(struct vm_fault *vmf)
>>  {
>
> This seems fragile, what if the kernel decides to ignore the inline
> hint? If it must be inlined to avoid compile errors then it should be
> __always_inline, right?

With gcc-4, "inline" is already #define'd to
#define inline inline           __attribute__((always_inline,unused)) notrace

> I also wonder if it's enough to just specify __always_inline to
> transparent_hugepage_enabled(), i.e. in case the compiler is making an
> uninlined copy of transparent_hugepage_enabled() in mm/memory.c.

Hence the answer is no.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1686343

On Thu, Jul 13, 2017 at 12:04 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Dan,
>
> On Thu, Jul 13, 2017 at 2:29 AM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Tue, Jul 11, 2017 at 11:57 PM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> With gcc 4.1.2:
>>>
>>>     mm/memory.o: In function `create_huge_pmd':
>>>     memory.c:(.text+0x93e): undefined reference to `do_huge_pmd_anonymous_page'
>>>
>>> Converting transparent_hugepage_enabled() from a macro to a static
>>> inline function reduced the ability of the compiler to remove unused
>>> code.
>>>
>>> Fix this by marking create_huge_pmd() inline.
>>>
>>> Fixes: 16981d763501c0e0 ("mm: improve readability of transparent_hugepage_enabled()")
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>> Interestingly, create_huge_pmd() is emitted in the assembler output, but
>>> never called.
>>> ---
>>>  mm/memory.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index cbb57194687e393a..0e517be91a89e162 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -3591,7 +3591,7 @@ static int do_numa_page(struct vm_fault *vmf)
>>>         return 0;
>>>  }
>>>
>>> -static int create_huge_pmd(struct vm_fault *vmf)
>>> +static inline int create_huge_pmd(struct vm_fault *vmf)
>>>  {
>>
>> This seems fragile, what if the kernel decides to ignore the inline
>> hint? If it must be inlined to avoid compile errors then it should be
>> __always_inline, right?
>
> With gcc-4, "inline" is already #define'd to
> #define inline inline           __attribute__((always_inline,unused)) notrace

Ah, ok.

Acked-by: Dan Williams <dan.j.williams@intel.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web