All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] objtool: remove generated files with make clean
@ 2019-02-14 21:08 Robin Meijboom
  2019-02-28 13:36 ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Meijboom @ 2019-02-14 21:08 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek
  Cc: linux-kbuild, Josh Poimboeuf, Peter Zijlstra

Make clean currently does not remove the generated files for objtool:
tools/objtool/objtool, tools/objtool/fixdep, and
tools/objtool/arch/x86/lib/inat-tables.c.

Clean these files up as part of make clean.

Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).

Signed-off-by: Robin Meijboom <robin@meijboom.info>
---
In the discussions I didn't find a reason for keeping the files, so I
assume it is an oversight. Otherwise I would have expected them to be
removed at least by make distconfig (which they are not).

Tested by compiling, cleaning, compiling again, and booting on x86_64.

diff --git a/Makefile b/Makefile
index 141653226f3c..81a8149a805f 100644
--- a/Makefile
+++ b/Makefile
@@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES

  # Directories & files removed with 'make clean'
  CLEAN_DIRS  += $(MODVERDIR) include/ksym
+CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
+           tools/objtool/arch/$(ARCH)/lib/inat-tables.c

  # Directories & files removed with 'make mrproper'
  MRPROPER_DIRS  += include/config usr/include include/generated          \

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-02-14 21:08 [PATCH] objtool: remove generated files with make clean Robin Meijboom
@ 2019-02-28 13:36 ` Masahiro Yamada
  2019-02-28 20:12   ` Robin Meijboom
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2019-02-28 13:36 UTC (permalink / raw)
  To: Robin Meijboom
  Cc: Michal Marek, Linux Kbuild mailing list, Josh Poimboeuf, Peter Zijlstra

On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
>
> Make clean currently does not remove the generated files for objtool:
> tools/objtool/objtool, tools/objtool/fixdep, and
> tools/objtool/arch/x86/lib/inat-tables.c.
>
> Clean these files up as part of make clean.
>
> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
>
> Signed-off-by: Robin Meijboom <robin@meijboom.info>
> ---
> In the discussions I didn't find a reason for keeping the files, so I
> assume it is an oversight. Otherwise I would have expected them to be
> removed at least by make distconfig (which they are not).
>
> Tested by compiling, cleaning, compiling again, and booting on x86_64.
>
> diff --git a/Makefile b/Makefile
> index 141653226f3c..81a8149a805f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
>
>   # Directories & files removed with 'make clean'
>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
>
>   # Directories & files removed with 'make mrproper'
>   MRPROPER_DIRS  += include/config usr/include include/generated          \
>



I see the same artifacts are cleaned up by tools/objtool/Makefile:

https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59


So, this patch proves the 'clean' target
in tools/objtool/Makefile is useless.


BTW, 'make clean' must keep all the generated files
that are needed to compile external modules.

I guess cleaning objtool is wrong.



