Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1350664 > unrolled thread

Re: [PATCH v5 5/8] kbuild: add fine grained build dependencies for exported symbols

Started byMichal Marek <mmarek@suse.cz>
First post2016-03-05 00:00 +0100
Last post2016-03-06 07:30 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v5 5/8] kbuild: add fine grained build dependencies for  exported symbols Michal Marek <mmarek@suse.cz> - 2016-03-05 00:00 +0100
    Re: [PATCH v5 5/8] kbuild: add fine grained build dependencies for  exported symbols Michal Marek <mmarek@suse.cz> - 2016-03-05 08:20 +0100
      Re: [PATCH v5 5/8] kbuild: add fine grained build dependencies for  exported symbols Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-03-06 03:40 +0100
        Re: [PATCH v5 5/8] kbuild: add fine grained build dependencies for  exported symbols Nicolas Pitre <nicolas.pitre@linaro.org> - 2016-03-06 07:30 +0100

#1350664 — Re: [PATCH v5 5/8] kbuild: add fine grained build dependencies for exported symbols

FromMichal Marek <mmarek@suse.cz>
Date2016-03-05 00:00 +0100
SubjectRe: [PATCH v5 5/8] kbuild: add fine grained build dependencies for exported symbols
Message-ID<r96Rs-5oV-17@gated-at.bofh.it>
Dne 4.3.2016 v 23:51 Michal Marek napsal(a):
> Dne 4.3.2016 v 06:40 Nicolas Pitre napsal(a):
>> +cmd_and_fixdep =                                                             \
>> +	$(echo-cmd) $(cmd_$(1));                                             \
>> +	$(ksym_dep_filter) |                                                 \
>> +		scripts/basic/fixdep -e $(depfile) $@ '$(make-cmd)'          \
>> +			> $(dot-target).tmp;	                             \
>> +	rm -f $(depfile);                                                    \
>> +	mv -f $(dot-target).tmp $(dot-target).cmd;
> 
> While trying this, I got a SIGBUS from fixdep once. My theory is that
> the depfile is mmap()ed by fixdep and modified by the preprocesor run at
> the same time. I could not reproduce this so far (still trying). But if
> it's really this race, the fix would be to disable dependency generation
> in the preprocessor by passing -Wp,MD,/dev/null or somesuch. But we
> never had this problem with genksyms, which is weird. It could as well
> be that my build machine's memory is faulty :(.

Actually, genksyms does not ran in parallel. neither before nor after
this patch.

Michal

[toc] | [next] | [standalone]


#1350812

FromMichal Marek <mmarek@suse.cz>
Date2016-03-05 08:20 +0100
Message-ID<r9eFj-2Gv-3@gated-at.bofh.it>
In reply to#1350664
On Fri, Mar 04, 2016 at 11:53:53PM +0100, Michal Marek wrote:
> Dne 4.3.2016 v 23:51 Michal Marek napsal(a):
> > Dne 4.3.2016 v 06:40 Nicolas Pitre napsal(a):
> >> +cmd_and_fixdep =                                                             \
> >> +	$(echo-cmd) $(cmd_$(1));                                             \
> >> +	$(ksym_dep_filter) |                                                 \
> >> +		scripts/basic/fixdep -e $(depfile) $@ '$(make-cmd)'          \
> >> +			> $(dot-target).tmp;	                             \
> >> +	rm -f $(depfile);                                                    \
> >> +	mv -f $(dot-target).tmp $(dot-target).cmd;
> > 
> > While trying this, I got a SIGBUS from fixdep once. My theory is that
> > the depfile is mmap()ed by fixdep and modified by the preprocesor run at
> > the same time. I could not reproduce this so far (still trying). But if
> > it's really this race, the fix would be to disable dependency generation
> > in the preprocessor by passing -Wp,MD,/dev/null or somesuch. But we
> > never had this problem with genksyms, which is weird. It could as well
> > be that my build machine's memory is faulty :(.
> 
> Actually, genksyms does not ran in parallel. neither before nor after
                              run
