lkml.org 
[lkml]   [2018]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v3 05/12] kconfig: make syncconfig update .config regardless of sym_change_count
    Date
    Masahiro Yamada <yamada.masahiro@socionext.com> writes:

    > 2018-07-12 20:32 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
    >> Masahiro Yamada <yamada.masahiro@socionext.com> writes:
    >>
    >>> 2018-07-09 20:39 GMT+09:00 Dirk Gouders <dirk@gouders.net>:
    >>>> Dirk Gouders <dirk@gouders.net> writes:
    >>>>
    >>>> I think, I solved the puzzle and perhaps, that saves others some time:
    >>>>
    >>>> The problem is that "if_changed" was not designed for multiple use
    >>>> inside a recipe and in the case of compressed/vmlinux, the 2-fold use
    >>>> created a kind of flip-flop for situations when nothing has to be done
    >>>> to build the target.
    >>>>
    >>>> Because each of the two users of "if_changed" stores it's footprint in
    >>>> .vmlinux.cmd but that file then isn't re-read, one of the two
    >>>> "if_changed" calculates that nothing has to be done wheras the other one
    >>>> recognizes a change in the commandline, because it sees the command-line
    >>>> for the other part of the reciepe.
    >>>>
    >>>> In the next make, the roles flip, because the previously satisfied
    >>>> "if_changed" now sees the command-line of the other one. And so on...
    >>>>
    >>>> I am not a Kbuild expert but the attached patch fixes that problem by
    >>>> introducing "if_changed_multi" that accepts two commands -- one whose
    >>>> commandline should be checked and a second one that should be
    >>>> executed.
    >>>
    >>>
    >>> if_changed should not appear multiple times in one target.
    >>>
    >>> I think the simplest fix-up is to
    >>> create a new command that combines
    >>> 'cmd_check_data_rel' and 'cmd_ld'.
    >>>
    >>>
    >>> quiet_cmd_link-vmlinux = LD $@
    >>> cmd_link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
    >>>
    >>> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
    >>> $(call if_changed,link-vmlinux)
    >>>
    >>> Kbuild also supports if_changed_rule,
    >>> but the usage is more complex.
    >>>
    >>> There are only a few usages:
    >>> https://github.com/torvalds/linux/blob/v4.17/scripts/Makefile.build#L288
    >>
    >> Just for completeness I will copy in part of a reply from Kees that
    >> shows how double-colon rules can also avoid multiple use of if_changed
    >> for one target:
    >>
    >> -$(obj)/vmlinux: $(vmlinux-objs-y) FORCE
    >> - $(call if_changed,check_data_rel)
    >> +$(obj)/vmlinux:: $(vmlinux-objs-y)
    >> + $(call cmd,check_data_rel)
    >> +$(obj)/vmlinux:: $(vmlinux-objs-y) FORCE
    >> $(call if_changed,ld)
    >
    > It is difficult to use double-colon rules in a _sane_ way.
    >
    > The first one just checks data_rel,
    > but does not actually generate anything.
    >
    > Such targets should be marked as .PHONY,
    > but $(obj)/vmlinux is not a phony target.
    > This is strange.
    >
    >> The combined command seems to have the advantage that every command to
    >> build the target gets recorded in the .cmd file
    >>
    >> A search showed me that we have two more users that use if_changed more
    >> than once for a single target:
    >>
    >> arch/microblaze/boot/Makefile (fourfold)
    >> arch/sparc/boot/Makefile (2 times twofold)
    >>
    >> The sparc case seems to apply to any of the two suggested fixes,
    >
    > Neither is correct.
    >
    >
    > $(obj)/uImage: $(obj)/image.gz
    > $(call if_changed,uimage)
    > $(call if_changed,uimage.o)
    >
    >
    > should be split into two targets.
    >
    >
    > $(obj)/uImage: $(obj)/image.gz FORCE
    > $(call if_changed,uimage)
    >
    > $(obj)/uImage.o: $(obj)/uImage FORCE
    > $(call if_changed,uimage.o)
    >
    >
    > It is wrong in multiple ways. FORCE is missing too.
    >
    > but
    >> microblaze uses if_changed in a pattern rule and also makes use of
    >> parameter arguments in the sub-commands:
    >>
    >> $(obj)/simpleImage.%: vmlinux FORCE
    >> $(call if_changed,cp,.unstrip)
    >> $(call if_changed,objcopy)
    >> $(call if_changed,uimage)
    >> $(call if_changed,strip,.strip)
    >> @echo 'Kernel: $(UIMAGE_OUT) is ready' ' (#'`cat .version`')'
    >
    >
    > Probably, this is the same.
    >
    > Create a target for each step.
    >
    >
    >> In this case, double colons would have a different meaning and the
    >> combined command solution would result in a change of the sub-commands,
    >> as well. I note this in case Michal perhaps has other preferences.
    >>
    >>
    >> In addition to extend the documentation, we could modify if_changed to
    >> warn about it is being used more than once for a target:
    >>
    >> # Execute command if command has changed or prerequisite(s) are updated.
    >> if_changed = $(if $(filter-out undefined,$(origin if_changed_$@)), \
    >> @set -e; \
    >> echo "Warning: $@: multiple use of if_changed!" >&2; , \
    >> @set -e $(eval if_changed_$@ := 1) ; ) \
    >> $(if $(strip $(any-prereq) $(arg-check)), \
    >> $(echo-cmd) $(cmd_$(1)); \
    >> printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, :)
    >>
    >> But this fires only if if_changed is actually called and it defines many
    >> variables for just that purpose, so this is perhaps not what we want...
    >>
    >
    > I do not want to mess up Makefile.
    >
    >
    > Please do this check in scripts/checkpatch.pl
    > if you want.

    Thank you for spending your time in the detailed explanation.
    I will use it to assemble that all to a fix and then send it for review.

    Dirk
    \
     
     \ /
      Last update: 2018-07-15 22:07    [W:3.317 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site