[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AANLkTinLAZubVih-y6=EsGnd0yLmFDUcG10u-EbozUS0@mail.gmail.com>
Date: Wed, 9 Mar 2011 11:16:30 +0100
From: Sedat Dilek <sedat.dilek@...glemail.com>
To: Ingo Molnar <mingo@...e.hu>
Cc: "H.J. Lu" <hjl.tools@...il.com>,
Alexander van Heukelum <heukelum@...tmail.fm>,
linux-next <linux-next@...r.kernel.org>,
psomas@...ab.ece.ntua.gr, Jan Beulich <JBeulich@...ell.com>,
"H. Peter Anvin" <hpa@...or.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org
Subject: Re: linux-next: Tree for March 8 (BROKEN: arch/x86/kernel/entry_32.S?
Debian's binutils/as?)
On Wed, Mar 9, 2011 at 9:51 AM, Ingo Molnar <mingo@...e.hu> wrote:
>
> * H.J. Lu <hjl.tools@...il.com> wrote:
>
>> On Tue, Mar 8, 2011 at 12:33 PM, Sedat Dilek <sedat.dilek@...glemail.com> wrote:
>> > On Tue, Mar 8, 2011 at 9:25 PM, Alexander van Heukelum
>> > <heukelum@...tmail.fm> wrote:
>> >> On Tue, 08 Mar 2011 18:53 +0100, "Sedat Dilek" <sedat.dilek@...glemail.com> wrote:
>> >>> On Tue, Mar 8, 2011 at 6:27 PM, Alexander van Heukelum
>> >>> <heukelum@...tmail.fm> wrote:
>> >>> > On Tue, 08 Mar 2011 16:42 +0100, "Sedat Dilek" <sedat.dilek@...glemail.com> wrote:
>> >>> >> On 3/8/11, Sedat Dilek <sedat.dilek@...glemail.com> wrote:
>> >>> >> > On 3/8/11, H.J. Lu <hjl.tools@...il.com> wrote:
>> >>> >> >> On Tue, Mar 8, 2011 at 2:44 AM, Sedat Dilek <sedat.dilek@...glemail.com>
>> >>> >> >> wrote:
>> >>> >> >>> Hi,
>> >>> >> >>>
>> >>> >> >>> my build of linux-next (next-20110308, the same with the one from
>> >>> >> >>> yesterday) is broken.
>> >>> >> >>> (I translated the German output.)
>> >>> >> >>>
>> >>> >> >>> [ build.log ]
>> >>> >> >>> AS arch/x86/kernel/entry_32.o
>> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> >>> >> >>> Assembler messages:
>> >>> >> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> >>> >> >>> Error: .size expression does not evaluate to a constant
>> >>> >> >>> make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1 (Error 1)
>> >>> >> >>> make[5]: *** [arch/x86/kernel] Fehler 2 (Error 2)
>> >>> >> >>> make[4]: *** [arch/x86] Fehler 2 (Error 2)
>> >>> >> >>> make[4]: *** Warte auf noch nicht beendete Prozesse... (Waiting for
>> >>> >> >>> unfinished jobs...)
>> >>> >> >>>
>> >>> >> >>
>> >>> >> >> This is a kernel bug. Please use the latest binutils from CVS.
>> >>> >> >> It will tell you which symbol causes this.
>> >>> >> >>
>> >>> >> >>
>> >>> >> >> --
>> >>> >> >> H.J.
>> >>> >> >>
>> >>> >> >
>> >>> >> > Yeah, I have cherry-picked these two upstream commits before you have
>> >>> >> > mentionned it...
>> >>> >> >
>> >>> >> > 0001-Mention-symbol-name-in-non-constant-.size-expression.patch
>> >>> >> > (Cherry-picked from commit b9521fc0be7945fc842ce1197e241a023378125d)
>> >>> >> > 0002-Revert-the-last-change-on-gas-elf-bad-size.err.patch
>> >>> >> > (Cherry-picked from commit cbd141bb69f791de7ea1581abe7afb34f0c61288)
>> >>> >> >
>> >>> >> > ... and have built with them a new binutils Debian package.
>> >>> >> >
>> >>> >> > The error looks now like this (sorry for the German output):
>> >>> >> > ...
>> >>> >> > AS arch/x86/kernel/entry_32.o
>> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:
>> >>> >> > Assembler messages:
>> >>> >> > /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/entry_32.S:1421:
>> >>> >> > Error: .size expression with symbol `apf_page_fault' does not evaluate
>> >>> >> > to a constant
>> >>> >> > make[6]: *** [arch/x86/kernel/entry_32.o] Fehler 1
>> >>> >> > make[5]: *** [arch/x86/kernel] Fehler 2
>> >>> >> > make[5]: *** Warte auf noch nicht beendete Prozesse...
>> >>> >> >
>> >>> >> > Anyway, before more riddling around it would be very helpful to have a
>> >>> >> > clear pointer if there is a fix around... That building, testing and
>> >>> >> > installing took me now several hours.
>> >>> >> > And... yeah, backports to 2.21-branch appreciated.
>> >>> >> >
>> >>> >> > - Sedat -
>> >>> >> >
>> >>> >>
>> >>> >> After a quick look into the source, it seems attached patch fixes the
>> >>> >> issue.
>> >>> >> Is that OK?
>> >>> >
>> >>> > Hi Sedat,
>> >>> >
>> >>> > The patch ( https://lkml.org/lkml/2011/3/8/203 ) is ok, feel free to add
>> >>> > Acked-by: Alexander van Heukelum <heukelum@...tmail.fm>
>> >>> >
>> >>> > Better description might be something like:
>> >>> >
>> >>> > i386: Fix mismatched ENTRY/END pair.
>> >>> >
>> >>> > Under CONFIG_KVM_GUEST=y, the following part of entry_32.S causes a compile failure.
>> >>> >
>> >>> > 1409 #ifdef CONFIG_KVM_GUEST
>> >>> > 1410 ENTRY(async_page_fault)
>> >>> > 1411 RING0_EC_FRAME
>> >>> > 1412 pushl $do_async_page_fault
>> >>> > 1413 CFI_ADJUST_CFA_OFFSET 4
>> >>> > 1414 jmp error_code
>> >>> > 1415 CFI_ENDPROC
>> >>> > 1416 END(apf_page_fault)
>> >>> > 1417 #endif
>> >>> >
>> >>> > Replace apf_page_fault with async_page_fault, as intended.
>> >>> >
>> >>> > Greetings,
>> >>> > Alexander
>> >>> >
>> >>> >> - Sedat -
>> >>> >>
>> >>> >> Email had 1 attachment:
>> >>> >> + 0001-x86-Fix-build-failure-with-binutils-as-from-upstream.patch
>> >>> >> 1k (text/x-patch)
>> >>> >
>> >>>
>> >>> As I said, quick view on the code, quick fix :-).
>> >>>
>> >>> Your description is definitive more meaningful.
>> >>> I can refresh my patch and add your ACK.
>> >>>
>> >>> Anyway, I continued after dinner and with the above patch I ran into
>> >>> the next problem:
>> >>> [ build.log ]
>> >>> ...
>> >>> AS arch/x86/kernel/acpi/wakeup_rm.o
>> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:
>> >>> Assembler messages:
>> >>> /home/sd/src/linux-2.6/linux-2.6.38-rc7/debian/build/source_i386_none/arch/x86/kernel/acpi/wakeup_rm.S:12:
>> >>> Error: .size expression with symbol `wakeup_code_start' does not
>> >>> evaluate to a constant
>> >>
>> >> No idea what's wrong there. But my version of wakeup_rm.S has only 10 lines...
>> >>
>> >> 1 /*
>> >> 2 * Wrapper script for the realmode binary as a transport object
>> >> 3 * before copying to low memory.
>> >> 4 */
>> >> 5 .section ".rodata","a"
>> >> 6 .globl wakeup_code_start, wakeup_code_end
>> >> 7 wakeup_code_start:
>> >> 8 .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
>> >> 9 wakeup_code_end:
>> >> 10 .size wakeup_code_start, .-wakeup_code_start
>> >>
>> >> And it compiles just fine.
>> >> The fix for entry_32.S is valid, though, and necessary for mainline.
>> >>
>> >> Greetings,
>> >> Alexander
>> >>
>> >>> I am unsure how to fix that and open for feedback.
>> >>>
>> >>> - Sedat -
>> >>>
>> >>
>> >
>> > The file in linux-next (next-20110308) looks different (the above code
>> > looks more logical to me)
>> >
>> > [ arch/x86/kernel/acpi/wakeup_rm.S ]
>> >
>> > /*
>> > * Wrapper script for the realmode binary as a transport object
>> > * before copying to low memory.
>> > */
>> > #include <asm/page_types.h>
>> >
>> > .section ".x86_trampoline","a"
>> > .balign PAGE_SIZE
>> > .globl acpi_wakeup_code
>> > acpi_wakeup_code:
>> > .incbin "arch/x86/kernel/acpi/realmode/wakeup.bin"
>> > .size wakeup_code_start, .-wakeup_code_start
>> >
>>
>> Those are simply wrong. 2.6.38-rc8 is OK.
>
> 2.6.37-rc8 is not OK: for example commit 631bc4878220932fe67fc46fc7cf7cccdb1ec597 is
> already upstream and if you enable KVM you see a broken kernel build with new
> binutils. This is from 2.6.38-rc8:
>
> #ifdef CONFIG_KVM_GUEST
> ENTRY(async_page_fault)
> RING0_EC_FRAME
> pushl $do_async_page_fault
> CFI_ADJUST_CFA_OFFSET 4
> jmp error_code
> CFI_ENDPROC
> END(apf_page_fault)
> #endif
>
> Yes, the .size directive not matching up is technically a bug, but it was not
> checked by binutils before, for *years* - and you cannot just flip a switch and
> break who knows how much code.
>
> You need to at least emit a warning for some time to give people a *chance* to fix
> bugs - not just stuff an incompatible binutils down their throat and break the
> kernel build for thousands of commits and break bisection.
>
> This binutils change is breaking numerous upstream kernel builds (and is making
> bisection with new binutils impossible) for no particular good reason: binutils was
> capable to figure out the symbol name before this change.
>
> At minimum you need to *understand* that what you are doing is an incompatible
> change and is disruptive to others ...
>
> Thanks,
>
> Ingo
>
OK, I am not a toolchain expert, but this recent changes to upstream
binutils were helpful to find at least the problematic place in the
code (fix see [1]).
I only needed a blink of an eye to catch it.
binutils 2.21 GIT has this change "PR gas/12519" which was not "perfect".
With the patch in upstream (see [2], which should be cherry-picked for
2.21-GIT as I did for my debianized binutils), there is more
"verbosity", now.
We have now an advantage as we know what's going on *before* final
binutils-2.21 release.
What are you expecting from binutils developers?
Shall they revert the above PR (yes, I also build a binutils with a
revert of it)?
The next place in code (I am on linux-next) could also be found easily
and with some help of x86 folks it could be fixed [2], too.
Can you please explain the "incompatibility" and "breakage" with older
kernels (especially for bisecting sounds awful)?
(I am just curious and the several builds of binutils and linux-next
kernels I did yesterday should not be for nothing :-).)
If I can help (like testing or whatever), please let me know.
- Sedat -
[1] https://patchwork.kernel.org/patch/619081/
[2] http://sourceware.org/ml/binutils/2011-03/msg00078.html
[3] https://patchwork.kernel.org/patch/619611/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists