2002-07-30 23:02:25

by Roman Zippel

[permalink] [raw]
Subject: [PATCH] automatic module_init ordering

Index: Makefile
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/Makefile,v
retrieving revision 1.1.1.30
diff -u -p -r1.1.1.30 Makefile
--- Makefile 27 Jul 2002 12:34:13 -0000 1.1.1.30
+++ Makefile 30 Jul 2002 21:57:02 -0000
@@ -268,6 +268,7 @@ cmd_link_vmlinux = $(LD) $(LDFLAGS) $(LD
$(DRIVERS) \
$(NETWORKS) \
--end-group \
+ init/generated-initcalls.o \
-o vmlinux

# set -e makes the rule exit immediately on error
@@ -284,9 +285,30 @@ define rule_link_vmlinux
$(NM) vmlinux | grep -v '\(compiled\)\|\(\.o$$\)\|\( [aUw] \)\|\(\.\.ng$$\)\|\(LASH[RL]DI\)' | sort > System.map
endef

-vmlinux: $(vmlinux-objs) FORCE
+init/generated-initcalls.c: .allbuiltin_mods
+ set -e; \
+ while read obj; do $(NM) $$obj | sed -n "s,^[0-9a-f ]*\([UTD] .*\),\1 $$obj,p"; done < $< | \
+ awk '/^U/ { print $$2 " " $$3 | "sort -u > .undefined.tmp" }; \
+ /^[TD]/ { print $$2 " " $$3 | "sort -u > .defined.tmp" }'; \
+ join -o "1.2 2.2" .defined.tmp .undefined.tmp | sort -u | tsort > .sorted.tmp; \
+ grep -v -f .sorted.tmp < $< > .other.tmp; \
+ cat .other.tmp >> .sorted.tmp; \
+ while read obj; do \
+ call=$$($(OBJDUMP) -t $$obj | awk '/F \.initcall\.module/ { print $$6 }' ); \
+ defs="$$defs extern int $$call(void);"; arr="$$arr $$call,"; \
+ done < .sorted.tmp; \
+ echo "#include <linux/init.h>" > $@; \
+ echo $$defs >> $@; \
+ echo 'initcall_t generated_initcalls[] = {' >> $@; \
+ echo $$arr >> $@; \
+ echo '0 };' >> $@
+
+vmlinux: $(vmlinux-objs) init/generated-initcalls.o FORCE
$(call if_changed_rule,link_vmlinux)

+.allbuiltin_mods: $(CORE_FILES) $(LIBS) $(DRIVERS) $(NETWORKS)
+ for s in $(dir $^); do sed "s,^,$$s," < $${s}.builtin_mods; done > $@
+
# The actual objects are generated when descending,
# make sure no implicit rule kicks in

@@ -586,6 +608,7 @@ defconfig:

# files removed with 'make clean'
CLEAN_FILES += \
+ init/generated-initcalls.c .allbuiltin_mods \
include/linux/compile.h \
vmlinux System.map \
drivers/char/consolemap_deftbl.c drivers/video/promcon_tbl.c \
@@ -647,7 +670,7 @@ clean: archclean
@echo 'Cleaning up'
@find . -name SCCS -prune -o -name BitKeeper -prune -o \
\( -name \*.[oas] -o -name core -o -name .\*.cmd -o \
- -name .\*.tmp -o -name .\*.d \) -type f -print \
+ -name .\*.tmp -o -name .\*.d -o -name .builtin_mods \) -type f -print \
| grep -v lxdialog/ | xargs rm -f
@rm -f $(CLEAN_FILES)
@$(MAKE) -f Documentation/DocBook/Makefile clean
Index: Rules.make
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/Rules.make,v
retrieving revision 1.1.1.17
diff -u -p -r1.1.1.17 Rules.make
--- Rules.make 6 Jul 2002 00:33:37 -0000 1.1.1.17
+++ Rules.make 30 Jul 2002 18:37:47 -0000
@@ -105,9 +105,16 @@ multi-objs-m := $(foreach m, $(multi-use
subdir-obj-y := $(foreach o,$(obj-y),$(if $(filter-out $(o),$(notdir $(o))),$(o)))

# Replace multi-part objects by their individual parts, look at local dir only
-real-objs-y := $(foreach m, $(filter-out $(subdir-obj-y), $(obj-y)), $(if $($(m:.o=-objs)),$($(m:.o=-objs)),$(m))) $(EXTRA_TARGETS)
+mod-objs-y := $(filter-out $(subdir-obj-y), $(obj-y))
+real-objs-y := $(foreach m, $(mod-objs-y), $(if $($(m:.o=-objs)),$($(m:.o=-objs)),$(m))) $(EXTRA_TARGETS)
real-objs-m := $(foreach m, $(obj-m), $(if $($(m:.o=-objs)),$($(m:.o=-objs)),$(m)))

+modnames := $(foreach m, $(sort $(mod-objs-y) $(objs-m)), $(if $($(m:.o=-objs)),$(foreach mm,$($(m:.o=-objs)),.$(mm).$(m))))
+
+modname = $(*F)
+
+$(multi-objs-y) $(multi-objs-m) : modname = $(patsubst .$@.%,%,$(filter .$@.%,$(modnames)))
+
# Only build module versions for files which are selected to be built
export-objs := $(filter $(export-objs),$(real-objs-y) $(real-objs-m))

@@ -168,6 +175,7 @@ $(addprefix $(MODVERDIR)/,$(export-objs:
c_flags = -Wp,-MD,$(depfile) $(CFLAGS) $(NOSTDINC_FLAGS) \
$(modkern_cflags) $(EXTRA_CFLAGS) $(CFLAGS_$(*F).o) \
-DKBUILD_BASENAME=$(subst $(comma),_,$(subst -,_,$(*F))) \
+ -DKBUILD_MODNAME=$(subst $(comma),_,$(subst -,_,$(modname:.o=))) \
$(export_flags)

# Our objects only depend on modversions.h, not on the individual .ver
@@ -248,7 +256,7 @@ endif
# The echo suppresses the "Nothing to be done for first_rule"
first_rule: $(if $(KBUILD_BUILTIN),$(O_TARGET) $(L_TARGET) $(EXTRA_TARGETS)) \
$(if $(KBUILD_MODULES),$(obj-m)) \
- sub_dirs
+ sub_dirs .builtin_mods
@echo -n

# Compile C sources (.c)
@@ -269,6 +277,7 @@ $(export-objs:.o=.lst): export_flags :
c_flags = -Wp,-MD,$(depfile) $(CFLAGS) $(NOSTDINC_FLAGS) \
$(modkern_cflags) $(EXTRA_CFLAGS) $(CFLAGS_$(*F).o) \
-DKBUILD_BASENAME=$(subst $(comma),_,$(subst -,_,$(*F))) \
+ -DKBUILD_MODNAME=$(subst $(comma),_,$(subst -,_,$(modname:.o=))) \
$(export_flags)

quiet_cmd_cc_s_c = CC $(echo_target)
@@ -294,6 +303,14 @@ cmd_cc_lst_c = $(CC) $(c_flags) -g -

%.lst: %.c FORCE
$(call if_changed_dep,cc_lst_c)
+
+$(subdir-y:%=%/.builtin_mods): sub_dirs
+
+.builtin_mods: $(mod-objs-y) $(subdir-y:%=%/.builtin_mods)
+ for o in $(mod-objs-y); do \
+ if [ -n "$$($(OBJDUMP) -h $$o | grep .initcall.module)" ]; then echo $$o; fi \
+ done > $@; \
+ for s in $(subdir-y); do sed "s,^,$$s/," < $$s/.builtin_mods; done >> $@

# Compile assembler sources (.S)
# ---------------------------------------------------------------------------
Index: drivers/ide/main.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/drivers/ide/main.c,v
retrieving revision 1.1.1.7
diff -u -p -r1.1.1.7 main.c
--- drivers/ide/main.c 27 Jul 2002 12:40:01 -0000 1.1.1.7
+++ drivers/ide/main.c 30 Jul 2002 18:37:47 -0000
@@ -1397,23 +1397,6 @@ static int __init ata_module_init(void)
}
# endif
#endif
-
- /*
- * Initialize all device type driver modules.
- */
-#ifdef CONFIG_BLK_DEV_IDEDISK
- idedisk_init();
-#endif
-#ifdef CONFIG_BLK_DEV_IDECD
- ide_cdrom_init();
-#endif
-#ifdef CONFIG_BLK_DEV_IDETAPE
- idetape_init();
-#endif
-#ifdef CONFIG_BLK_DEV_IDEFLOPPY
- idefloppy_init();
-#endif
-
initializing = 0;

register_reboot_notifier(&ata_notifier);
Index: fs/dnotify.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/fs/dnotify.c,v
retrieving revision 1.1.1.5
diff -u -p -r1.1.1.5 dnotify.c
--- fs/dnotify.c 27 Jul 2002 12:34:15 -0000 1.1.1.5
+++ fs/dnotify.c 30 Jul 2002 18:37:47 -0000
@@ -155,4 +155,4 @@ static int __init dnotify_init(void)
return 0;
}

-module_init(dnotify_init)
+fs_initcall(dnotify_init);
Index: fs/fcntl.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/fs/fcntl.c,v
retrieving revision 1.1.1.8
diff -u -p -r1.1.1.8 fcntl.c
--- fs/fcntl.c 27 Jul 2002 12:34:14 -0000 1.1.1.8
+++ fs/fcntl.c 30 Jul 2002 18:37:47 -0000
@@ -568,4 +568,4 @@ static int __init fasync_init(void)
return 0;
}

-module_init(fasync_init)
+fs_initcall(fasync_init);
Index: fs/locks.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/fs/locks.c,v
retrieving revision 1.1.1.9
diff -u -p -r1.1.1.9 locks.c
--- fs/locks.c 27 Jul 2002 12:34:14 -0000 1.1.1.9
+++ fs/locks.c 30 Jul 2002 18:37:47 -0000
@@ -1936,4 +1936,4 @@ static int __init filelock_init(void)
return 0;
}

