mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Mike Frysinger <vapier.adi@gmail.com>, Michal Marek <mmarek@suse.cz>
Cc: Hendrik Brueckner <brueckner@linux.vnet.ibm.com>,
	mmarek@suse.cz, Michael Holzheu <holzheu@linux.vnet.ibm.com>,
	tabbott@ksplice.com, vda.linux@googlemail.com,
	hpa@linux.intel.com, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, heiko.carstens@de.ibm.com,
	schwidefsky@de.ibm.com
Subject: Re: [PATCH] initramfs: Fix build break on symbol-prefixed archs
Date: Tue, 19 Oct 2010 23:31:12 +0200	[thread overview]
Message-ID: <20101019213112.GA11432@merkur.ravnborg.org> (raw)
In-Reply-To: <AANLkTimLi8BYy_u8NhD15s=+iiODzuG1H_CgZ-cd7jep@mail.gmail.com>

On Tue, Oct 19, 2010 at 04:11:53PM -0400, Mike Frysinger wrote:
> On Tue, Oct 19, 2010 at 07:08, Hendrik Brueckner wrote:
> > --- a/usr/Makefile
> > +++ b/usr/Makefile
> > @@ -19,6 +19,9 @@ suffix_$(CONFIG_INITRAMFS_COMPRESSION_LZ
> >  suffix_$(CONFIG_INITRAMFS_COMPRESSION_LZO)   = .lzo
> >
> >  AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/initramfs_data.cpio$(suffix_y)"
> > +ifdef CONFIG_SYMBOL_PREFIX
> > +AFLAGS_initramfs_data.o += -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> > +endif
> >
> >  # Generate builtin.o based on initramfs_data.o
> >  obj-$(CONFIG_BLK_DEV_INITRD) := initramfs_data.o
> 
> i dont think we want to go this route.  keep it in one place:
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -120,7 +120,9 @@ _c_flags += $(if $(patsubst n%,, \
>  endif
> 
>  ifdef CONFIG_SYMBOL_PREFIX
> -_cpp_flags += -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> +_sym_flags = -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
> +_cpp_flags += $(_sym_flags)
> +_a_flags += $(_sym_flags)
>  endif

The right fix would be to fix some unfortunate breakage in kbuild.
The value of _cpp_flags is only used for preprocessing linker scripts,
but should be used for as and gcc too.

A small analysis:
Today we have different ways to set preprocessor flags.

KBUILD_CPPFLAGS - set in top-level Makefile + arch MAkefile
                  used by all of cpp, as, gcc

EXTRA_CPPFLAGS  - not used at all and can be deleted

cppflags-y      - set in Kbuild files
                - only used when preprocessing linker scripts

CPPFLAGS_<target> - used for linker scripts in many places

It looks like the following untested patch should fix it.
With this patch we have SYMBOL_PREFIX defined when
building assembler files too.

The patch fix it so flags used for preprocessing is also applied
to as and gcc.

@Michal - do you follow my rationale and do you agree?


Another way to fix this would be to fix asm-generic/vmlinx.lds.h
to check for CONFIG_SYMBOL_PREFIX direct.
I cannot see why we require the SYMBOL_PREFIX definition.

See patch number two below for my take on it.
This is my preferred way to deal with this.
The cppflags fix can wait a little.

	Sam

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 54fd1b7..1b8f15d 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -109,10 +109,10 @@ else
 debug_flags =
 endif
 
-orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(KBUILD_SUBDIR_CCFLAGS) \
+orig_c_flags   = $(KBUILD_CFLAGS) $(KBUILD_SUBDIR_CCFLAGS) \
                  $(ccflags-y) $(CFLAGS_$(basetarget).o)
 _c_flags       = $(filter-out $(CFLAGS_REMOVE_$(basetarget).o), $(orig_c_flags))
-_a_flags       = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(KBUILD_SUBDIR_ASFLAGS) \
+_a_flags       = $(KBUILD_AFLAGS) $(KBUILD_SUBDIR_ASFLAGS) \
                  $(asflags-y) $(AFLAGS_$(basetarget).o)
 _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(@F))
 
@@ -150,16 +150,14 @@ __a_flags	=                          $(call flags,_a_flags)
 __cpp_flags     =                          $(call flags,_cpp_flags)
 endif
 
-c_flags        = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
-		 $(__c_flags) $(modkern_cflags)                           \
+cpp_flags      = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
+		 $(__cpp_flags)
+
+c_flags        = $(cpp_flags) $(__c_flags) $(modkern_cflags)              \
 		 -D"KBUILD_STR(s)=\#s" $(basename_flags) $(modname_flags) \
 		  $(debug_flags)
 
-a_flags        = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
-		 $(__a_flags) $(modkern_aflags)
-
-cpp_flags      = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE)     \
-		 $(__cpp_flags)
+a_flags        = $(cpp_flags) $(__a_flags) $(modkern_aflags)
 
 ld_flags       = $(LDFLAGS) $(ldflags-y)
 





diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8a92a17..520e4ac 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -52,12 +52,12 @@
 #define LOAD_OFFSET 0
 #endif
 
-#ifndef SYMBOL_PREFIX
+#ifndef CONFIG_SYMBOL_PREFIX
 #define VMLINUX_SYMBOL(sym) sym
 #else
 #define PASTE2(x,y) x##y
 #define PASTE(x,y) PASTE2(x,y)
-#define VMLINUX_SYMBOL(sym) PASTE(SYMBOL_PREFIX, sym)
+#define VMLINUX_SYMBOL(sym) PASTE(CONFIG_SYMBOL_PREFIX, sym)
 #endif
 
 /* Align . to a 8 byte boundary equals to maximum function alignment. */
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 54fd1b7..08e13b1 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -127,10 +127,6 @@ _c_flags += $(if $(patsubst n%,, \
 		$(CFLAGS_GCOV))
 endif
 
-ifdef CONFIG_SYMBOL_PREFIX
-_cpp_flags += -DSYMBOL_PREFIX=$(patsubst "%",%,$(CONFIG_SYMBOL_PREFIX))
-endif
-
 
 # If building the kernel in a separate objtree expand all occurrences
 # of -Idir to -I$(srctree)/dir except for absolute paths (starting with '/').

  reply	other threads:[~2010-10-19 21:31 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-31  8:23 [PATCH 0/2] initramfs: Cleanup and fix initramfs size calculation Hendrik Brueckner
2010-08-31  8:23 ` [PATCH 1/2] initramfs: Generalize initramfs_data.xxx.S variants Hendrik Brueckner
2010-08-31  8:23 ` [PATCH 2/2] initramfs: Fix initramfs size calculation Hendrik Brueckner
2010-09-01  7:47   ` Américo Wang
2010-10-17 18:28   ` Mike Frysinger
2010-10-18 10:37     ` Hendrik Brueckner
2010-10-18 23:07       ` Mike Frysinger
2010-10-19 11:08         ` [PATCH] initramfs: Fix build break on symbol-prefixed archs Hendrik Brueckner
2010-10-19 20:11           ` Mike Frysinger
2010-10-19 21:31             ` Sam Ravnborg [this message]
2010-10-19 21:39               ` Mike Frysinger
2010-10-27 18:23                 ` Mike Frysinger
2010-10-27 22:46                   ` Michal Marek
2010-10-27 23:10                     ` Mike Frysinger
2010-10-27 23:33                       ` Michal Marek
2010-10-27 23:58                         ` Mike Frysinger
2010-10-28 12:08                           ` Michal Marek
2010-10-28 19:46                             ` Mike Frysinger
2010-10-28 19:51                               ` Mike Frysinger
2010-10-28 20:35                     ` Sam Ravnborg
2010-10-28 20:42                       ` Mike Frysinger
2010-10-28 20:53                         ` Mike Frysinger
2010-10-29  6:12                         ` Sam Ravnborg
2010-10-28 21:19                       ` Michal Marek
2010-11-13 23:17                         ` Mike Frysinger
2010-11-24  8:40                           ` Mike Frysinger
2010-11-29 22:38                             ` Andrew Morton
2010-11-29 23:29                               ` Mike Frysinger
2010-11-30 15:36                                 ` Michal Marek
2010-12-02  4:12                                   ` Mike Frysinger
2010-08-31 14:08 ` [PATCH 0/2] initramfs: Cleanup and fix initramfs size calculation Michal Marek
2010-09-07 21:25   ` H. Peter Anvin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101019213112.GA11432@merkur.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=akpm@linux-foundation.org \
    --cc=brueckner@linux.vnet.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=holzheu@linux.vnet.ibm.com \
    --cc=hpa@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=schwidefsky@de.ibm.com \
    --cc=tabbott@ksplice.com \
    --cc=vapier.adi@gmail.com \
    --cc=vda.linux@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®