> this patch.

I reproduced the SIGBUS after a few iterations, and it crashes in
parse_dep_file(). I'm now testing this

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 0be7e09ba381..4fdd8348acf6 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -270,10 +270,12 @@ else
 
 # Filter out exported kernel symbol names from the preprocessor output.
 # See also __KSYM_DEPS__ in include/linux/export.h.
+# We disable the depfile generation here, so as not to overwrite the existing
+# depfile while fixdep is parsing it
 ksym_dep_filter =                                                            \
 	case "$(1)" in                                                       \
-	cc_*_c) $(CPP) $(c_flags) -D__KSYM_DEPS__ $< ;;                      \
-	as_*_S) $(CPP) $(a_flags) -D__KSYM_DEPS__ $< ;;                      \
+	cc_*_c) $(CPP) $(filter-out -Wp$(comma)-M%, $(c_flags)) -D__KSYM_DEPS__ $< ;; \
+	as_*_S) $(CPP) $(filter-out -Wp$(comma)-M%, $(a_flags)) -D__KSYM_DEPS__ $< ;; \
 	cpp_lds_S) : ;;                                                      \
 	*) echo "Don't know how to preprocess $(1)"; false ;;                \
 	esac | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'

Michal

[toc] | [prev] | [next] | [standalone]


#1351030

FromNicolas Pitre <nicolas.pitre@linaro.org>
Date2016-03-06 03:40 +0100
Message-ID<r9wLU-73U-5@gated-at.bofh.it>
In reply to#1350812
On Sat, 5 Mar 2016, Michal Marek wrote:

> On Fri, Mar 04, 2016 at 11:53:53PM +0100, Michal Marek wrote:
> > Dne 4.3.2016 v 23:51 Michal Marek napsal(a):
> > > Dne 4.3.2016 v 06:40 Nicolas Pitre napsal(a):
> > >> +cmd_and_fixdep =                                                             \
> > >> +	$(echo-cmd) $(cmd_$(1));                                             \
> > >> +	$(ksym_dep_filter) |                                                 \
> > >> +		scripts/basic/fixdep -e $(depfile) $@ '$(make-cmd)'          \
> > >> +			> $(dot-target).tmp;	                             \
> > >> +	rm -f $(depfile);                                                    \
> > >> +	mv -f $(dot-target).tmp $(dot-target).cmd;
> > > 
> > > While trying this, I got a SIGBUS from fixdep once. My theory is that
> > > the depfile is mmap()ed by fixdep and modified by the preprocesor run at
> > > the same time. I could not reproduce this so far (still trying). But if
> > > it's really this race, the fix would be to disable dependency generation
> > > in the preprocessor by passing -Wp,MD,/dev/null or somesuch. But we
> > > never had this problem with genksyms, which is weird. It could as well
> > > be that my build machine's memory is faulty :(.
> > 
> > Actually, genksyms does not ran in parallel. neither before nor after
>                               run
> > this patch.
> 
> I reproduced the SIGBUS after a few iterations, and it crashes in
> parse_dep_file(). I'm now testing this
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 0be7e09ba381..4fdd8348acf6 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -270,10 +270,12 @@ else
>  
>  # Filter out exported kernel symbol names from the preprocessor output.
>  # See also __KSYM_DEPS__ in include/linux/export.h.
> +# We disable the depfile generation here, so as not to overwrite the existing
> +# depfile while fixdep is parsing it
>  ksym_dep_filter =                                                            \
>  	case "$(1)" in                                                       \
> -	cc_*_c) $(CPP) $(c_flags) -D__KSYM_DEPS__ $< ;;                      \
> -	as_*_S) $(CPP) $(a_flags) -D__KSYM_DEPS__ $< ;;                      \
> +	cc_*_c) $(CPP) $(filter-out -Wp$(comma)-M%, $(c_flags)) -D__KSYM_DEPS__ $< ;; \
> +	as_*_S) $(CPP) $(filter-out -Wp$(comma)-M%, $(a_flags)) -D__KSYM_DEPS__ $< ;; \
>  	cpp_lds_S) : ;;                                                      \
>  	*) echo "Don't know how to preprocess $(1)"; false ;;                \
>  	esac | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'