-module_init(filelock_init)
+fs_initcall(filelock_init);
Index: include/linux/init.h
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/include/linux/init.h,v
retrieving revision 1.1.1.6
diff -u -p -r1.1.1.6 init.h
--- include/linux/init.h 13 Jun 2002 00:22:57 -0000 1.1.1.6
+++ include/linux/init.h 30 Jul 2002 18:37:47 -0000
@@ -106,6 +106,9 @@ extern struct kernel_param __setup_start
#define __FINIT .previous
#define __INITDATA .section ".data.init","aw"

+#define ___concat(a,b) a##b
+#define __concat(a,b) ___concat(a,b)
+
/**
* module_init() - driver initialization entry point
* @x: function to be run at kernel boot time or module insertion
@@ -116,7 +119,10 @@ extern struct kernel_param __setup_start
* routine with init_module() which is used by insmod and
* modprobe when the driver is used as a module.
*/
-#define module_init(x) __initcall(x);
+#define module_init(x) \
+int __attribute__((__section__ (".initcall.module"))) \
+__concat(init_module_,KBUILD_MODNAME)(void) \
+{ return x(); }

/**
* module_exit() - driver exit entry point
Index: kernel/module.c
===================================================================
RCS file: /usr/src/cvsroot/linux-2.5/kernel/module.c,v
retrieving revision 1.1.1.3
diff -u -p -r1.1.1.3 module.c
--- kernel/module.c 14 Apr 2002 20:01:36 -0000 1.1.1.3
+++ kernel/module.c 30 Jul 2002 22:23:53 -0000
@@ -252,6 +252,19 @@ void __init init_modules(void)
arch_init_modules(&kernel_module);
}

+extern initcall_t generated_initcalls[];
+
+static int __init init_builtin_modules(void)
+{
+ initcall_t *call;
+
+ for (call = generated_initcalls; *call; call++)
+ (*call)();
+ return 0;
+}
+
+device_initcall(init_builtin_modules);
+
/*
* Copy the name of a module from user space.
*/