--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-02-28 13:36 ` Masahiro Yamada
@ 2019-02-28 20:12   ` Robin Meijboom
  2019-02-28 22:29     ` Josh Poimboeuf
  0 siblings, 1 reply; 8+ messages in thread
From: Robin Meijboom @ 2019-02-28 20:12 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Michal Marek, Linux Kbuild mailing list, Josh Poimboeuf, Peter Zijlstra

Hi Masahiro,

On 28-02-19 14:36, Masahiro Yamada wrote:
> On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
>>
>> Make clean currently does not remove the generated files for objtool:
>> tools/objtool/objtool, tools/objtool/fixdep, and
>> tools/objtool/arch/x86/lib/inat-tables.c.
>>
>> Clean these files up as part of make clean.
>>
>> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
>> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
>>
>> Signed-off-by: Robin Meijboom <robin@meijboom.info>
>> ---
>> In the discussions I didn't find a reason for keeping the files, so I
>> assume it is an oversight. Otherwise I would have expected them to be
>> removed at least by make distconfig (which they are not).
>>
>> Tested by compiling, cleaning, compiling again, and booting on x86_64.
>>
>> diff --git a/Makefile b/Makefile
>> index 141653226f3c..81a8149a805f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
>>
>>   # Directories & files removed with 'make clean'
>>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
>> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
>> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
>>
>>   # Directories & files removed with 'make mrproper'
>>   MRPROPER_DIRS  += include/config usr/include include/generated          \
>>
> 
> 
> 
> I see the same artifacts are cleaned up by tools/objtool/Makefile:
> 
> https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59
> 
> 
> So, this patch proves the 'clean' target
> in tools/objtool/Makefile is useless.
> 

True. I believe this is because kbuild does not descend into that directory
as it does with the other directories. It works if you issue 'make clean'
from tools or tools/objtool.

> 
> BTW, 'make clean' must keep all the generated files
> that are needed to compile external modules.
> 
> I guess cleaning objtool is wrong.

I was considering the same. However, I did not find this in the discussions,
and as you pointed out the authors have included these items under the
'make clean' target in tools/objtool/Makefile (which does not work for the
above reason).

Additionally, if cleaning objtool is wrong, I would expect it to be at
least included in 'make distclean' or 'make mrproper' target (which it is
not).

Josh, feel free to comment on the above.

Kind regards,
Robin Meijboom

> 
> 
> 
> --
> Best Regards
> Masahiro Yamada
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-02-28 20:12   ` Robin Meijboom
@ 2019-02-28 22:29     ` Josh Poimboeuf
  2019-03-04 22:43       ` Robin Meijboom
  2019-03-05  5:52       ` Masahiro Yamada
  0 siblings, 2 replies; 8+ messages in thread
From: Josh Poimboeuf @ 2019-02-28 22:29 UTC (permalink / raw)
  To: Robin Meijboom
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Peter Zijlstra

On Thu, Feb 28, 2019 at 09:12:19PM +0100, Robin Meijboom wrote:
> Hi Masahiro,
> 
> On 28-02-19 14:36, Masahiro Yamada wrote:
> > On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
> >>
> >> Make clean currently does not remove the generated files for objtool:
> >> tools/objtool/objtool, tools/objtool/fixdep, and
> >> tools/objtool/arch/x86/lib/inat-tables.c.
> >>
> >> Clean these files up as part of make clean.
> >>
> >> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
> >> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
> >>
> >> Signed-off-by: Robin Meijboom <robin@meijboom.info>
> >> ---
> >> In the discussions I didn't find a reason for keeping the files, so I
> >> assume it is an oversight. Otherwise I would have expected them to be
> >> removed at least by make distconfig (which they are not).
> >>
> >> Tested by compiling, cleaning, compiling again, and booting on x86_64.
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 141653226f3c..81a8149a805f 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
> >>
> >>   # Directories & files removed with 'make clean'
> >>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
> >> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
> >> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
> >>
> >>   # Directories & files removed with 'make mrproper'
> >>   MRPROPER_DIRS  += include/config usr/include include/generated          \
> >>
> > 
> > 
> > 
> > I see the same artifacts are cleaned up by tools/objtool/Makefile:
> > 
> > https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59
> > 
> > 
> > So, this patch proves the 'clean' target
> > in tools/objtool/Makefile is useless.
> > 
> 
> True. I believe this is because kbuild does not descend into that directory
> as it does with the other directories. It works if you issue 'make clean'
> from tools or tools/objtool.

Right.  Objtool is intended to be a standalone tool, though it's
currently a bit kernel-specific.  So a clean target within tools/objtool
makes sense I think.

> > BTW, 'make clean' must keep all the generated files
> > that are needed to compile external modules.
> > 
> > I guess cleaning objtool is wrong.
> 
> I was considering the same. However, I did not find this in the discussions,
> and as you pointed out the authors have included these items under the
> 'make clean' target in tools/objtool/Makefile (which does not work for the
> above reason).
> 
> Additionally, if cleaning objtool is wrong, I would expect it to be at
> least included in 'make distclean' or 'make mrproper' target (which it is
> not).
> 
> Josh, feel free to comment on the above.

Objtool is needed for module builds, so it should *not* be removed with
'make clean' from the top-level directory.

I guess it makes sense to remove it for mrproper (and by extension,
distclean).  Maybe mrproper could use tools/objtool's clean target
somehow.

-- 
Josh

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-02-28 22:29     ` Josh Poimboeuf
@ 2019-03-04 22:43       ` Robin Meijboom
  2019-03-05  2:05         ` Josh Poimboeuf
  2019-03-05  5:56         ` Masahiro Yamada
  2019-03-05  5:52       ` Masahiro Yamada
  1 sibling, 2 replies; 8+ messages in thread
