2008-06-03 21:07:05

by Sam Ravnborg

[permalink] [raw]
Subject: [RFC PATCH] drop support for KALLSYMS_EXTRA_PASS

We have not seen any reports on inconsistent kallsysms data recently
as in the last one or two years. At least I do not recall these
and google had no hits in my searching.

So I suggest removing this as to simplify the final linking.
When doing "make allyesconfig" build this extra linking
takes considerably time (and I usually forgoet to turn it off).

In addition this patch removes the debug target: debug_kallsyms

Sam

Suggested patch:

diff --git a/Makefile b/Makefile
index 8db70fe..911cb90 100644
--- a/Makefile
+++ b/Makefile
@@ -702,7 +702,6 @@ define rule_vmlinux__
rm -f $@; \
/bin/false; \
fi;
- $(verify_kallsyms)
endef


@@ -719,28 +718,8 @@ ifdef CONFIG_KALLSYMS
# o The correct .tmp_kallsyms2.o is linked into the final vmlinux.
# o Verify that the System.map from vmlinux matches the map from
# .tmp_vmlinux2, just in case we did not generate kallsyms correctly.
-# o If CONFIG_KALLSYMS_EXTRA_PASS is set, do an extra pass using
-# .tmp_vmlinux3 and .tmp_kallsyms3.o. This is only meant as a
-# temporary bypass to allow the kernel to be built while the
-# maintainers work out what went wrong with kallsyms.

-ifdef CONFIG_KALLSYMS_EXTRA_PASS
-last_kallsyms := 3
-else
-last_kallsyms := 2
-endif
-
-kallsyms.o := .tmp_kallsyms$(last_kallsyms).o
-
-define verify_kallsyms
- $(Q)$(if $($(quiet)cmd_sysmap), \
- echo ' $($(quiet)cmd_sysmap) .tmp_System.map' &&) \
- $(cmd_sysmap) .tmp_vmlinux$(last_kallsyms) .tmp_System.map
- $(Q)cmp -s System.map .tmp_System.map || \
- (echo Inconsistent kallsyms data; \
- echo Try setting CONFIG_KALLSYMS_EXTRA_PASS; \
- rm .tmp_kallsyms* ; /bin/false )
-endef
+kallsyms.o := .tmp_kallsyms2.o

# Update vmlinux version before link
# Use + in front of this rule to silent warning about make -j1
@@ -758,7 +737,7 @@ quiet_cmd_kallsyms = KSYM $@
cmd_kallsyms = $(NM) -n $< | $(KALLSYMS) \
$(if $(CONFIG_KALLSYMS_ALL),--all-symbols) > $@

-.tmp_kallsyms1.o .tmp_kallsyms2.o .tmp_kallsyms3.o: %.o: %.S scripts FORCE
+.tmp_kallsyms1.o .tmp_kallsyms2.o: %.o: %.S scripts FORCE
$(call if_changed_dep,as_o_S)

.tmp_kallsyms%.S: .tmp_vmlinux% $(KALLSYMS)
@@ -771,22 +750,9 @@ quiet_cmd_kallsyms = KSYM $@
.tmp_vmlinux2: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms1.o FORCE
$(call if_changed,vmlinux__)

-.tmp_vmlinux3: $(vmlinux-lds) $(vmlinux-all) .tmp_kallsyms2.o FORCE
- $(call if_changed,vmlinux__)
-
# Needs to visit scripts/ before $(KALLSYMS) can be used.
$(KALLSYMS): scripts ;

-# Generate some data for debugging strange kallsyms problems
-debug_kallsyms: .tmp_map$(last_kallsyms)
-
-.tmp_map%: .tmp_vmlinux% FORCE
- ($(OBJDUMP) -h $< | $(AWK) '/^ +[0-9]/{print $$4 " 0 " $$2}'; $(NM) $<) | sort > $@
-
-.tmp_map3: .tmp_map2
-
-.tmp_map2: .tmp_map1
-
endif # ifdef CONFIG_KALLSYMS

# Do modpost on a prelinked vmlinux. The finally linked vmlinux has
diff --git a/init/Kconfig b/init/Kconfig
index 6199d11..5fc1067 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -588,18 +588,6 @@ config KALLSYMS_ALL

Say N.

-config KALLSYMS_EXTRA_PASS
- bool "Do an extra kallsyms pass"
- depends on KALLSYMS
- help
- If kallsyms is not working correctly, the build will fail with
- inconsistent kallsyms data. If that occurs, log a bug report and
- turn on KALLSYMS_EXTRA_PASS which should result in a stable build.
- Always say N here unless you find a bug in kallsyms, which must be
- reported. KALLSYMS_EXTRA_PASS is only a temporary workaround while
- you wait for kallsyms to be fixed.
-
-
config HOTPLUG
bool "Support for hot-pluggable devices" if EMBEDDED
default y


