Hi all,
I didn't get any comment on this patch, so I try to re-submit.
My ARM board hanged at the initial "Calibrating delay loop" message.
After some inspection, I found out the problem to be with commit
adf8b37bafc1495393201a2ae4235846371870d0. This commit introduces stack
unwinding for ARM, and set it enabled by default. However, it seems to
not work with buggy or not-EABI compilers.
My suggestion is to keep the feature (which is fine) but change the
default setting of the option (see the attached patch).
Having this option enabled by default, in fact, means that the kernel
does not boot if the user has a wrong compiler. If so, inspecting the
reason of the hang may require too much time (especially for people not
familiar with this option).
Consider that people may not know that their problem is related to the
ARM_UNWIND option. Therefore, they may waste time bisecting just to
understand where their problem really is.
If we disable this option by default, users who know the real meaning of
the option will still be able of setting it to the proper value.
Any comment ?
Many thanks,
Claudio
On Mon, 2009-10-26 at 09:27 +0100, Claudio Scordino wrote:
> My ARM board hanged at the initial "Calibrating delay loop" message.
>
> After some inspection, I found out the problem to be with commit
> adf8b37bafc1495393201a2ae4235846371870d0. This commit introduces stack
> unwinding for ARM, and set it enabled by default. However, it seems to
> not work with buggy or not-EABI compilers.
>
> My suggestion is to keep the feature (which is fine) but change the
> default setting of the option (see the attached patch).
The option still depends on EXPERIMENTAL, so you get ARM_UNWIND on when
enabling that.
I'm more in favour of a #warning on #error in the unwind.c file based on
the compiler version rather than not having it on by default. The reason
is that people reported performance improvements when compiling the
kernel without frame pointers.
--
Catalin
Catalin Marinas ha scritto:
> On Mon, 2009-10-26 at 09:27 +0100, Claudio Scordino wrote:
>
>> My ARM board hanged at the initial "Calibrating delay loop" message.
>>
>> After some inspection, I found out the problem to be with commit
>> adf8b37bafc1495393201a2ae4235846371870d0. This commit introduces stack
>> unwinding for ARM, and set it enabled by default. However, it seems to
>> not work with buggy or not-EABI compilers.
>>
>> My suggestion is to keep the feature (which is fine) but change the
>> default setting of the option (see the attached patch).
>>
>
> The option still depends on EXPERIMENTAL, so you get ARM_UNWIND on when
> enabling that.
>
> I'm more in favour of a #warning on #error in the unwind.c file based on
> the compiler version rather than not having it on by default. The reason
> is that people reported performance improvements when compiling the
> kernel without frame pointers.
>
This solution is fine too (even if I still think that changing the
default setting is better).
Please, consider the patch in attachment: is it like you would have it ?
BTW, do we have any list of buggy or not-EABI versions of the gcc compiler ?
Many thanks,
Claudio
Catalin Marinas ha scritto:
> On Mon, 2009-10-26 at 09:27 +0100, Claudio Scordino wrote:
>
>> My ARM board hanged at the initial "Calibrating delay loop" message.
>>
>> After some inspection, I found out the problem to be with commit
>> adf8b37bafc1495393201a2ae4235846371870d0. This commit introduces stack
>> unwinding for ARM, and set it enabled by default. However, it seems to
>> not work with buggy or not-EABI compilers.
>>
>> My suggestion is to keep the feature (which is fine) but change the
>> default setting of the option (see the attached patch).
>>
>
> The option still depends on EXPERIMENTAL, so you get ARM_UNWIND on when
> enabling that.
>
> I'm more in favour of a #warning on #error in the unwind.c file based on
> the compiler version rather than not having it on by default. The reason
> is that people reported performance improvements when compiling the
> kernel without frame pointers.
>
[Sorry, I sent the wrong patch... This is the new one.]
This solution is fine too (even if I still think that changing the
default setting is better).
Please, consider the patch in attachment: is it like you would have it ?
BTW, do we have any list of buggy or not-EABI versions of the gcc compiler ?
Many thanks,
Claudio
On Wed, 2009-10-28 at 12:37 +0100, Claudio Scordino wrote:
> Catalin Marinas ha scritto:
> > On Mon, 2009-10-26 at 09:27 +0100, Claudio Scordino wrote:
> >
> >> My ARM board hanged at the initial "Calibrating delay loop" message.
> >>
> >> After some inspection, I found out the problem to be with commit
> >> adf8b37bafc1495393201a2ae4235846371870d0. This commit introduces stack
> >> unwinding for ARM, and set it enabled by default. However, it seems to
> >> not work with buggy or not-EABI compilers.
> >>
> >> My suggestion is to keep the feature (which is fine) but change the
> >> default setting of the option (see the attached patch).
> >
> > The option still depends on EXPERIMENTAL, so you get ARM_UNWIND on when
> > enabling that.
> >
> > I'm more in favour of a #warning on #error in the unwind.c file based on
> > the compiler version rather than not having it on by default. The reason
> > is that people reported performance improvements when compiling the
> > kernel without frame pointers.
>
> [Sorry, I sent the wrong patch... This is the new one.]
and the wrong ARM kernel list (I changed the Cc line).
> This solution is fine too (even if I still think that changing the
> default setting is better).
>
> Please, consider the patch in attachment: is it like you would have it ?
You can just send it to Russell's patch system once there are no more
comments on it.
See below for my comments.
> BTW, do we have any list of buggy or not-EABI versions of the gcc
> compiler ?
No, but for non-EABI compilers I think the __ARM_EABI__ check is enough.
> From: Claudio Scordino <[email protected]>
> Date: Mon, 26 Oct 2009 17:13:18 +0100
> Subject: [PATCH 1/1] Some warnings when compiling ARM unwind support with a buggy or not-EABI compiler.
>
> ARM unwind support is known to build only with EABI and not-buggy compilers.
> Now we check the compiler and raise a #warning in case of wrong compiler.
Maybe you can clarify a bit that the problem is not unwinding
information but the -fno-frame-pointer option added as a result of
!CONFIG_FRAME_POINTER.
> Signed-off-by: Claudio Scordino <[email protected]>
> ---
> arch/arm/kernel/unwind.c | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/kernel/unwind.c b/arch/arm/kernel/unwind.c
> index 39baf11..47345c1 100644
> --- a/arch/arm/kernel/unwind.c
> +++ b/arch/arm/kernel/unwind.c
> @@ -26,6 +26,15 @@
> * http://infocenter.arm.com/help/topic/com.arm.doc.subset.swdev.abi/index.html
> */
>
> +#if !defined (__ARM_EABI__)
> +#warning Your compiler does not have EABI support.
> +#warning ARM unwind support it is known to compile only on EABI compilers.
Maybe "ARM unwind is known ... only with ..." (though English is not my
first language).
> +#warning Change compiler or disable ARM_UNWIND option.
> +#elif (__GNUC__ == 4 && __GNUC_MINOR__ == 2)
Could we assume this for (__GNUC_MINOR__ <= 2)?
> +#warning Your compiler is too buggy; it is known to not compile ARM unwind support.
> +#warning Change compiler or disable ARM_UNWIND option.
> +#endif
> +
> #include <linux/kernel.h>
> #include <linux/init.h>
> #include <linux/module.h>
--
Catalin
Hi,
I modified the patch according to your suggestions.
>
> You can just send it to Russell's patch system once there are no more
> comments on it.
Done. I'm attaching the latest version here too.
Many thanks,
Claudio
On Fri, 2009-10-30 at 12:11 +0100, Claudio Scordino wrote:
> From: Claudio Scordino <[email protected]>
> Date: Fri, 30 Oct 2009 11:46:00 +0100
> Subject: [PATCH 1/1] ARM unwind is known to compile only with EABI and
> not-buggy compilers.
>
> ARM unwind is known to compile only with EABI and not-buggy compilers.
> The problem is not the unwinding information but the
> -fno-frame-pointer option added as a result of !CONFIG_FRAME_POINTER.
> Now we check the compiler and raise a #warning in case of wrong
> compiler.
>
>
> Signed-off-by: Claudio Scordino <[email protected]>
Unless others have objections, here's my ack:
Acked-by: Catalin Marinas <[email protected]>
--
Catalin