From: Robin Meijboom @ 2019-03-04 22:43 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Peter Zijlstra

On 28-02-19 23:29, Josh Poimboeuf wrote:
> On Thu, Feb 28, 2019 at 09:12:19PM +0100, Robin Meijboom wrote:
>> Hi Masahiro,
>>
>> On 28-02-19 14:36, Masahiro Yamada wrote:
>>> On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
>>>>
>>>> Make clean currently does not remove the generated files for objtool:
>>>> tools/objtool/objtool, tools/objtool/fixdep, and
>>>> tools/objtool/arch/x86/lib/inat-tables.c.
>>>>
>>>> Clean these files up as part of make clean.
>>>>
>>>> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
>>>> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
>>>>
>>>> Signed-off-by: Robin Meijboom <robin@meijboom.info>
>>>> ---
>>>> In the discussions I didn't find a reason for keeping the files, so I
>>>> assume it is an oversight. Otherwise I would have expected them to be
>>>> removed at least by make distconfig (which they are not).
>>>>
>>>> Tested by compiling, cleaning, compiling again, and booting on x86_64.
>>>>
>>>> diff --git a/Makefile b/Makefile
>>>> index 141653226f3c..81a8149a805f 100644
>>>> --- a/Makefile
>>>> +++ b/Makefile
>>>> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
>>>>
>>>>   # Directories & files removed with 'make clean'
>>>>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
>>>> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
>>>> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
>>>>
>>>>   # Directories & files removed with 'make mrproper'
>>>>   MRPROPER_DIRS  += include/config usr/include include/generated          \
>>>>
>>>
>>>
>>>

Hi Josh,

Thanks for your feedback.

>>> I see the same artifacts are cleaned up by tools/objtool/Makefile:
>>>
>>> https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59
>>>
>>>
>>> So, this patch proves the 'clean' target
>>> in tools/objtool/Makefile is useless.
>>>
>>
>> True. I believe this is because kbuild does not descend into that directory
>> as it does with the other directories. It works if you issue 'make clean'
>> from tools or tools/objtool.
> 
> Right.  Objtool is intended to be a standalone tool, though it's
> currently a bit kernel-specific.  So a clean target within tools/objtool
> makes sense I think.
> 

Clear. Currently objtool is also cleaned by the 'clean' target in
tools/Makefile. Would the above suggest that should be removed, or is it
fine as long as it is not in the top-level Makefile?

>>> BTW, 'make clean' must keep all the generated files
>>> that are needed to compile external modules.
>>>
>>> I guess cleaning objtool is wrong.
>>
>> I was considering the same. However, I did not find this in the discussions,
>> and as you pointed out the authors have included these items under the
>> 'make clean' target in tools/objtool/Makefile (which does not work for the
>> above reason).
>>
>> Additionally, if cleaning objtool is wrong, I would expect it to be at
>> least included in 'make distclean' or 'make mrproper' target (which it is
>> not).
>>
>> Josh, feel free to comment on the above.
> 
> Objtool is needed for module builds, so it should *not* be removed with
> 'make clean' from the top-level directory.
> 
> I guess it makes sense to remove it for mrproper (and by extension,
> distclean).  Maybe mrproper could use tools/objtool's clean target
> somehow.
> 

Since we have to use an exception anyway, my initial thought was to
use the 'standard exception' and include the artifacts in
MRPROPER_FILES. However, your idea is of course to _not_ have to
update the top-level Makefile every time you make a change to objtool
artifacts. I'm thinking about an elegant solution.