This makes perfect sense even if I can't reproduce on my side.

Are you willing to fold this patch in, or do you prefer that I repost?


Nicolas

[toc] | [prev] | [next] | [standalone]


#1351056

FromNicolas Pitre <nicolas.pitre@linaro.org>
Date2016-03-06 07:30 +0100
Message-ID<r9Amt-1hT-7@gated-at.bofh.it>
In reply to#1351030
On Sat, 5 Mar 2016, Nicolas Pitre wrote:

> On Sat, 5 Mar 2016, Michal Marek wrote:
> 
> > I reproduced the SIGBUS after a few iterations, and it crashes in
> > parse_dep_file(). I'm now testing this
> > 
> > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> > index 0be7e09ba381..4fdd8348acf6 100644
> > --- a/scripts/Kbuild.include
> > +++ b/scripts/Kbuild.include
> > @@ -270,10 +270,12 @@ else
> >  
> >  # Filter out exported kernel symbol names from the preprocessor output.
> >  # See also __KSYM_DEPS__ in include/linux/export.h.
> > +# We disable the depfile generation here, so as not to overwrite the existing
> > +# depfile while fixdep is parsing it
> >  ksym_dep_filter =                                                            \
> >  	case "$(1)" in                                                       \
> > -	cc_*_c) $(CPP) $(c_flags) -D__KSYM_DEPS__ $< ;;                      \
> > -	as_*_S) $(CPP) $(a_flags) -D__KSYM_DEPS__ $< ;;                      \
> > +	cc_*_c) $(CPP) $(filter-out -Wp$(comma)-M%, $(c_flags)) -D__KSYM_DEPS__ $< ;; \
> > +	as_*_S) $(CPP) $(filter-out -Wp$(comma)-M%, $(a_flags)) -D__KSYM_DEPS__ $< ;; \
> >  	cpp_lds_S) : ;;                                                      \
> >  	*) echo "Don't know how to preprocess $(1)"; false ;;                \
> >  	esac | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
> 
> This makes perfect sense even if I can't reproduce on my side.

I folded the following patch into my tree and pushed it out.

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 0be7e09ba3..2c14a27e39 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -270,14 +270,17 @@ else
 
 # Filter out exported kernel symbol names from the preprocessor output.
 # See also __KSYM_DEPS__ in include/linux/export.h.
+# We disable the depfile generation here, so as not to overwrite the existing
+# depfile while fixdep is parsing it
+flags_nodeps = $(filter-out -Wp$(comma)-M%, $($(1)))
 ksym_dep_filter =                                                            \
 	case "$(1)" in                                                       \
-	cc_*_c) $(CPP) $(c_flags) -D__KSYM_DEPS__ $< ;;                      \
-	as_*_S) $(CPP) $(a_flags) -D__KSYM_DEPS__ $< ;;                      \
+	cc_*_c) $(CPP) $(call flags_nodeps,c_flags) -D__KSYM_DEPS__ $< ;;    \
+	as_*_S) $(CPP) $(call flags_nodeps,a_flags) -D__KSYM_DEPS__ $< ;;    \
 	cpp_lds_S) : ;;                                                      \
-	*) echo "Don't know how to preprocess $(1)"; false ;;                \
+	*) echo "Don't know how to preprocess $(1)" >&2; false ;;            \
 	esac | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
-	
+
 cmd_and_fixdep =                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
 	$(ksym_dep_filter) |                                                 \

Nicolas

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web