Attachments:
initcall.diff (8.97 kB)

2002-07-31 02:30:25

by Kai Germaschewski

[permalink] [raw]
Subject: Re: [PATCH] automatic module_init ordering

On Wed, 31 Jul 2002, Roman Zippel wrote:

> Hi,
>
> Rusty Russell wrote:
>
> > Yes, I think we should do this: merge the two together. You seem to
> > be in a coding frenzy: want to do the first cut?
>
> I attached a new version, that only orders module_init(). I did some
> small compile performance tests and I could only see a few seconds
> difference, so the overhead should be negligible.

Okay, here's a suggestion to slightly reorganize the KBUILD_MODNAME
part in Rules.make.

- Some sreorganization of the c_flags since they're needed for
generating modversions (.ver) and compiling
- Use the right KBUILD_MODNAME also when the user just wants a .i/.s/.lst
file for debugging and also when generating modversions
- It looks like with your current approach you can't have a ',' or '-' in
KBUILD_MODNAME - however, that means that KBUILD_MODNAME is not quite
right for passing module parameters for built-in modules on the command
line, it would be confusing to pass parameters for ide-cd as
ide_cd.foo=whatever. So that part could use a little more thought.
- If you think your module_names trick makes a noticable difference, feel
free to re-add it.
- It's possible that objects are linked into more than one module - I
suppose this shouldn't be a problem, since these objects hopefully
don't have a module_init() nor do they export symbols. Not sure if your
patch did handle this.