--
Kind regards,
Robin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-03-04 22:43       ` Robin Meijboom
@ 2019-03-05  2:05         ` Josh Poimboeuf
  2019-03-05  5:56         ` Masahiro Yamada
  1 sibling, 0 replies; 8+ messages in thread
From: Josh Poimboeuf @ 2019-03-05  2:05 UTC (permalink / raw)
  To: Robin Meijboom
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list, Peter Zijlstra

On Mon, Mar 04, 2019 at 11:43:13PM +0100, Robin Meijboom wrote:
> On 28-02-19 23:29, Josh Poimboeuf wrote:
> > On Thu, Feb 28, 2019 at 09:12:19PM +0100, Robin Meijboom wrote:
> >> Hi Masahiro,
> >>
> >> On 28-02-19 14:36, Masahiro Yamada wrote:
> >>> On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
> >>>>
> >>>> Make clean currently does not remove the generated files for objtool:
> >>>> tools/objtool/objtool, tools/objtool/fixdep, and
> >>>> tools/objtool/arch/x86/lib/inat-tables.c.
> >>>>
> >>>> Clean these files up as part of make clean.
> >>>>
> >>>> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
> >>>> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
> >>>>
> >>>> Signed-off-by: Robin Meijboom <robin@meijboom.info>
> >>>> ---
> >>>> In the discussions I didn't find a reason for keeping the files, so I
> >>>> assume it is an oversight. Otherwise I would have expected them to be
> >>>> removed at least by make distconfig (which they are not).
> >>>>
> >>>> Tested by compiling, cleaning, compiling again, and booting on x86_64.
> >>>>
> >>>> diff --git a/Makefile b/Makefile
> >>>> index 141653226f3c..81a8149a805f 100644
> >>>> --- a/Makefile
> >>>> +++ b/Makefile
> >>>> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
> >>>>
> >>>>   # Directories & files removed with 'make clean'
> >>>>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
> >>>> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
> >>>> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
> >>>>
> >>>>   # Directories & files removed with 'make mrproper'
> >>>>   MRPROPER_DIRS  += include/config usr/include include/generated          \
> >>>>
> >>>
> >>>
> >>>
> 
> Hi Josh,
> 
> Thanks for your feedback.
> 
> >>> I see the same artifacts are cleaned up by tools/objtool/Makefile:
> >>>
> >>> https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59
> >>>
> >>>
> >>> So, this patch proves the 'clean' target
> >>> in tools/objtool/Makefile is useless.
> >>>
> >>
> >> True. I believe this is because kbuild does not descend into that directory
> >> as it does with the other directories. It works if you issue 'make clean'
> >> from tools or tools/objtool.
> > 
> > Right.  Objtool is intended to be a standalone tool, though it's
> > currently a bit kernel-specific.  So a clean target within tools/objtool
> > makes sense I think.
> > 
> 
> Clear. Currently objtool is also cleaned by the 'clean' target in
> tools/Makefile. Would the above suggest that should be removed, or is it
> fine as long as it is not in the top-level Makefile?

The top-level makefile uses the tools/Makefile to build objtool via the
tools/objtool target, which then descends to the objtool Makefile.  So
IMO, for consistency, the objtool_clean target should remain in
tools/Makefile as well, so the top-level Makefile can use that target to
clean objtool.

> >>> BTW, 'make clean' must keep all the generated files
> >>> that are needed to compile external modules.
> >>>
> >>> I guess cleaning objtool is wrong.
> >>
> >> I was considering the same. However, I did not find this in the discussions,
> >> and as you pointed out the authors have included these items under the
> >> 'make clean' target in tools/objtool/Makefile (which does not work for the
> >> above reason).
> >>
> >> Additionally, if cleaning objtool is wrong, I would expect it to be at
> >> least included in 'make distclean' or 'make mrproper' target (which it is
> >> not).
> >>
> >> Josh, feel free to comment on the above.
> > 
> > Objtool is needed for module builds, so it should *not* be removed with
> > 'make clean' from the top-level directory.
> > 
> > I guess it makes sense to remove it for mrproper (and by extension,
> > distclean).  Maybe mrproper could use tools/objtool's clean target
> > somehow.
> > 
> 
> Since we have to use an exception anyway, my initial thought was to
> use the 'standard exception' and include the artifacts in
> MRPROPER_FILES. However, your idea is of course to _not_ have to
> update the top-level Makefile every time you make a change to objtool
> artifacts. I'm thinking about an elegant solution.

How about this?  Seems to work for me.

diff --git a/Makefile b/Makefile
index ac5ac28a24e9..7e6696c9b862 100644
--- a/Makefile
+++ b/Makefile
@@ -1364,7 +1364,7 @@ PHONY += $(mrproper-dirs) mrproper
 $(mrproper-dirs):
 	$(Q)$(MAKE) $(clean)=$(patsubst _mrproper_%,%,$@)
 
-mrproper: clean $(mrproper-dirs)
+mrproper: clean $(mrproper-dirs) tools/objtool_clean
 	$(call cmd,rmdirs)
 	$(call cmd,rmfiles)
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-02-28 22:29     ` Josh Poimboeuf
  2019-03-04 22:43       ` Robin Meijboom
@ 2019-03-05  5:52       ` Masahiro Yamada
  1 sibling, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2019-03-05  5:52 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Robin Meijboom, Michal Marek, Linux Kbuild mailing list, Peter Zijlstra

