linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [kbuild 1/5] Warn when building external modules without modversions
  2005-01-18 18:41 [kbuild 0/5] Some of our patches Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 5/5] Dont include absolute filenames in binaries Andreas Gruenbacher
@ 2005-01-18 18:41 ` Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 3/5] Add cloneconfig target Andreas Gruenbacher
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-18 18:41 UTC (permalink / raw)
  To: linux-kernel, Sam Ravnborg; +Cc: Rusty Russell

[-- Attachment #1: check-symvers.diff --]
[-- Type: text/plain, Size: 1385 bytes --]

This adds a warning when building external modules (M= or SUBDIRS=
syntax) and there is no Module.symvers in the object tree. A missing
Module.symvers is a clear sign that the kernel tree itself was never
compiled. The resulting modules will work, but no symbol version
information will be attached to kernel symbols the module uses (because
that information comes from Module.symvers), and so the module will be
more unsafe.

The test works with CONFIG_MODVERSIONS enabled or disabled.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc1-bk6/Makefile
===================================================================
--- linux-2.6.11-rc1-bk6.orig/Makefile
+++ linux-2.6.11-rc1-bk6/Makefile
@@ -1093,9 +1093,17 @@ KBUILD_MODULES := 1
 crmodverdir:
 	$(Q)mkdir -p $(MODVERDIR)
 
+.PHONY: $(objtree)/Module.symvers
+$(objtree)/Module.symvers:
+	@test -e $(objtree)/Module.symvers || ( \
+	echo; \
+	echo "WARNING: Symbol version dump $(objtree)/Module.symvers is " \
+	     "missing; modules will have no modversions."; \
+	echo )
+
 module-dirs := $(addprefix _module_,$(KBUILD_EXTMOD))
 .PHONY: $(module-dirs) modules
-$(module-dirs): crmodverdir
+$(module-dirs): crmodverdir $(objtree)/Module.symvers
 	$(Q)$(MAKE) $(build)=$(patsubst _module_%,%,$@)
 
 modules: $(module-dirs)

--
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [kbuild 0/5] Some of our patches
@ 2005-01-18 18:41 Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 5/5] Dont include absolute filenames in binaries Andreas Gruenbacher
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-18 18:41 UTC (permalink / raw)
  To: linux-kernel, Sam Ravnborg; +Cc: Rusty Russell

Hello,

here are some of our current patches that I consider appropriate for
mainline:

patches/check-symvers.diff
  Warn when building external modules without modversions

  Users often try to build additional external modules without compiling
  the whole kernel first, and without the Module.synvers dump file.
  This results in modules without modversions, which can be dangerous.

patches/default-configuration.diff
  Don't use the running kernel's config file by default

  Targets such as menuconfig fetch their default configuration from the
  running kernel if .config doesn't exist. This often is the wrong
  thing. Don't look at the running kernel for the usual configure
  commands. Instead, add a new cloneconfig target (see next patch).

patches/cloneconfig.diff
  Add cloneconfig target

  This fetches the configuration from the running kernel, and configures
  the kernel source tree based on that.

patches/mod_param-typeinfo.diff
  Include type information as module info where possible

  This patch was on the LKML before; it should have been merged but got
  lost.
  
patches/remove-buildenv-paths-from-binaries.diff
  Don't include absolute filenames in binaries

  When building RPMs, we don't want build environment specific path names
  to appear in binaries.

Regards,
--
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [kbuild 3/5] Add cloneconfig target
  2005-01-18 18:41 [kbuild 0/5] Some of our patches Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 5/5] Dont include absolute filenames in binaries Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 1/5] Warn when building external modules without modversions Andreas Gruenbacher
@ 2005-01-18 18:41 ` Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 4/5] Include type information as module info where possible Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 2/5] Dont use the running kernels config file by default Andreas Gruenbacher
  4 siblings, 0 replies; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-18 18:41 UTC (permalink / raw)
  To: linux-kernel, Sam Ravnborg; +Cc: Rusty Russell

[-- Attachment #1: cloneconfig.diff --]
[-- Type: text/plain, Size: 1346 bytes --]

Cloneconfig takes the first configuration it finds which appears to
belong to the running kernel, and configures the kernel sources to match
this configuration as closely as possible.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc1-bk6/scripts/kconfig/Makefile
===================================================================
--- linux-2.6.11-rc1-bk6.orig/scripts/kconfig/Makefile
+++ linux-2.6.11-rc1-bk6/scripts/kconfig/Makefile
@@ -37,6 +37,22 @@ allnoconfig: $(obj)/conf
 allmodconfig: $(obj)/conf
 	$< -m arch/$(ARCH)/Kconfig
 
+UNAME_RELEASE := $(shell uname -r)
+CLONECONFIG := $(firstword $(wildcard /proc/config.gz \
+				      /lib/modules/$(UNAME_RELEASE)/.config \
+				      /etc/kernel-config \
+				      /boot/config-$(UNAME_RELEASE)))
+cloneconfig: $(obj)/conf
+	$(Q)case "$(CLONECONFIG)" in				\
+	'')	echo -e "The configuration of the running"	\
+			"kernel could not be determined\n";	\
+		false ;;					\
+	*.gz)	gzip -cd $(CLONECONFIG) > .config.running ;;	\
+	*)	cat $(CLONECONFIG) > .config.running ;;		\
+	esac &&							\
+	echo -e "Cloning configuration file $(CLONECONFIG)\n"
+	$(Q)$< -D .config.running arch/$(ARCH)/Kconfig
+
 defconfig: $(obj)/conf
 ifeq ($(KBUILD_DEFCONFIG),)
 	$< -d arch/$(ARCH)/Kconfig

