Hi Arnd,
2017-03-29 18:30 GMT+09:00 Arnd Bergmann <arnd@xxxxxxxx>:
> On Wed, Mar 29, 2017 at 4:07 AM, Masahiro Yamada
> <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>> Hi Arnd,
>>
>>
>> 2017-03-15 6:37 GMT+09:00 Arnd Bergmann <arnd@xxxxxxxx>:
>>> When ftrace is enabled and we build with gcc-4.7 or older, we
>>> get a warning for each file on architectures that select
>>> CONFIG_LD_DEAD_CODE_DATA_ELIMINATION:
>>>
>>> warning: -ffunction-sections disabled; it makes profiling impossible [enabled by default]
>>>
>>> This turns off function sections in that specific case, leaving
>>> it enabled for all other configurations.
>>>
>>> Fixes: b67067f1176d ("kbuild: allow archs to select link dead code/data elimination")
>>> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
>>> Cc: Namhyung Kim <namhyung.with.foss@xxxxxxxxx>
>>> ---
>>> v2: accidentally resend the same patch as before
>>> v3: send the exact same patch once more, without doing the change I wanted
>>> v4: actually fixed version number in check as pointed out by Namhyung Kim (I hope)
>>> ---
>>> Makefile | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 6e7e644a0b84..3a964fa3a787 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -662,7 +662,11 @@ KBUILD_CFLAGS += -Wextra
>>> KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
>>>
>>> ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>>> +ifdef CONFIG_FUNCTION_TRACER
>>> +KBUILD_CFLAGS += $(call cc-ifversion, -ge,0408,$(call cc-option,-ffunction-sections,))
>>> +else
>>> KBUILD_CFLAGS += $(call cc-option,-ffunction-sections,)
>>> +endif
>>> KBUILD_CFLAGS += $(call cc-option,-fdata-sections,)
>>> endif
>>>
>>
>>
>> I have two questions.
>>
>> [1]
>> How did you produce this warning?
>> I do not see any architecture that selects this option at this point of time.
>
> I saw it on ARM randconfig builds
>
>> Did you edit Kconfig locally to select LD_DEAD_CODE_DATA_ELIMINATION?
>> If so, is this patch not so urgent as the "Fixes" tag claims?
>
> Good point, I forgot that I had enabled this manually on ARM in an
> earlier patch in my randconfig-fixup series. I thought this was selected
> on powerpc, but that part of Nick's series apparently didn't make it in
> yet.
>
> I don't see the 'Fixes' tag as claiming the patch to be urgent, just
> documenting what patch introduced the problem, but you could
> argue here that it only gets introduced by a future patch that
> actually selects the symbol.
I won't argue. Fixes is reviewer-friendly
for finding the related commit.
I just personally used the "Fix" keyword as a hint for priority
because there are still many kbuild patches piled up,
and I have not been able to catch up yet.
>> [2]
>> This question is more technical.
>>
>> The cause of the problem seems that GCC warns it cannot support the
>> two at the same time,
>> but it does handle it as an error. So, cc-option assumes this
>> combination is OK.
>>
>> Is it a good idea to add -Werror to cc-option checking?
>
> Hmm, I've actually been running with that change as a side-effect
> of having the llvmlinux patches applied, which contain this one:
>
> commit 03497934e211325fc2913476897daf7a5ddb008a
> Author: Mark Charlebois <charlebm@xxxxxxxxx>
> Date: Mon Sep 22 14:33:05 2014 -0700
>
> kbuild, LLVMLinux: Add -Werror to cc-option to support clang
>
> Clang will warn about unknown warnings but will not return false
> unless -Werror is set. GCC will return false if an unknown
> warning is passed.
>
> Adding -Werror make both compiler behave the same.
>
> Signed-off-by: Mark Charlebois <charlebm@xxxxxxxxx>
> Signed-off-by: Behan Webster <behanw@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Jan-Simon Möller <dl9pf@xxxxxx>
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 285acc57c0a4..3629bd9c7e79 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -116,12 +116,12 @@ CC_OPTION_CFLAGS = $(filter-out
> $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS))
> # Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
>
> cc-option = $(call try-run,\
> - $(CC) $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c
> /dev/null -o "$$TMP",$(1),$(2))
> + $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c
> -x c /dev/null -o "$$TMP",$(1),$(2))
>
> # cc-option-yn
> # Usage: flag := $(call cc-option-yn,-march=winchip-c6)
> cc-option-yn = $(call try-run,\
> - $(CC) $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c -x c
> /dev/null -o "$$TMP",y,n)
> + $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) $(1) -c
> -x c /dev/null -o "$$TMP",y,n)
>
> # cc-option-align
> # Prefix align with either -falign or -malign
> @@ -131,7 +131,7 @@ cc-option-align = $(subst -functions=0,,\
> # cc-disable-warning
> # Usage: cflags-y += $(call cc-disable-warning,unused-but-set-variable)
> cc-disable-warning = $(call try-run,\
> - $(CC) $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip $(1))
> -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
> + $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -W$(strip
> $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
>
> # cc-name
> # Expands to either gcc or clang
>
> This is identical to your version, except it applies the same
> thing to cc-disable-warning.
Ah, I see.
I'm also moving
KBUILD_CFLAGS += $(call cc-option,-ffunction-sections,)
below
CC_FLAGS_FTRACE := -pg
to fix the warning.
> I think this is good to get merged,
> and there are probably a couple of other patches in that series
> that we may want to look at again.
I agree.
At least, 03497934e21 looks good to be merged.
(I need to get access to Mark, and ask him to send this patch.)
Then, swap the -ffunction-sections and -pg order.
I hope this will make you and clang guys happy.
--
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