On Fri, Mar 1, 2019 at 7:29 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Thu, Feb 28, 2019 at 09:12:19PM +0100, Robin Meijboom wrote:
> > Hi Masahiro,
> >
> > On 28-02-19 14:36, Masahiro Yamada wrote:
> > > On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
> > >>
> > >> Make clean currently does not remove the generated files for objtool:
> > >> tools/objtool/objtool, tools/objtool/fixdep, and
> > >> tools/objtool/arch/x86/lib/inat-tables.c.
> > >>
> > >> Clean these files up as part of make clean.
> > >>
> > >> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
> > >> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
> > >>
> > >> Signed-off-by: Robin Meijboom <robin@meijboom.info>
> > >> ---
> > >> In the discussions I didn't find a reason for keeping the files, so I
> > >> assume it is an oversight. Otherwise I would have expected them to be
> > >> removed at least by make distconfig (which they are not).
> > >>
> > >> Tested by compiling, cleaning, compiling again, and booting on x86_64.
> > >>
> > >> diff --git a/Makefile b/Makefile
> > >> index 141653226f3c..81a8149a805f 100644
> > >> --- a/Makefile
> > >> +++ b/Makefile
> > >> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
> > >>
> > >>   # Directories & files removed with 'make clean'
> > >>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
> > >> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
> > >> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
> > >>
> > >>   # Directories & files removed with 'make mrproper'
> > >>   MRPROPER_DIRS  += include/config usr/include include/generated          \
> > >>
> > >
> > >
> > >
> > > I see the same artifacts are cleaned up by tools/objtool/Makefile:
> > >
> > > https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59
> > >
> > >
> > > So, this patch proves the 'clean' target
> > > in tools/objtool/Makefile is useless.
> > >
> >
> > True. I believe this is because kbuild does not descend into that directory
> > as it does with the other directories. It works if you issue 'make clean'
> > from tools or tools/objtool.
>
> Right.  Objtool is intended to be a standalone tool, though it's
> currently a bit kernel-specific.  So a clean target within tools/objtool
> makes sense I think.


Yeah, I remember you mentioned this before.
https://patchwork.kernel.org/patch/9983535/#20996149

Do you mean objtool will be split out from the kernel tree
in the future?

If objtool is useful for other projects in general
(like Sparse), it would be good,
and the top-level Makefile of kernel will add

export OBJTOOL  = /usr/bin/objtool

in case the user may want to install objtool
in a different path.





-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] objtool: remove generated files with make clean
  2019-03-04 22:43       ` Robin Meijboom
  2019-03-05  2:05         ` Josh Poimboeuf