2008-06-04 14:02:17

by Paulo Marques

[permalink] [raw]
Subject: Re: [RFC PATCH] drop support for KALLSYMS_EXTRA_PASS

Sam Ravnborg wrote:
> We have not seen any reports on inconsistent kallsysms data recently
> as in the last one or two years. At least I do not recall these
> and google had no hits in my searching.
>
> So I suggest removing this as to simplify the final linking.
> When doing "make allyesconfig" build this extra linking
> takes considerably time (and I usually forgoet to turn it off).

I don't remember any recent (read: in the last few years) problems
either, so I guess this is ok.

> In addition this patch removes the debug target: debug_kallsyms

This might be useful if changes in some binutils version creates new
problems for kallsyms, but it doesn't seem like a lot of work to just
send a reporter a patch to add this back if needed. Since there haven't
been any reports in a few years, I have no problems with this.

Acked-by: Paulo Marques <[email protected]>

--
Paulo Marques - http://www.grupopie.com

"'thinking outside the box' works better if I know what's inside the box."

2008-06-05 09:43:33

by Keith Owens

[permalink] [raw]
Subject: Re: [RFC PATCH] drop support for KALLSYMS_EXTRA_PASS

Sam Ravnborg (on Tue, 3 Jun 2008 23:07:32 +0200) wrote:
>We have not seen any reports on inconsistent kallsysms data recently
>as in the last one or two years. At least I do not recall these
>and google had no hits in my searching.
>
>So I suggest removing this as to simplify the final linking.
>When doing "make allyesconfig" build this extra linking
>takes considerably time (and I usually forgoet to turn it off).
>
>In addition this patch removes the debug target: debug_kallsyms
>
>diff --git a/Makefile b/Makefile
>-define verify_kallsyms
>- $(Q)$(if $($(quiet)cmd_sysmap), \
>- echo ' $($(quiet)cmd_sysmap) .tmp_System.map' &&) \
>- $(cmd_sysmap) .tmp_vmlinux$(last_kallsyms) .tmp_System.map
>- $(Q)cmp -s System.map .tmp_System.map || \
>- (echo Inconsistent kallsyms data; \
>- echo Try setting CONFIG_KALLSYMS_EXTRA_PASS; \
>- rm .tmp_kallsyms* ; /bin/false )
>-endef

I am not happy about removing the check for inconsistent kallsyms data.
Changes to the tool chain could cause this problem to recur and we
would not pick up on it. If we do keep the check for inconsistent data
then we still need some way for the user to continue while the problem
is investigated.

Since your real complaint seems to be the extra time taken in make
allyesconfig then the solution is easy. Change CONFIG_KALLSYMS_EXTRA_PASS
to a Makefile option, KALLSYMS_EXTRA_PASS=y. No extra time required in
make allyesconfig.

2008-06-07 10:51:30

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [RFC PATCH] drop support for KALLSYMS_EXTRA_PASS

On Thu, Jun 05, 2008 at 07:19:24PM +1000, Keith Owens wrote:
> Sam Ravnborg (on Tue, 3 Jun 2008 23:07:32 +0200) wrote:
> >We have not seen any reports on inconsistent kallsysms data recently
> >as in the last one or two years. At least I do not recall these
> >and google had no hits in my searching.
> >
> >So I suggest removing this as to simplify the final linking.
> >When doing "make allyesconfig" build this extra linking
> >takes considerably time (and I usually forgoet to turn it off).
> >
> >In addition this patch removes the debug target: debug_kallsyms
> >
> >diff --git a/Makefile b/Makefile
> >-define verify_kallsyms
> >- $(Q)$(if $($(quiet)cmd_sysmap), \
> >- echo ' $($(quiet)cmd_sysmap) .tmp_System.map' &&) \
> >- $(cmd_sysmap) .tmp_vmlinux$(last_kallsyms) .tmp_System.map
> >- $(Q)cmp -s System.map .tmp_System.map || \
> >- (echo Inconsistent kallsyms data; \
> >- echo Try setting CONFIG_KALLSYMS_EXTRA_PASS; \
> >- rm .tmp_kallsyms* ; /bin/false )
> >-endef
>
> I am not happy about removing the check for inconsistent kallsyms data.
> Changes to the tool chain could cause this problem to recur and we
> would not pick up on it. If we do keep the check for inconsistent data
> then we still need some way for the user to continue while the problem
> is investigated.
>
> Since your real complaint seems to be the extra time taken in make
> allyesconfig then the solution is easy. Change CONFIG_KALLSYMS_EXTRA_PASS
> to a Makefile option, KALLSYMS_EXTRA_PASS=y. No extra time required in
> make allyesconfig.
The primary complaint are the additional complexity in the Makefile.
See http://lkml.org/lkml/2008/6/4/436 for a much simpler
and thus more hackable method for linking vmlinux.

Sam