--Kai


===== Rules.make 1.68 vs edited =====
--- 1.68/Rules.make Mon Jul 29 14:55:41 2002
+++ edited/Rules.make Tue Jul 30 21:27:46 2002
@@ -95,11 +95,15 @@
multi-used-y := $(filter-out $(list-multi),$(__multi-used-y))
multi-used-m := $(filter-out $(list-multi),$(__multi-used-m))

+multi-used := $(multi-used-y) $(multi-used-m)
+
# Build list of the parts of our composite objects, our composite
# objects depend on those (obviously)
multi-objs-y := $(foreach m, $(multi-used-y), $($(m:.o=-objs)))
multi-objs-m := $(foreach m, $(multi-used-m), $($(m:.o=-objs)))

+multi-objs := $(multi-objs-y) $(multi-objs-m)
+
# $(subdir-obj-y) is the list of objects in $(obj-y) which do not live
# in the local directory
subdir-obj-y := $(foreach o,$(obj-y),$(if $(filter-out $(o),$(notdir $(o))),$(o)))
@@ -115,6 +119,23 @@
# contain a comma
depfile = $(subst $(comma),_,$(@D)/.$(@F).d)

+# These flags are needed for modversions and compiling, so we define them here
+# already
+# $(modname_flags) #defines KBUILD_MODNAME as the name of the module it will
+# end up in (or would, if it gets compiled in)
+# Note: It's possible that one object gets potentially linked into more
+# than one module. In that case KBUILD_MODNAME will be set to foo_bar,
+# where foo and bar are the name of the modules.
+basename_flags = -DKBUILD_BASENAME=$(subst $(comma),_,$(subst -,_,$(*F)))
+modname_flags = -DKBUILD_MODNAME=$(subst $(comma),_,$(subst -,_,$(modname)))
+c_flags = -Wp,-MD,$(depfile) $(CFLAGS) $(NOSTDINC_FLAGS) \
+ $(modkern_cflags) $(EXTRA_CFLAGS) $(CFLAGS_$(*F).o) \
+ $(basename_flags) $(modname_flags) $(export_flags)
+
+# Finds the multi-part object the current object will be linked into
+modname-multi = $(subst $(space),_,$(strip $(foreach m,$(multi-used),\
+ $(if $(filter $(*F).o,$($(m:.o=-objs))),$(m:.o=)))))
+
# We're called for one of three purposes:
# o fastdep: build module version files (.ver) for $(export-objs) in
# the current directory
@@ -164,11 +185,10 @@
$(addprefix $(MODVERDIR)/,$(real-objs-y:.o=.ver)): modkern_cflags := $(CFLAGS_KERNEL)
$(addprefix $(MODVERDIR)/,$(real-objs-m:.o=.ver)): modkern_cflags := $(CFLAGS_MODULE)
$(addprefix $(MODVERDIR)/,$(export-objs:.o=.ver)): export_flags := -D__GENKSYMS__
+# Default for not multi-part modules
+modname = $(*F)

-c_flags = -Wp,-MD,$(depfile) $(CFLAGS) $(NOSTDINC_FLAGS) \
- $(modkern_cflags) $(EXTRA_CFLAGS) $(CFLAGS_$(*F).o) \
- -DKBUILD_BASENAME=$(subst $(comma),_,$(subst -,_,$(*F))) \
- $(export_flags)
+$(addprefix $(MODVERDIR)/,$(multi-objs:.o=.ver)) : modname = $(modname-multi)

# Our objects only depend on modversions.h, not on the individual .ver
# files (fix-dep filters them), so touch modversions.h if any of the .ver
@@ -259,6 +279,7 @@

$(real-objs-m) : modkern_cflags := $(CFLAGS_MODULE)
$(real-objs-m:.o=.i) : modkern_cflags := $(CFLAGS_MODULE)
+$(real-objs-m:.o=.s) : modkern_cflags := $(CFLAGS_MODULE)
$(real-objs-m:.o=.lst): modkern_cflags := $(CFLAGS_MODULE)

