Message ID | bca6deb0-ba70-12af-9381-72ae7d94c0c5@meijboom.info (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | objtool: remove generated files with make clean | expand |
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
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 >
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.
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
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)
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.
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/
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 \
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.