--
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [kbuild 5/5] Dont include absolute filenames in binaries
  2005-01-18 18:41 [kbuild 0/5] Some of our patches Andreas Gruenbacher
@ 2005-01-18 18:41 ` Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 1/5] Warn when building external modules without modversions Andreas Gruenbacher
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-18 18:41 UTC (permalink / raw)
  To: linux-kernel, Sam Ravnborg; +Cc: Rusty Russell

[-- Attachment #1: remove-buildenv-paths-from-binaries.diff --]
[-- Type: text/plain, Size: 2474 bytes --]

The kbuild utilities are compiled with absolute patch names, so paths
starting with $RPM_BUILD_ROOT would end up in the binaries.  To avoid
this, remove all references to __FILE__ (directly and indirectly via
assert()).

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc1-bk6/scripts/genksyms/genksyms.h
===================================================================
--- linux-2.6.11-rc1-bk6.orig/scripts/genksyms/genksyms.h
+++ linux-2.6.11-rc1-bk6/scripts/genksyms/genksyms.h
@@ -25,7 +25,6 @@
 #define MODUTILS_GENKSYMS_H 1
 
 #include <stdio.h>
-#include <assert.h>
 
 
 enum symbol_type
@@ -89,8 +88,17 @@ void error_with_pos(const char *, ...);
 
 #define MODUTILS_VERSION "<in-kernel>"
 
-#define xmalloc(size) ({ void *__ptr = malloc(size); assert(__ptr || size == 0); __ptr; })
-#define xstrdup(str)  ({ char *__str = strdup(str); assert(__str); __str; })
-
+#define xmalloc(size) ({ void *__ptr = malloc(size);		\
+	if(!__ptr && size != 0) {				\
+		fprintf(stderr, "out of memory\n");		\
+		exit(1);					\
+	}							\
+	__ptr; })
+#define xstrdup(str)  ({ char *__str = strdup(str);		\
+	if (!__str) {						\
+		fprintf(stderr, "out of memory\n");		\
+		exit(1);					\
+	}							\
+	__str; })
 
 #endif /* genksyms.h */
Index: linux-2.6.11-rc1-bk6/scripts/mod/modpost.c
===================================================================
--- linux-2.6.11-rc1-bk6.orig/scripts/mod/modpost.c
+++ linux-2.6.11-rc1-bk6/scripts/mod/modpost.c
@@ -47,11 +47,10 @@ warn(const char *fmt, ...)
 	va_end(arglist);
 }
 
-void *do_nofail(void *ptr, const char *file, int line, const char *expr)
+void *do_nofail(void *ptr, const char *expr)
 {
 	if (!ptr) {
-		fatal("Memory allocation failure %s line %d: %s.\n",
-		      file, line, expr);
+		fatal("Memory allocation failure: %s.\n", expr);
 	}
 	return ptr;
 }
Index: linux-2.6.11-rc1-bk6/scripts/mod/modpost.h
===================================================================
--- linux-2.6.11-rc1-bk6.orig/scripts/mod/modpost.h
+++ linux-2.6.11-rc1-bk6/scripts/mod/modpost.h
@@ -53,8 +53,8 @@ static inline void __endian(const void *
 
 #endif
 
-#define NOFAIL(ptr)   do_nofail((ptr), __FILE__, __LINE__, #ptr)
-void *do_nofail(void *ptr, const char *file, int line, const char *expr);
+#define NOFAIL(ptr)   do_nofail((ptr), #ptr)
+void *do_nofail(void *ptr, const char *expr);
 
 struct buffer {
 	char *p;

--
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-18 18:41 [kbuild 0/5] Some of our patches Andreas Gruenbacher
                   ` (3 preceding siblings ...)
  2005-01-18 18:41 ` [kbuild 4/5] Include type information as module info where possible Andreas Gruenbacher
@ 2005-01-18 18:41 ` Andreas Gruenbacher
  2005-01-18 20:15   ` Roman Zippel
  4 siblings, 1 reply; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-18 18:41 UTC (permalink / raw)
  To: linux-kernel, Sam Ravnborg; +Cc: Rusty Russell

[-- Attachment #1: default-configuration.diff --]
[-- Type: text/plain, Size: 1160 bytes --]

A user ran into the following problem: They grab a SuSE kernel-source
package that is more recent than their running kernel. The tree under
/usr/src/linux is unconfigured by default; there is no .config. User
does a ``make menuconfig'', which gets its default values from
/boot/config-$(uname -r). User tries to build the kernel, which doesn't
work.

I would like to get rid of using the running kernel's configuration
unless the user explicitly requests it. Instead, please add the
cloneconfig target to clone the running kernel's config file. See other
patch.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc1-bk6/scripts/kconfig/confdata.c
===================================================================
--- linux-2.6.11-rc1-bk6.orig/scripts/kconfig/confdata.c
+++ linux-2.6.11-rc1-bk6/scripts/kconfig/confdata.c
@@ -20,9 +20,6 @@ const char conf_defname[] = "arch/$ARCH/
 
 const char *conf_confnames[] = {
 	".config",
-	"/lib/modules/$UNAME_RELEASE/.config",
-	"/etc/kernel-config",
-	"/boot/config-$UNAME_RELEASE",
 	conf_defname,
 	NULL,
 };

--
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [kbuild 4/5] Include type information as module info where possible
  2005-01-18 18:41 [kbuild 0/5] Some of our patches Andreas Gruenbacher
                   ` (2 preceding siblings ...)
  2005-01-18 18:41 ` [kbuild 3/5] Add cloneconfig target Andreas Gruenbacher
@ 2005-01-18 18:41 ` Andreas Gruenbacher
  2005-01-19 16:14   ` Andreas Gruenbacher
  2005-01-18 18:41 ` [kbuild 2/5] Dont use the running kernels config file by default Andreas Gruenbacher
  4 siblings, 1 reply; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-18 18:41 UTC (permalink / raw)
  To: linux-kernel, Sam Ravnborg; +Cc: Rusty Russell

