* Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm
2018-09-26 17:56 ` Nadav Amit
@ 2018-09-27 7:52 ` Rasmus Villemoes
2018-10-01 22:01 ` Masahiro Yamada
2018-10-02 10:59 ` Ingo Molnar
2 siblings, 0 replies; 9+ messages in thread
From: Rasmus Villemoes @ 2018-09-27 7:52 UTC (permalink / raw)
To: Nadav Amit, Ingo Molnar
Cc: LKML, X86 ML, Sam Ravnborg, Michal Marek, Thomas Gleixner,
H. Peter Anvin, linux-kbuild@vger.kernel.org
On 2018-09-26 19:56, Nadav Amit wrote:
> at 1:58 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
>>> +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
>>> +export ASM_MACRO_FLAGS
>>> +KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
>> How does this affect what gets rebuilt when one of the asm/foo.h files
>> going into macros.s changes? Does that cause a global rebuild because
>> everything depends on macros.s, or do we still only rebuild the files
>> that actually include asm/foo.h?
>
> There will not be a global rebuild. Any source file that uses the header
> files that are included in macros.S will be rebuilt.
>
> But your question actually raises two issues:
>
> 1. If macros.S changes, there *should* be a global rebuild, and currently
> there wouldn’t be one. Do you happen to know what would be the appropriate
> way to trigger one?
Hm, that's a question for Kbuild folks, I think. Maybe one could just
ensure macros.s gets written into the dependency file for each
translation unit (maybe fixdep already has support for that). But if we
do that, we probably want macros.s to be generated with some if_changed
makefile magic, so that changes in one of the asm/ headers that do not
actually change macros.s does not cause a global rebuild (it would still
rebuild the files that actually inclue that asm/ header, of course).
> 2. Someone might mistakenly use the macros through inline assembly without
> including the header file.
That's rather unlikely, I think. An open-coded asm("SOME_MACRO ...")
would hopefully be caught in review. But yeah...
> It would be better to detect such cases and fail
> the build. I may be able to create another asm macro in the C part of the
> header that would cause the build to fail. But let me know if you any
> better idea.
The best I can think of (and I'm not suggesting to do this, or that it
would actually work): In asm/foo.h, inside its cpp include guard, and
inside a guard that excludes this piece when building macros.s itself,
have a 'asm("___asm_foo_included__ = 1\n")'. Then inside each asm macro
defined in asm/foo.h, have a ".ifndef ___asm_foo_included___\n.error
...\n.endif". That latter part might be macrofied itself so that it's
just one line, "CHECK_INCLUDE ___asm_foo_included___\n", with
CHECK_INCLUDE being defined in macros.S.
Rasmus
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm
2018-09-26 17:56 ` Nadav Amit
2018-09-27 7:52 ` Rasmus Villemoes
@ 2018-10-01 22:01 ` Masahiro Yamada
2018-10-03 19:41 ` Nadav Amit
2018-10-02 10:59 ` Ingo Molnar
2 siblings, 1 reply; 9+ messages in thread
From: Masahiro Yamada @ 2018-10-01 22:01 UTC (permalink / raw)
To: Nadav Amit
Cc: Rasmus Villemoes, Ingo Molnar, Linux Kernel Mailing List, X86 ML,
Sam Ravnborg, Michal Marek, Thomas Gleixner, H. Peter Anvin,
Linux Kbuild mailing list
Hi.
2018年9月27日(木) 2:57 Nadav Amit <namit@vmware.com>:
>
> at 1:58 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
> > On 2018-09-18 23:28, Nadav Amit wrote:
> >
> >> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >> index 8f6e7eb8ae9f..944fa3bc9376 100644
> >> --- a/arch/x86/Makefile
> >> +++ b/arch/x86/Makefile
> >> @@ -214,8 +214,8 @@ ifdef CONFIG_X86_64
> >> KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
> >> endif
> >>
> >> -# Speed up the build
> >> -KBUILD_CFLAGS += -pipe
> >> +# We cannot use -pipe flag since we give an additional .s file to the compiler
> >> +#KBUILD_CFLAGS += -pipe
> >
> > Is this really necessary? The gas manual says that one can use -- to
> > name stdin, though that's probably a typo and should just be - . Doing
> >
> > gcc -pipe -Wa,foo.s -Wa,-
>
> Good idea. I didn’t think of it. Yes, this can do the trick. I’ll do it in
> v9.
>
> > does seem to work as expected (and would also make it possible to append
> > some .s file should that ever be required).
> >> +archmacros:
> >> + $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
> >> +
> >> +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
> >> +export ASM_MACRO_FLAGS
> >> +KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
> > How does this affect what gets rebuilt when one of the asm/foo.h files
> > going into macros.s changes? Does that cause a global rebuild because
> > everything depends on macros.s, or do we still only rebuild the files
> > that actually include asm/foo.h?
>
> There will not be a global rebuild. Any source file that uses the header
> files that are included in macros.S will be rebuilt.
>
> But your question actually raises two issues:
>
> 1. If macros.S changes, there *should* be a global rebuild,
Looking at this series, I can see this rule:
"macros and inline functions that calls them are placed in the same header".
For example,
REFCOUNT_CHECK_LE_ZERO is defined in <asm/refcount.h>,
and REFCOUNT_CHECK_LE_ZERO is invoked via refcount_sub_and_test() etc.
in the same header.
Therefore, all users of REFCOUNT_CHECK_LE_ZERO must have included
<asm/refcount.h>
This means all C files using REFCOUNT_CHECK_LE_ZERO
will be appropriately recompiled as long as the rule above is observed.
> and currently
> there wouldn’t be one. Do you happen to know what would be the appropriate
> way to trigger one?
>
> 2. Someone might mistakenly use the macros through inline assembly without
> including the header file.
Right, it is possible to use REFCOUNT_CHECK_LE_ZERO directly.
If this happens, my assumption breaks.
It would be unlikely to happen, though...
> It would be better to detect such cases and fail
> the build. I may be able to create another asm macro in the C part of the
> header that would cause the build to fail. But let me know if you any
> better idea.
>
> Regards,
> Nadav
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm
2018-10-01 22:01 ` Masahiro Yamada
@ 2018-10-03 19:41 ` Nadav Amit
0 siblings, 0 replies; 9+ messages in thread
From: Nadav Amit @ 2018-10-03 19:41 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Rasmus Villemoes, Ingo Molnar, Linux Kernel Mailing List, X86 ML,
Sam Ravnborg, Michal Marek, Thomas Gleixner, H. Peter Anvin,
Linux Kbuild mailing list
at 3:01 PM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Hi.
Thanks for your reply. Your advices are always valuable!
>
>
>
> 2018年9月27日(木) 2:57 Nadav Amit <namit@vmware.com>:
>> at 1:58 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>>
>>> On 2018-09-18 23:28, Nadav Amit wrote:
>>>
>>>> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>>>> index 8f6e7eb8ae9f..944fa3bc9376 100644
>>>> --- a/arch/x86/Makefile
>>>> +++ b/arch/x86/Makefile
>>>> @@ -214,8 +214,8 @@ ifdef CONFIG_X86_64
>>>> KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
>>>> endif
>>>>
>>>> -# Speed up the build
>>>> -KBUILD_CFLAGS += -pipe
>>>> +# We cannot use -pipe flag since we give an additional .s file to the compiler
>>>> +#KBUILD_CFLAGS += -pipe
>>>
>>> Is this really necessary? The gas manual says that one can use -- to
>>> name stdin, though that's probably a typo and should just be - . Doing
>>>
>>> gcc -pipe -Wa,foo.s -Wa,-
>>
>> Good idea. I didn’t think of it. Yes, this can do the trick. I’ll do it in
>> v9.
>>
>>> does seem to work as expected (and would also make it possible to append
>>> some .s file should that ever be required).
>>>> +archmacros:
>>>> + $(Q)$(MAKE) $(build)=arch/x86/kernel arch/x86/kernel/macros.s
>>>> +
>>>> +ASM_MACRO_FLAGS = -Wa,arch/x86/kernel/macros.s
>>>> +export ASM_MACRO_FLAGS
>>>> +KBUILD_CFLAGS += $(ASM_MACRO_FLAGS)
>>> How does this affect what gets rebuilt when one of the asm/foo.h files
>>> going into macros.s changes? Does that cause a global rebuild because
>>> everything depends on macros.s, or do we still only rebuild the files
>>> that actually include asm/foo.h?
>>
>> There will not be a global rebuild. Any source file that uses the header
>> files that are included in macros.S will be rebuilt.
>>
>> But your question actually raises two issues:
>>
>> 1. If macros.S changes, there *should* be a global rebuild,
>
>
> Looking at this series, I can see this rule:
> "macros and inline functions that calls them are placed in the same header".
>
>
> For example,
> REFCOUNT_CHECK_LE_ZERO is defined in <asm/refcount.h>,
> and REFCOUNT_CHECK_LE_ZERO is invoked via refcount_sub_and_test() etc.
> in the same header.
>
> Therefore, all users of REFCOUNT_CHECK_LE_ZERO must have included
> <asm/refcount.h>
>
> This means all C files using REFCOUNT_CHECK_LE_ZERO
> will be appropriately recompiled as long as the rule above is observed.
Yes. My concern is not what happens if any of the files included from
macros.S changes, but what happens if macros.S changes (e.g., to include
an additional header).
Maybe I can create some artificial dependency based on __ASSEMBLY__ . I’ll
give it another thought.
>> and currently
>> there wouldn’t be one. Do you happen to know what would be the appropriate
>> way to trigger one?
>>
>> 2. Someone might mistakenly use the macros through inline assembly without
>> including the header file.
>
> Right, it is possible to use REFCOUNT_CHECK_LE_ZERO directly.
> If this happens, my assumption breaks.
>
> It would be unlikely to happen, though...
Yes. I’ll let it go for now.
Thanks,
Nadav
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm
2018-09-26 17:56 ` Nadav Amit
2018-09-27 7:52 ` Rasmus Villemoes
2018-10-01 22:01 ` Masahiro Yamada
@ 2018-10-02 10:59 ` Ingo Molnar
2018-10-03 19:43 ` hpa
2 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2018-10-02 10:59 UTC (permalink / raw)
To: Nadav Amit
Cc: Rasmus Villemoes, Ingo Molnar, LKML, X86 ML, Sam Ravnborg,
Michal Marek, Thomas Gleixner, H. Peter Anvin,
linux-kbuild@vger.kernel.org
* Nadav Amit <namit@vmware.com> wrote:
> at 1:58 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>
> > On 2018-09-18 23:28, Nadav Amit wrote:
> >
> >> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >> index 8f6e7eb8ae9f..944fa3bc9376 100644
> >> --- a/arch/x86/Makefile
> >> +++ b/arch/x86/Makefile
> >> @@ -214,8 +214,8 @@ ifdef CONFIG_X86_64
> >> KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
> >> endif
> >>
> >> -# Speed up the build
> >> -KBUILD_CFLAGS += -pipe
> >> +# We cannot use -pipe flag since we give an additional .s file to the compiler
> >> +#KBUILD_CFLAGS += -pipe
> >
> > Is this really necessary? The gas manual says that one can use -- to
> > name stdin, though that's probably a typo and should just be - . Doing
> >
> > gcc -pipe -Wa,foo.s -Wa,-
>
> Good idea. I didn’t think of it. Yes, this can do the trick. I’ll do it in
> v9.
Ok, will wait for v9.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm
2018-10-02 10:59 ` Ingo Molnar
@ 2018-10-03 19:43 ` hpa
2018-10-03 20:29 ` Nadav Amit
0 siblings, 1 reply; 9+ messages in thread
From: hpa @ 2018-10-03 19:43 UTC (permalink / raw)
To: Ingo Molnar, Nadav Amit
Cc: Rasmus Villemoes, Ingo Molnar, LKML, X86 ML, Sam Ravnborg,
Michal Marek, Thomas Gleixner, linux-kbuild@vger.kernel.org
On October 2, 2018 3:59:52 AM PDT, Ingo Molnar <mingo@kernel.org> wrote:
>
>* Nadav Amit <namit@vmware.com> wrote:
>
>> at 1:58 AM, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
>>
>> > On 2018-09-18 23:28, Nadav Amit wrote:
>> >
>> >> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>> >> index 8f6e7eb8ae9f..944fa3bc9376 100644
>> >> --- a/arch/x86/Makefile
>> >> +++ b/arch/x86/Makefile
>> >> @@ -214,8 +214,8 @@ ifdef CONFIG_X86_64
>> >> KBUILD_LDFLAGS += $(call ld-option, -z max-page-size=0x200000)
>> >> endif
>> >>
>> >> -# Speed up the build
>> >> -KBUILD_CFLAGS += -pipe
>> >> +# We cannot use -pipe flag since we give an additional .s file to
>the compiler
>> >> +#KBUILD_CFLAGS += -pipe
>> >
>> > Is this really necessary? The gas manual says that one can use --
>to
>> > name stdin, though that's probably a typo and should just be - .
>Doing
>> >
>> > gcc -pipe -Wa,foo.s -Wa,-
>>
>> Good idea. I didn’t think of it. Yes, this can do the trick. I’ll do
>it in
>> v9.
>
>Ok, will wait for v9.
>
>Thanks,
>
> Ingo
Does -pipe actually win anything these days?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm
2018-10-03 19:43 ` hpa
@ 2018-10-03 20:29 ` Nadav Amit
0 siblings, 0 replies; 9+ messages in thread
From: Nadav Amit @ 2018-10-03 20:29 UTC (permalink / raw)
To: hpa@zytor.com, Ingo Molnar
Cc: Rasmus Villemoes, Ingo Molnar, LKML, X86 ML, Sam Ravnborg,
Michal Marek, Thomas Gleixner, linux-kbuild@vger.kernel.org
at 12:43 PM, hpa@zytor.com wrote:
>
> Does -pipe actually win anything these days?
Sorry, I don’t have the time to check it right now. Anyhow, I presume it is
best that such a change will be in a separate patch.
^ permalink raw reply [flat|nested] 9+ messages in thread