@ 2019-03-05  5:56         ` Masahiro Yamada
  1 sibling, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2019-03-05  5:56 UTC (permalink / raw)
  To: Robin Meijboom
  Cc: Josh Poimboeuf, Michal Marek, Linux Kbuild mailing list, Peter Zijlstra

On Tue, Mar 5, 2019 at 7:43 AM Robin Meijboom <robin@meijboom.info> wrote:
>
> On 28-02-19 23:29, Josh Poimboeuf wrote:
> > On Thu, Feb 28, 2019 at 09:12:19PM +0100, Robin Meijboom wrote:
> >> Hi Masahiro,
> >>
> >> On 28-02-19 14:36, Masahiro Yamada wrote:
> >>> On Fri, Feb 15, 2019 at 11:15 AM Robin Meijboom <robin@meijboom.info> wrote:
> >>>>
> >>>> Make clean currently does not remove the generated files for objtool:
> >>>> tools/objtool/objtool, tools/objtool/fixdep, and
> >>>> tools/objtool/arch/x86/lib/inat-tables.c.
> >>>>
> >>>> Clean these files up as part of make clean.
> >>>>
> >>>> Fixes: b9ab5ebb14ec ("objtool: Add CONFIG_STACK_VALIDATION option") and
> >>>> bug report 199485 (https://bugzilla.kernel.org/show_bug.cgi?id=199485).
> >>>>
> >>>> Signed-off-by: Robin Meijboom <robin@meijboom.info>
> >>>> ---
> >>>> In the discussions I didn't find a reason for keeping the files, so I
> >>>> assume it is an oversight. Otherwise I would have expected them to be
> >>>> removed at least by make distconfig (which they are not).
> >>>>
> >>>> Tested by compiling, cleaning, compiling again, and booting on x86_64.
> >>>>
> >>>> diff --git a/Makefile b/Makefile
> >>>> index 141653226f3c..81a8149a805f 100644
> >>>> --- a/Makefile
> >>>> +++ b/Makefile
> >>>> @@ -1328,6 +1328,8 @@ endif # CONFIG_MODULES
> >>>>
> >>>>   # Directories & files removed with 'make clean'
> >>>>   CLEAN_DIRS  += $(MODVERDIR) include/ksym
> >>>> +CLEAN_FILES += tools/objtool/objtool tools/objtool/fixdep \
> >>>> +           tools/objtool/arch/$(ARCH)/lib/inat-tables.c
> >>>>
> >>>>   # Directories & files removed with 'make mrproper'
> >>>>   MRPROPER_DIRS  += include/config usr/include include/generated          \
> >>>>
> >>>
> >>>
> >>>
>
> Hi Josh,
>
> Thanks for your feedback.
>
> >>> I see the same artifacts are cleaned up by tools/objtool/Makefile:
> >>>
> >>> https://github.com/torvalds/linux/blob/v5.0-rc7/tools/objtool/Makefile#L59
> >>>
> >>>
> >>> So, this patch proves the 'clean' target
> >>> in tools/objtool/Makefile is useless.
> >>>
> >>
> >> True. I believe this is because kbuild does not descend into that directory
> >> as it does with the other directories. It works if you issue 'make clean'
> >> from tools or tools/objtool.
> >
> > Right.  Objtool is intended to be a standalone tool, though it's
> > currently a bit kernel-specific.  So a clean target within tools/objtool
> > makes sense I think.
> >
>
> Clear. Currently objtool is also cleaned by the 'clean' target in
> tools/Makefile. Would the above suggest that should be removed, or is it
> fine as long as it is not in the top-level Makefile?
>
> >>> BTW, 'make clean' must keep all the generated files
> >>> that are needed to compile external modules.
> >>>
> >>> I guess cleaning objtool is wrong.
> >>
> >> I was considering the same. However, I did not find this in the discussions,
> >> and as you pointed out the authors have included these items under the
> >> 'make clean' target in tools/objtool/Makefile (which does not work for the
> >> above reason).
> >>
> >> Additionally, if cleaning objtool is wrong, I would expect it to be at
> >> least included in 'make distclean' or 'make mrproper' target (which it is
> >> not).
> >>
> >> Josh, feel free to comment on the above.
> >
> > Objtool is needed for module builds, so it should *not* be removed with
> > 'make clean' from the top-level directory.
> >
> > I guess it makes sense to remove it for mrproper (and by extension,
> > distclean).  Maybe mrproper could use tools/objtool's clean target
> > somehow.
> >


I agree that cleaning the objtool's artifacts by 'make mrproper'
is the right thing to do.

But, Kbuild does not care under tools/
since we expect standalone tools there,
and the tools build system is completely different stuff, sadly.



> Since we have to use an exception anyway, my initial thought was to
> use the 'standard exception' and include the artifacts in
> MRPROPER_FILES. However, your idea is of course to _not_ have to
> update the top-level Makefile every time you make a change to objtool
> artifacts. I'm thinking about an elegant solution.


The most elegant solution would be
to move tools/objtool to scripts/objtool.

I just posted the series.

https://lore.kernel.org/patchwork/patch/1047807/
https://lore.kernel.org/patchwork/patch/1047808/
https://lore.kernel.org/patchwork/patch/1047809/




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-03-05  5:58 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 21:08 [PATCH] objtool: remove generated files with make clean Robin Meijboom
2019-02-28 13:36 ` Masahiro Yamada
2019-02-28 20:12   ` Robin Meijboom
2019-02-28 22:29     ` Josh Poimboeuf
2019-03-04 22:43       ` Robin Meijboom
2019-03-05  2:05         ` Josh Poimboeuf
2019-03-05  5:56         ` Masahiro Yamada
2019-03-05  5:52       ` Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.