[-- Attachment #1: mod_param-typeinfo.diff --]
[-- Type: text/plain, Size: 3541 bytes --]

Originally-from: Rusty Russell <rusty@rustcorp.com.au>
Subject: Include type information as module info where possible
References: 48396

Module parameters no longer have a type in general, as we use a callback
system (module_param_call()).  However, it's useful to include type
information in the commonly-used wrappers: module_param,
module_param_string and module_param_array.

This adds a parmtype modinfo tag for each parameter defined using
module_param() or MODULE_PARM(). This allows modinfo to easily print all
parameters and their types, independent of whether or not the parameter
has a description (MODULE_PARM_DESC()).

Originally-signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc1-bk6/include/linux/moduleparam.h
===================================================================
--- linux-2.6.11-rc1-bk6.orig/include/linux/moduleparam.h
+++ linux-2.6.11-rc1-bk6/include/linux/moduleparam.h
@@ -64,7 +64,8 @@ struct kparam_array
    param_set_XXX and param_check_XXX. */
 #define module_param_named(name, value, type, perm)			   \
 	param_check_##type(name, &(value));				   \
-	module_param_call(name, param_set_##type, param_get_##type, &value, perm)
+	module_param_call(name, param_set_##type, param_get_##type, &value, perm); \
+	MODULE_PARM_TYPE(name, #type)
 
 #define module_param(name, type, perm)				\
 	module_param_named(name, name, type, perm)
@@ -74,7 +75,8 @@ struct kparam_array
 	static struct kparam_string __param_string_##name		\
 		= { len, string };					\
 	module_param_call(name, param_set_copystring, param_get_string,	\
-		   &__param_string_##name, perm)
+		   &__param_string_##name, perm);			\
+	MODULE_PARM_TYPE(name, "string")
 
 /* Called on module insert or kernel boot */
 extern int parse_args(const char *name,
@@ -135,7 +137,8 @@ extern int param_get_invbool(char *buffe
 	= { ARRAY_SIZE(array), nump, param_set_##type, param_get_##type,\
 	    sizeof(array[0]), array };					\
 	module_param_call(name, param_array_set, param_array_get, 	\
-			  &__param_arr_##name, perm)
+			  &__param_arr_##name, perm);			\
+	MODULE_PARM_TYPE(name, "array of " #type)
 
 #define module_param_array(name, type, nump, perm)		\
 	module_param_array_named(name, name, type, nump, perm)
Index: linux-2.6.11-rc1-bk6/include/linux/module.h
===================================================================
--- linux-2.6.11-rc1-bk6.orig/include/linux/module.h
+++ linux-2.6.11-rc1-bk6/include/linux/module.h
@@ -138,6 +138,10 @@ extern struct module __this_module;
 /* What your module does. */
 #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
 
+/* Type information for a module parameter. */
+#define MODULE_PARM_TYPE(name, _type) \
+	__MODULE_INFO(parmtype, name##type, #name ":" _type)
+
 /* One for each parameter, describing how to use it.  Some files do
    multiple of these per line, so can't just use MODULE_INFO. */
 #define MODULE_PARM_DESC(_parm, desc) \
@@ -560,7 +564,8 @@ static inline void MODULE_PARM_(void) { 
 /* DEPRECATED: Do not use. */
 #define MODULE_PARM(var,type)						    \
 struct obsolete_modparm __parm_##var __attribute__((section("__obsparm"))) = \
-{ __stringify(var), type, &MODULE_PARM_ };
+{ __stringify(var), type, &MODULE_PARM_ }; \
+MODULE_PARM_TYPE(var, type);
 #else
 #define MODULE_PARM(var,type) static void __attribute__((__unused__)) *__parm_##var = &MODULE_PARM_;
 #endif

--
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-18 18:41 ` [kbuild 2/5] Dont use the running kernels config file by default Andreas Gruenbacher
@ 2005-01-18 20:15   ` Roman Zippel
  2005-01-19 17:51     ` Andreas Gruenbacher
  0 siblings, 1 reply; 17+ messages in thread
From: Roman Zippel @ 2005-01-18 20:15 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: linux-kernel, Sam Ravnborg, Rusty Russell

Hi,

On Tue, 18 Jan 2005, Andreas Gruenbacher wrote:

> A user ran into the following problem: They grab a SuSE kernel-source
> package that is more recent than their running kernel. The tree under
> /usr/src/linux is unconfigured by default; there is no .config. User
> does a ``make menuconfig'', which gets its default values from
> /boot/config-$(uname -r). User tries to build the kernel, which doesn't
> work.

NAK. This isn't normally supposed to happen and it shouldn't be as bad 
anymore as it used to be. Removing these path doesn't magically create a 
working kernel.

bye, Roman

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 4/5] Include type information as module info where possible
  2005-01-18 18:41 ` [kbuild 4/5] Include type information as module info where possible Andreas Gruenbacher
@ 2005-01-19 16:14   ` Andreas Gruenbacher
  2005-01-20 11:54     ` Andreas Gruenbacher
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-19 16:14 UTC (permalink / raw)
  To: linux-kernel; +Cc: Sam Ravnborg, Rusty Russell

[-- Attachment #1: Type: text/plain, Size: 193 bytes --]

Hello,

MODULE_PARM_TYPE needs to be moved to moduleparam.h: several files
include moduleparam.h but not module.h.

Regards,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX GMBH

[-- Attachment #2: mod_param-typeinfo.diff --]
[-- Type: message/rfc822, Size: 3464 bytes --]

From: Andreas Gruenbacher <agruen@suse.de>
Subject: Include type information as module info where possible
Date: Wed, 19 Jan 2005 17:12:48 +0100
Message-ID: <1106151168.8642.9.camel@winden.suse.de>

Module parameters no longer have a type in general, as we use a callback
system (module_param_call()).  However, it's useful to include type
information in the commonly-used wrappers: module_param,
module_param_string and module_param_array.

This adds a parmtype modinfo tag for each parameter defined using
module_param() or MODULE_PARM(). This allows modinfo to easily print all
parameters and their types, independent of whether or not the parameter
has a description (MODULE_PARM_DESC()).

Originally-signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc1-bk6/include/linux/moduleparam.h
===================================================================
--- linux-2.6.11-rc1-bk6.orig/include/linux/moduleparam.h
+++ linux-2.6.11-rc1-bk6/include/linux/moduleparam.h
@@ -13,6 +13,10 @@
 #define MODULE_PARAM_PREFIX __stringify(KBUILD_MODNAME) "."
 #endif
 
+/* Type information for a module parameter. */
+#define MODULE_PARM_TYPE(name, _type) \
+	__MODULE_INFO(parmtype, name##type, #name ":" _type)
+
 struct kernel_param;
 
 /* Returns 0, or -errno.  arg is in kp->arg. */
@@ -64,7 +68,8 @@ struct kparam_array
    param_set_XXX and param_check_XXX. */
 #define module_param_named(name, value, type, perm)			   \
 	param_check_##type(name, &(value));				   \
-	module_param_call(name, param_set_##type, param_get_##type, &value, perm)
+	module_param_call(name, param_set_##type, param_get_##type, &value, perm); \
+	MODULE_PARM_TYPE(name, #type)
 
 #define module_param(name, type, perm)				\
 	module_param_named(name, name, type, perm)
@@ -74,7 +79,8 @@ struct kparam_array
 	static struct kparam_string __param_string_##name		\
 		= { len, string };					\
 	module_param_call(name, param_set_copystring, param_get_string,	\
-		   &__param_string_##name, perm)
+		   &__param_string_##name, perm);			\
+	MODULE_PARM_TYPE(name, "string")
 
 /* Called on module insert or kernel boot */
 extern int parse_args(const char *name,
@@ -135,7 +141,8 @@ extern int param_get_invbool(char *buffe
 	= { ARRAY_SIZE(array), nump, param_set_##type, param_get_##type,\
 	    sizeof(array[0]), array };					\
 	module_param_call(name, param_array_set, param_array_get, 	\
-			  &__param_arr_##name, perm)
+			  &__param_arr_##name, perm);			\
+	MODULE_PARM_TYPE(name, "array of " #type)
 
 #define module_param_array(name, type, nump, perm)		\
 	module_param_array_named(name, name, type, nump, perm)
Index: linux-2.6.11-rc1-bk6/include/linux/module.h
===================================================================
--- linux-2.6.11-rc1-bk6.orig/include/linux/module.h
+++ linux-2.6.11-rc1-bk6/include/linux/module.h
@@ -560,7 +560,8 @@ static inline void MODULE_PARM_(void) { 
 /* DEPRECATED: Do not use. */
 #define MODULE_PARM(var,type)						    \
 struct obsolete_modparm __parm_##var __attribute__((section("__obsparm"))) = \
-{ __stringify(var), type, &MODULE_PARM_ };
+{ __stringify(var), type, &MODULE_PARM_ }; \
+MODULE_PARM_TYPE(var, type);
 #else
 #define MODULE_PARM(var,type) static void __attribute__((__unused__)) *__parm_##var = &MODULE_PARM_;
 #endif

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-18 20:15   ` Roman Zippel
@ 2005-01-19 17:51     ` Andreas Gruenbacher
  2005-01-19 18:18       ` Roman Zippel
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-19 17:51 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel, Sam Ravnborg, Rusty Russell

On Tue, 2005-01-18 at 21:15, Roman Zippel wrote:
> Hi,
> 
> On Tue, 18 Jan 2005, Andreas Gruenbacher wrote:
> 
> > A user ran into the following problem: They grab a SuSE kernel-source
> > package that is more recent than their running kernel. The tree under
> > /usr/src/linux is unconfigured by default; there is no .config. User
> > does a ``make menuconfig'', which gets its default values from
> > /boot/config-$(uname -r). User tries to build the kernel, which doesn't
> > work.
> 
> NAK. This isn't normally supposed to happen and it shouldn't be as bad 
> anymore as it used to be. Removing these path doesn't magically create a 
> working kernel.

"Not normally supposed to happen" and "shouldn't be as bad anymore"
aren't very sound arguments. It's fundamentally broken to use a
semi-random configuration for a kernel source tree that may be
arbitrarily far apart. In the best case you notice that the
configuration doesn't work anymore. In the worst case you will fall flat
on your nose and only notice what happened after a long time.

It's not uncommon that users who build their own kernel modules often
are very clueless. Nevertheless we shouldn't cause them pain
unnecessarily.

Removing the running kernel's paths at least ensures that we don't get
arbitrary, unexpected results. I'd much prefer the user to be explicit
when he wants to clone the running kernel's configuration. That's what
patch 3/5 in this set does.

Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-19 17:51     ` Andreas Gruenbacher
@ 2005-01-19 18:18       ` Roman Zippel
  2005-01-19 18:42         ` Andreas Gruenbacher
  0 siblings, 1 reply; 17+ messages in thread
From: Roman Zippel @ 2005-01-19 18:18 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: linux-kernel, Sam Ravnborg, Rusty Russell

Hi,

On Wed, 19 Jan 2005, Andreas Gruenbacher wrote:

> > > A user ran into the following problem: They grab a SuSE kernel-source
> > > package that is more recent than their running kernel. The tree under
> > > /usr/src/linux is unconfigured by default; there is no .config. User
> > > does a ``make menuconfig'', which gets its default values from
> > > /boot/config-$(uname -r). User tries to build the kernel, which doesn't
> > > work.
> > 
> > NAK. This isn't normally supposed to happen and it shouldn't be as bad 
> > anymore as it used to be. Removing these path doesn't magically create a 
> > working kernel.
> 
> "Not normally supposed to happen" and "shouldn't be as bad anymore"
> aren't very sound arguments.

It's as precise as above problem report. 

> It's fundamentally broken to use a
> semi-random configuration for a kernel source tree that may be
> arbitrarily far apart.

No, it's not. Please provide more information why exactly this is broken.

> It's not uncommon that users who build their own kernel modules often
> are very clueless. Nevertheless we shouldn't cause them pain
> unnecessarily.

So they should first try the 2.6 kernel provided by the distribution and 
then try compiling their own kernel. In this situation it's actually more 
likely that they produce a working kernel with the current behaviour, the 
defconfig is not a guarantee for a working kernel either.
Sorry, but as long as nobody writes an autoconfig tool for the kernel, the 
kernel configuration is not a simple process and any default can only be a 
compromise and may fail. If you have evidence that there are better 
defaults, we can change this, but your problem report above is not enough.

bye, Roman

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-19 18:18       ` Roman Zippel
@ 2005-01-19 18:42         ` Andreas Gruenbacher
  2005-01-19 19:27           ` Roman Zippel
  2005-01-19 23:41           ` Gerd Knorr
  0 siblings, 2 replies; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-19 18:42 UTC (permalink / raw)
  To: Roman Zippel; +Cc: linux-kernel, Sam Ravnborg, Rusty Russell

On Wed, 2005-01-19 at 19:18, Roman Zippel wrote:
> Hi,
> 
> On Wed, 19 Jan 2005, Andreas Gruenbacher wrote:
> 
> > > > A user ran into the following problem: They grab a SuSE kernel-source
> > > > package that is more recent than their running kernel. The tree under
> > > > /usr/src/linux is unconfigured by default; there is no .config. User
> > > > does a ``make menuconfig'', which gets its default values from
> > > > /boot/config-$(uname -r). User tries to build the kernel, which doesn't
> > > > work.
> > > 
> > > NAK. This isn't normally supposed to happen and it shouldn't be as bad 
> > > anymore as it used to be. Removing these path doesn't magically create a 
> > > working kernel.
> > 
> > "Not normally supposed to happen" and "shouldn't be as bad anymore"
> > aren't very sound arguments.
> 
> It's as precise as above problem report. 
>
> > It's fundamentally broken to use a
> > semi-random configuration for a kernel source tree that may be
> > arbitrarily far apart.
> 
> No, it's not. Please provide more information why exactly this is broken.

Okay, more verbose then. On your machine which is running kernel version
x you build kernel version y. You grab the version y kernel source tree,
let's say a vendor tree, which has meaningful default configurations in
arch/$ARCH/defconfig. The runnig kernel's configuration may also work
for that kernel source tree, or it may not.

The user does a ``make menuconfig'', and expects to see sane defaults.
What kconfig really does is take the running kernel's configuration
instead. This is a ad choice; it makes much more sense to take
arch/$ARCH/defconfig.

> > It's not uncommon that users who build their own kernel modules often
> > are very clueless. Nevertheless we shouldn't cause them pain
> > unnecessarily.
> 
> So they should first try the 2.6 kernel provided by the distribution and 
> then try compiling their own kernel. In this situation it's actually more 
> likely that they produce a working kernel with the current behaviour, the 
> defconfig is not a guarantee for a working kernel either.

You assume that the user is already running the kind of kernel he is
trying to produce. Al least to me this assumption seems weird. Instead
my proposal is to use a different make target if you actually do want to
clone the running kernel's configuration, but this shouldn't be the
default.

> Sorry, but as long as nobody writes an autoconfig tool for the kernel, the 
> kernel configuration is not a simple process and any default can only be a 
> compromise and may fail. If you have evidence that there are better 
> defaults, we can change this, but your problem report above is not enough.

I'm not trying to add more magic, I'm trying to get magic taken out,
because nothing guarantees that taking the running kernel's
configuration makes sense. The kernel packages we ship have meaningful
default configurations on all architectures that allow this. You won't
end up with a highly optimized configuration for your particular
machine, but nothing guarantees that you will end up in a better state
with the running kernel's config.

Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX GMBH


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-19 18:42         ` Andreas Gruenbacher
@ 2005-01-19 19:27           ` Roman Zippel
  2005-01-19 23:41           ` Gerd Knorr
  1 sibling, 0 replies; 17+ messages in thread
From: Roman Zippel @ 2005-01-19 19:27 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: linux-kernel, Sam Ravnborg, Rusty Russell

Hi,

On Wed, 19 Jan 2005, Andreas Gruenbacher wrote:

> Okay, more verbose then. On your machine which is running kernel version
> x you build kernel version y. You grab the version y kernel source tree,
> let's say a vendor tree, which has meaningful default configurations in
> arch/$ARCH/defconfig. The runnig kernel's configuration may also work
> for that kernel source tree, or it may not.

How is that more verbose?
Please provide an example config that worked under 2.4 but doesn't produce 
a reasonable result under 2.6.

> > So they should first try the 2.6 kernel provided by the distribution and 
> > then try compiling their own kernel. In this situation it's actually more 
> > likely that they produce a working kernel with the current behaviour, the 
> > defconfig is not a guarantee for a working kernel either.
> 
> You assume that the user is already running the kind of kernel he is
> trying to produce. Al least to me this assumption seems weird.

Why is that weird?

bye, Roman

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 2/5] Dont use the running kernels config file by default
  2005-01-19 18:42         ` Andreas Gruenbacher
  2005-01-19 19:27           ` Roman Zippel
@ 2005-01-19 23:41           ` Gerd Knorr
  1 sibling, 0 replies; 17+ messages in thread
From: Gerd Knorr @ 2005-01-19 23:41 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: Roman Zippel, linux-kernel, Sam Ravnborg, Rusty Russell

Andreas Gruenbacher <agruen@suse.de> writes:

> The user does a ``make menuconfig'', and expects to see sane defaults.
> What kconfig really does is take the running kernel's configuration
> instead. This is a ad choice; it makes much more sense to take
> arch/$ARCH/defconfig.

IIRC the vanilla kernel's arch/i386/defconfig is the "this config
boots Linus' machine" configuration (or has that changed recently?).
I certainly wouldn't call that a sane default ...

For vendor kernels with a generic defconfig that makes sense though.

  Gerd

-- 
#define printk(args...) fprintf(stderr, ## args)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 4/5] Include type information as module info where possible
  2005-01-19 16:14   ` Andreas Gruenbacher
@ 2005-01-20 11:54     ` Andreas Gruenbacher
  2005-01-30 15:56       ` Sam Ravnborg
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-20 11:54 UTC (permalink / raw)
  To: linux-kernel; +Cc: Sam Ravnborg, Rusty Russell

On Wednesday 19 January 2005 17:14, you wrote:
> Hello,
>
> MODULE_PARM_TYPE needs to be moved to moduleparam.h: several files
> include moduleparam.h but not module.h.

Ah, and __MODULE_INFO needs to be moved to moduleparam.h now as well:


Index: linux-2.6.10/include/linux/moduleparam.h
===================================================================
--- linux-2.6.10.orig/include/linux/moduleparam.h
+++ linux-2.6.10/include/linux/moduleparam.h
@@ -13,6 +13,21 @@
 #define MODULE_PARAM_PREFIX __stringify(KBUILD_MODNAME) "."
 #endif
 
+#ifdef MODULE
+#define ___module_cat(a,b) __mod_ ## a ## b
+#define __module_cat(a,b) ___module_cat(a,b)
+#define __MODULE_INFO(tag, name, info)					  \
+static const char __module_cat(name,__LINE__)[]				  \
+  __attribute_used__							  \
+  __attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info
+#else  /* !MODULE */
+#define __MODULE_INFO(tag, name, info)
+#endif
+
+/* Type information for a module parameter. */
+#define MODULE_PARM_TYPE(name, _type) \
+	__MODULE_INFO(parmtype, name##type, #name ":" _type)
+
 struct kernel_param;
 
 /* Returns 0, or -errno.  arg is in kp->arg. */
@@ -64,7 +79,8 @@ struct kparam_array
    param_set_XXX and param_check_XXX. */
 #define module_param_named(name, value, type, perm)			   \
 	param_check_##type(name, &(value));				   \
-	module_param_call(name, param_set_##type, param_get_##type, &value, perm)
+	module_param_call(name, param_set_##type, param_get_##type, &value, perm); \
+	MODULE_PARM_TYPE(name, #type)
 
 #define module_param(name, type, perm)				\
 	module_param_named(name, name, type, perm)
@@ -74,7 +90,8 @@ struct kparam_array
 	static struct kparam_string __param_string_##name		\
 		= { len, string };					\
 	module_param_call(name, param_set_copystring, param_get_string,	\
-		   &__param_string_##name, perm)
+		   &__param_string_##name, perm);			\
+	MODULE_PARM_TYPE(name, "string")
 
 /* Called on module insert or kernel boot */
 extern int parse_args(const char *name,
@@ -135,7 +152,8 @@ extern int param_get_invbool(char *buffe
 	= { ARRAY_SIZE(array), nump, param_set_##type, param_get_##type,\
 	    sizeof(array[0]), array };					\
 	module_param_call(name, param_array_set, param_array_get, 	\
-			  &__param_arr_##name, perm)
+			  &__param_arr_##name, perm);			\
+	MODULE_PARM_TYPE(name, "array of " #type)
 
 #define module_param_array(name, type, nump, perm)		\
 	module_param_array_named(name, name, type, nump, perm)
Index: linux-2.6.10/include/linux/module.h
===================================================================
--- linux-2.6.10.orig/include/linux/module.h
+++ linux-2.6.10/include/linux/module.h
@@ -76,13 +76,6 @@ void sort_main_extable(void);
 extern struct subsystem module_subsys;
 
 #ifdef MODULE
-#define ___module_cat(a,b) __mod_ ## a ## b
-#define __module_cat(a,b) ___module_cat(a,b)
-#define __MODULE_INFO(tag, name, info)					  \
-static const char __module_cat(name,__LINE__)[]				  \
-  __attribute_used__							  \
-  __attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info
-
 #define MODULE_GENERIC_TABLE(gtype,name)			\
 extern const struct gtype##_id __mod_##gtype##_table		\
   __attribute__ ((unused, alias(__stringify(name))))
@@ -93,7 +86,6 @@ extern struct module __this_module;
 #else  /* !MODULE */
 
 #define MODULE_GENERIC_TABLE(gtype,name)
-#define __MODULE_INFO(tag, name, info)
 #define THIS_MODULE ((struct module *)0)
 #endif
 
@@ -564,7 +556,8 @@ static inline void MODULE_PARM_(void) { 
 /* DEPRECATED: Do not use. */
 #define MODULE_PARM(var,type)						    \
 struct obsolete_modparm __parm_##var __attribute__((section("__obsparm"))) = \
-{ __stringify(var), type, &MODULE_PARM_ };
+{ __stringify(var), type, &MODULE_PARM_ }; \
+MODULE_PARM_TYPE(var, type);
 #else
 #define MODULE_PARM(var,type) static void __attribute__((__unused__)) *__parm_##var = &MODULE_PARM_;
 #endif


Regards,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 4/5] Include type information as module info where possible
  2005-01-20 11:54     ` Andreas Gruenbacher
@ 2005-01-30 15:56       ` Sam Ravnborg
  2005-01-30 22:06         ` Andreas Gruenbacher
  0 siblings, 1 reply; 17+ messages in thread
From: Sam Ravnborg @ 2005-01-30 15:56 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: linux-kernel, Sam Ravnborg, Rusty Russell

On Thu, Jan 20, 2005 at 12:54:46PM +0100, Andreas Gruenbacher wrote:
> On Wednesday 19 January 2005 17:14, you wrote:
> > Hello,
> >
> > MODULE_PARM_TYPE needs to be moved to moduleparam.h: several files
> > include moduleparam.h but not module.h.
> 
> Ah, and __MODULE_INFO needs to be moved to moduleparam.h now as well:

Rusty got the original patch applied.
Can you please update this to latest kernel and resend including updated
changelog.

Thanks,
	Sam

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 4/5] Include type information as module info where possible
  2005-01-30 15:56       ` Sam Ravnborg
@ 2005-01-30 22:06         ` Andreas Gruenbacher
  2005-01-30 22:19           ` Sam Ravnborg
  0 siblings, 1 reply; 17+ messages in thread
From: Andreas Gruenbacher @ 2005-01-30 22:06 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: linux-kernel, Rusty Russell

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

On Sunday 30 January 2005 16:56, Sam Ravnborg wrote:
> Rusty got the original patch applied.

Not completely. The fix is attached.

Cheers,
-- 
Andreas Gruenbacher <agruen@suse.de>
SUSE Labs, SUSE LINUX PRODUCTS GMBH

[-- Attachment #2: module-parmtype.diff --]
[-- Type: text/x-diff, Size: 4477 bytes --]

From: Andreas Gruenbacher <agruen@suse.de>
Subject: Module parameter type fixes

The wrong version of the parmtype patch was merged, incompletely, and
the part that got merged got broken on the way.  Here are the fixes:

Move __MODULE_INFO to modparam.h: This macro is used in modparam.h;
there are users who include this header but not module.h.  The latter
includes modparam.h already.

__MODULE_INFO(parmtype, name##type, #name ":" #type) does not evaluate
to __MODULE_INFO(parmtype, footype, "foo:int") as was the idea, but to
__MODULE_INFO(parmtype, fooint, "foo:int") when type is bound to int.
In more complicated cases, we get syntax erros.  Re-introduce the
__MODULE_PARM_TYPE macro; this is cleaner than renaming the type parameter.

Add the parmtype definition which was dropped during the merge to to the
obsolete but still heavily used MODULE_PARM macro.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>

Index: linux-2.6.11-rc2-bk8/include/linux/module.h
===================================================================
--- linux-2.6.11-rc2-bk8.orig/include/linux/module.h
+++ linux-2.6.11-rc2-bk8/include/linux/module.h
@@ -77,24 +77,14 @@ void sort_main_extable(void);
 extern struct subsystem module_subsys;
 
 #ifdef MODULE
-#define ___module_cat(a,b) __mod_ ## a ## b
-#define __module_cat(a,b) ___module_cat(a,b)
-#define __MODULE_INFO(tag, name, info)					  \
-static const char __module_cat(name,__LINE__)[]				  \
-  __attribute_used__							  \
-  __attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info
-
 #define MODULE_GENERIC_TABLE(gtype,name)			\
 extern const struct gtype##_id __mod_##gtype##_table		\
   __attribute__ ((unused, alias(__stringify(name))))
 
 extern struct module __this_module;
 #define THIS_MODULE (&__this_module)
-
 #else  /* !MODULE */
-
 #define MODULE_GENERIC_TABLE(gtype,name)
-#define __MODULE_INFO(tag, name, info)
 #define THIS_MODULE ((struct module *)0)
 #endif
 
@@ -560,7 +550,8 @@ static inline void MODULE_PARM_(void) { 
 /* DEPRECATED: Do not use. */
 #define MODULE_PARM(var,type)						    \
 struct obsolete_modparm __parm_##var __attribute__((section("__obsparm"))) = \
-{ __stringify(var), type, &MODULE_PARM_ };
+{ __stringify(var), type, &MODULE_PARM_ }; \
+__MODULE_PARM_TYPE(var, type);
 #else
 #define MODULE_PARM(var,type) static void __attribute__((__unused__)) *__parm_##var = &MODULE_PARM_;
 #endif
Index: linux-2.6.11-rc2-bk8/include/linux/moduleparam.h
===================================================================
--- linux-2.6.11-rc2-bk8.orig/include/linux/moduleparam.h
+++ linux-2.6.11-rc2-bk8/include/linux/moduleparam.h
@@ -13,6 +13,19 @@
 #define MODULE_PARAM_PREFIX __stringify(KBUILD_MODNAME) "."
 #endif
 
+#ifdef MODULE
+#define ___module_cat(a,b) __mod_ ## a ## b
+#define __module_cat(a,b) ___module_cat(a,b)
+#define __MODULE_INFO(tag, name, info)					  \
+static const char __module_cat(name,__LINE__)[]				  \
+  __attribute_used__							  \
+  __attribute__((section(".modinfo"),unused)) = __stringify(tag) "=" info
+#else  /* !MODULE */
+#define __MODULE_INFO(tag, name, info)
+#endif
+#define __MODULE_PARM_TYPE(name, _type)					  \
+  __MODULE_INFO(parmtype, name##type, #name ":" _type)
+
 struct kernel_param;
 
 /* Returns 0, or -errno.  arg is in kp->arg. */
@@ -65,7 +78,7 @@ struct kparam_array
 #define module_param_named(name, value, type, perm)			   \
 	param_check_##type(name, &(value));				   \
 	module_param_call(name, param_set_##type, param_get_##type, &value, perm); \
-	__MODULE_INFO(parmtype, name##type, #name ":" #type)
+	__MODULE_PARM_TYPE(name, #type)
 
 #define module_param(name, type, perm)				\
 	module_param_named(name, name, type, perm)
@@ -76,7 +89,7 @@ struct kparam_array
 		= { len, string };					\
 	module_param_call(name, param_set_copystring, param_get_string,	\
 		   &__param_string_##name, perm);			\
-	__MODULE_INFO(parmtype, name##type, #name ":string")
+	__MODULE_PARM_TYPE(name, "string")
 
 /* Called on module insert or kernel boot */
 extern int parse_args(const char *name,
@@ -138,7 +151,7 @@ extern int param_get_invbool(char *buffe
 	    sizeof(array[0]), array };					\
 	module_param_call(name, param_array_set, param_array_get, 	\
 			  &__param_arr_##name, perm);			\
-	__MODULE_INFO(parmtype, name##type, #name ":array of " #type)
+	__MODULE_PARM_TYPE(name, "array of " #type)
 
 #define module_param_array(name, type, nump, perm)		\
 	module_param_array_named(name, name, type, nump, perm)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [kbuild 4/5] Include type information as module info where possible
  2005-01-30 22:06         ` Andreas Gruenbacher
@ 2005-01-30 22:19           ` Sam Ravnborg
  0 siblings, 0 replies; 17+ messages in thread
From: Sam Ravnborg @ 2005-01-30 22:19 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: Sam Ravnborg, linux-kernel, Rusty Russell

On Sun, Jan 30, 2005 at 11:06:11PM +0100, Andreas Gruenbacher wrote:
> On Sunday 30 January 2005 16:56, Sam Ravnborg wrote:
> > Rusty got the original patch applied.
> 
> Not completely. The fix is attached.

Applied.

	Sam

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2005-01-30 22:18 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-18 18:41 [kbuild 0/5] Some of our patches Andreas Gruenbacher
2005-01-18 18:41 ` [kbuild 5/5] Dont include absolute filenames in binaries Andreas Gruenbacher
2005-01-18 18:41 ` [kbuild 1/5] Warn when building external modules without modversions Andreas Gruenbacher
2005-01-18 18:41 ` [kbuild 3/5] Add cloneconfig target Andreas Gruenbacher
2005-01-18 18:41 ` [kbuild 4/5] Include type information as module info where possible Andreas Gruenbacher
2005-01-19 16:14   ` Andreas Gruenbacher
2005-01-20 11:54     ` Andreas Gruenbacher
2005-01-30 15:56       ` Sam Ravnborg
2005-01-30 22:06         ` Andreas Gruenbacher
2005-01-30 22:19           ` Sam Ravnborg
2005-01-18 18:41 ` [kbuild 2/5] Dont use the running kernels config file by default Andreas Gruenbacher
2005-01-18 20:15   ` Roman Zippel
2005-01-19 17:51     ` Andreas Gruenbacher
2005-01-19 18:18       ` Roman Zippel
2005-01-19 18:42         ` Andreas Gruenbacher
2005-01-19 19:27           ` Roman Zippel
2005-01-19 23:41           ` Gerd Knorr

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).