Re: [PATCH v8 02/10] Makefile: Prepare for using macros for inline asm

[Nadav Amit]

at 3:01 PM, Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote:

> Hi.

Thanks for your reply. Your advices are always valuable!

> 
> 
> 
> 2018å9æ27æ(æ) 2:57 Nadav Amit <namit@xxxxxxxxxx>:
>> at 1:58 AM, Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx> 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