2006-11-24 17:00:16

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH] efi_limit_regions triggers link failure when CONFIG_EFI is not defined

The following patch is needed to get 2.6.19-rc6-mm1 to compile with
CONFIG_EFI disabled. This is the 'shortest' fix. However, it does
appear that there is some overlap with EFI implmentation partly
being in e820.c and partly in efi.c. It might make sense to move
everything efi related over to efi.c.

-apw

=== 8< ===
efi_limit_regions triggers link failure when CONFIG_EFI is not defined

The changes in the patch x86_64-mm-i386-efi-memmap extracted
the guts of limit_regions out into a new efi_limit_regions().
This exposes this code to the compiler uncondionally, previously
it was under an if (efi_enabled) which allowed it to be optimised
away without comment. This leads to link errors looking for an
undefined memmap. Make the routine body conditional on CONFIG_EFI.

Signed-off-by: Andy Whitcroft <[email protected]>
---
diff --git a/arch/i386/kernel/e820.c b/arch/i386/kernel/e820.c
index 6f3fda4..393b87a 100644
--- a/arch/i386/kernel/e820.c
+++ b/arch/i386/kernel/e820.c
@@ -743,6 +743,7 @@ void __init print_memory_map(char *who)

static __init void efi_limit_regions(unsigned long long size)
{
+#ifdef CONFIG_EFI
unsigned long long current_addr = 0;
efi_memory_desc_t *md, *next_md;
void *p, *p1;
@@ -779,6 +780,7 @@ static __init void efi_limit_regions(uns
memmap.nr_map = j;
memmap.map_end = memmap.map +
(memmap.nr_map * memmap.desc_size);
+#endif /* CONFIG_EFI */
}

void __init limit_regions(unsigned long long size)


2006-11-24 17:05:55

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] efi_limit_regions triggers link failure when CONFIG_EFI is not defined

On Friday 24 November 2006 17:59, Andy Whitcroft wrote:
> The following patch is needed to get 2.6.19-rc6-mm1 to compile with
> CONFIG_EFI disabled. This is the 'shortest' fix. However, it does
> appear that there is some overlap with EFI implmentation partly
> being in e820.c and partly in efi.c. It might make sense to move
> everything efi related over to efi.c.

It compiles here. And the ifdef status hasn't changed at all.

Ah maybe your compiler failed to inline the function so the compiler
couldn't optimize it away. What compiler were you using? Does it
go away if you add a "inline" to efi_limit_regions()?

-Andi

2006-11-24 17:24:32

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH] efi_limit_regions triggers link failure when CONFIG_EFI is not defined

Andi Kleen wrote:
> On Friday 24 November 2006 17:59, Andy Whitcroft wrote:
>> The following patch is needed to get 2.6.19-rc6-mm1 to compile with
>> CONFIG_EFI disabled. This is the 'shortest' fix. However, it does
>> appear that there is some overlap with EFI implmentation partly
>> being in e820.c and partly in efi.c. It might make sense to move
>> everything efi related over to efi.c.
>
> It compiles here. And the ifdef status hasn't changed at all.

Right, when it was in the function directly the optimiser seems to have
lopped it off nice and early and got rid of the link failure.

> Ah maybe your compiler failed to inline the function so the compiler
> couldn't optimize it away. What compiler were you using? Does it
> go away if you add a "inline" to efi_limit_regions()?

Compiler is as below:

gcc version 3.3.4 (Debian 1:3.3.4-3)

Yes, making efi_limit_regions() inline also seems to work. Can we
guarentee it will be inlined though? I had the feeling that inline was
advisory and if it does not inline then we will get the link failures.

-apw

2006-11-24 17:33:56

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] efi_limit_regions triggers link failure when CONFIG_EFI is not defined


> Compiler is as below:
>
> gcc version 3.3.4 (Debian 1:3.3.4-3)

Ah, pre unit-at-a-time and some other quirks too. Hopefully
at some point we can unsupport it.

> Yes, making efi_limit_regions() inline also seems to work.

Ok i will make it so.

> Can we
> guarentee it will be inlined though? I had the feeling that inline was
> advisory and if it does not inline then we will get the link failures.

It's defined to __attribute__((always_inline)) inline

-Andi

2006-11-24 21:31:33

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] efi_limit_regions triggers link failure when CONFIG_EFI is not defined

On Fri, 24 Nov 2006 18:33:47 +0100
Andi Kleen <[email protected]> wrote:

> > Can we
> > guarentee it will be inlined though? I had the feeling that inline was
> > advisory and if it does not inline then we will get the link failures.
>
> It's defined to __attribute__((always_inline)) inline

That's an internal implementation detail. Please use __always_inline

2006-11-26 02:15:25

by Matthew Frost

[permalink] [raw]
Subject: Re: [PATCH] efi_limit_regions triggers link failure when CONFIG_EFI is not defined

Andi Kleen wrote:
> On Friday 24 November 2006 17:59, Andy Whitcroft wrote:
>> The following patch is needed to get 2.6.19-rc6-mm1 to compile with
>> CONFIG_EFI disabled. This is the 'shortest' fix. However, it does
>> appear that there is some overlap with EFI implmentation partly
>> being in e820.c and partly in efi.c. It might make sense to move
>> everything efi related over to efi.c.
>
> It compiles here. And the ifdef status hasn't changed at all.

With the "easy fix" it compiles here, too. Thanks!

>
> Ah maybe your compiler failed to inline the function so the compiler
> couldn't optimize it away. What compiler were you using? Does it
> go away if you add a "inline" to efi_limit_regions()?
>
> -Andi

Matt