$(export-objs) : export_flags := $(EXPORT_FLAGS)
@@ -266,10 +287,13 @@
$(export-objs:.o=.s) : export_flags := $(EXPORT_FLAGS)
$(export-objs:.o=.lst): export_flags := $(EXPORT_FLAGS)

-c_flags = -Wp,-MD,$(depfile) $(CFLAGS) $(NOSTDINC_FLAGS) \
- $(modkern_cflags) $(EXTRA_CFLAGS) $(CFLAGS_$(*F).o) \
- -DKBUILD_BASENAME=$(subst $(comma),_,$(subst -,_,$(*F))) \
- $(export_flags)
+# Default for not multi-part modules
+modname = $(*F)
+
+$(multi-objs) : modname = $(modname-multi)
+$(multi-objs:.o=.i) : modname = $(modname-multi)
+$(multi-objs:.o=.s) : modname = $(modname-multi)
+$(multi-objs:.o=.lst) : modname = $(modname-multi)

quiet_cmd_cc_s_c = CC $(echo_target)
cmd_cc_s_c = $(CC) $(c_flags) -S -o $@ $<

2002-07-31 04:38:35

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] automatic module_init ordering

In message <[email protected]> y
ou write:
> - It looks like with your current approach you can't have a ',' or '-' in
> KBUILD_MODNAME - however, that means that KBUILD_MODNAME is not quite
> right for passing module parameters for built-in modules on the command
> line, it would be confusing to pass parameters for ide-cd as
> ide_cd.foo=whatever. So that part could use a little more thought.

My PARAM code actually maps - to _ in parameter parsing, for exactly
this reason. And only a complete idiot would put , in a module name,
so I don't care 8)

> - It's possible that objects are linked into more than one module - I
> suppose this shouldn't be a problem, since these objects hopefully
> don't have a module_init() nor do they export symbols. Not sure if your
> patch did handle this.

There's one piece of code I know which is linked in three places, and
has a module parameter (net/ipv4/netfilter/ip_conntrack_core.c, linked
into ipfwadm.o ipchains.o and ip_conntrack.o.

As it happens, the configuration doesn't allow more than one to be
built in (they can all be modules though), so it's not actually a
problem even after parameter unification.

Thanks,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2002-07-31 17:03:29

by Kai Germaschewski

[permalink] [raw]
Subject: Re: [PATCH] automatic module_init ordering

On Wed, 31 Jul 2002, Rusty Russell wrote:

> My PARAM code actually maps - to _ in parameter parsing, for exactly
> this reason. And only a complete idiot would put , in a module name,
> so I don't care 8)

Tell that to the author of 53c7,8xx.o ;)

> > - It's possible that objects are linked into more than one module - I
> > suppose this shouldn't be a problem, since these objects hopefully
> > don't have a module_init() nor do they export symbols. Not sure if your
> > patch did handle this.
>
> There's one piece of code I know which is linked in three places, and
> has a module parameter (net/ipv4/netfilter/ip_conntrack_core.c, linked
> into ipfwadm.o ipchains.o and ip_conntrack.o.
>
> As it happens, the configuration doesn't allow more than one to be
> built in (they can all be modules though), so it's not actually a
> problem even after parameter unification.

Hmmh, I think that'll need some testing. It will be fine if only one of
the three is "y", the others being "n/undef". However, it looks like it's
possible to have sth like "m/m/y", which would go wrong with the current
approach.

--Kai


2002-07-31 23:36:15

by Rusty Russell

[permalink] [raw]
Subject: Re: [PATCH] automatic module_init ordering

In message <[email protected]> y
ou write:
> On Wed, 31 Jul 2002, Rusty Russell wrote:
>
> > My PARAM code actually maps - to _ in parameter parsing, for exactly
> > this reason. And only a complete idiot would put , in a module name,
> > so I don't care 8)
>
> Tell that to the author of 53c7,8xx.o ;)

Consider that done.

> > As it happens, the configuration doesn't allow more than one to be
> > built in (they can all be modules though), so it's not actually a
> > problem even after parameter unification.
>
> Hmmh, I think that'll need some testing. It will be fine if only one of
> the three is "y", the others being "n/undef". However, it looks like it's
> possible to have sth like "m/m/y", which would go wrong with the current
> approach.

That's a bug. That configuration makes no sense (the modules won't
load). Hmmm... more Config.in complexity coming up 8(

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.