[PATCH] kbuild: implement modules.order, take #2

[PATCH] kbuild: implement modules.order, take #2

[Tejun Heo]

When multiple built-in modules (especially drivers) provide the same
capability, they're prioritized by link order specified by the order
listed in Makefile.  This implicit ordering is lost for loadable
modules.

When driver modules are loaded by udev, what comes first in
modules.alias file is selected.  However, the order in this file is
indeterministic (depends on filesystem listing order of installed
modules).  This causes confusion.

The solution is two-parted.  This patch updates kbuild such that it
generates and installs modules.order which contains the name of
modules ordered according to Makefile.  The second part is update to
depmod such that it generates output files according to this file.

Note that both obj-y and obj-m subdirs can contain modules and
ordering information between those two are lost from beginning.
Currently obj-y subdirs are put before obj-m subdirs.

Sam Ravnborg cleaned up Makefile modifications and suggested using awk
to remove duplicate lines from modules.order instead of using separate
C program.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Bill Nottingham <notting@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Kay Sievers <kay.sievers@vrfy.org>
---
 Makefile               |    8 +++++++-
 scripts/Makefile.build |   17 ++++++++++++++++-
 scripts/Makefile.lib   |    6 ++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 92dc3cb..1542dd2 100644
--- a/Makefile
+++ b/Makefile
@@ -1020,9 +1020,14 @@ ifdef CONFIG_MODULES
 all: modules
 
 #	Build modules
+#
+#	A module can be listed more than once in obj-m resulting in
+#	duplicate lines in modules.order files.  Those are removed
+#	using awk while concatenating to the final file.
 
 PHONY += modules
 modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux)
+	$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
 	@echo '  Building modules, stage 2.';
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
 
@@ -1050,6 +1055,7 @@ _modinst_:
 		rm -f $(MODLIB)/build ; \
 		ln -s $(objtree) $(MODLIB)/build ; \
 	fi
+	@cp -f $(objtree)/modules.order $(MODLIB)/
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
 
 # This depmod is only for convenience to give the initial
@@ -1109,7 +1115,7 @@ clean: archclean $(clean-dirs)
 	@find . $(RCS_FIND_IGNORE) \
 		\( -name '*.[oas]' -o -name '*.ko' -o -name '.*.cmd' \
 		-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
-		-o -name '*.symtypes' \) \
+		-o -name '*.symtypes' -o -name 'modules.order' \) \
 		-type f -print | xargs rm -f
 
 # mrproper - Delete all generated files, including .config
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index de9836e..875cbdb 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -83,10 +83,12 @@ ifneq ($(strip $(obj-y) $(obj-m) $(obj-n) $(obj-) $(lib-target)),)
 builtin-target := $(obj)/built-in.o
 endif
 
+modorder-target := $(obj)/modules.order
+
 # We keep a list of all modules in $(MODVERDIR)
 
 __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
-	 $(if $(KBUILD_MODULES),$(obj-m)) \
+	 $(if $(KBUILD_MODULES),$(obj-m) $(modorder-target)) \
 	 $(subdir-ym) $(always)
 	@:
 
@@ -276,6 +278,19 @@ targets += $(builtin-target)
 endif # builtin-target
 
 #
+# Rule to create modules.order file
+#
+# Create commands to either record .ko file or cat modules.order from
+# a subdirectory
+modorder-cmds =						\
+	$(foreach m, $(modorder),			\
+		$(if $(filter %/modules.order, $m),	\
+			cat $m;, echo kernel/$m;)) 
+
+$(modorder-target): $(subdir-ym) FORCE
+	$(Q)(cat /dev/null; $(modorder-cmds)) > $@
+
+#
 # Rule to compile a set of .o files into one .a file
 #
 ifdef lib-target
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 3c5e88b..8e44023 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -25,6 +25,11 @@ lib-y := $(filter-out $(obj-y), $(sort $(lib-y) $(lib-m)))
 # o if we encounter foo/ in $(obj-m), remove it from $(obj-m) 
 #   and add the directory to the list of dirs to descend into: $(subdir-m)
 
+# Determine modorder.
+# Unfortunately, we don't have information about ordering between -y
+# and -m subdirs.  Just put -y's first.
+modorder	:= $(patsubst %/,%/modules.order, $(filter %/, $(obj-y)) $(obj-m:.o=.ko))
+
 __subdir-y	:= $(patsubst %/,%,$(filter %/, $(obj-y)))
 subdir-y	+= $(__subdir-y)
 __subdir-m	:= $(patsubst %/,%,$(filter %/, $(obj-m)))
@@ -64,6 +69,7 @@ real-objs-m := $(foreach m, $(obj-m), $(if $(strip $($(m:.o=-objs)) $($(m:.o=-y)
 extra-y		:= $(addprefix $(obj)/,$(extra-y))
 always		:= $(addprefix $(obj)/,$(always))
 targets		:= $(addprefix $(obj)/,$(targets))
+modorder	:= $(addprefix $(obj)/,$(modorder))
 obj-y		:= $(addprefix $(obj)/,$(obj-y))
 obj-m		:= $(addprefix $(obj)/,$(obj-m))
 lib-y		:= $(addprefix $(obj)/,$(lib-y))

[PATCH] depmod: sort output according to modules.order, take #2

[Tejun Heo]

Kbuild now generates and installs modules.order along with modules.
This patch updates depmod such that it sorts module list according to
the file before generating output files.  Modules which aren't on
modules.order are put after modules which are ordered by
modules.order.

This makes modprobe to prioritize modules according to kernel
Makefile's just as built-in modules are link-ordered by them.

This patch is against module-init-tools 3.3-pre1.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Bill Nottingham <notting@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Kay Sievers <kay.sievers@vrfy.org>
---
Comment added and path comparion logic slightly modified such that
dirname part of mode->pathname is ignored instead of prepending
dirname to lines read from modules.order.  Behavior-wise it's
identical to the previous version.

Thanks.

 depmod.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/depmod.c b/depmod.c
index ea7ad05..c3ae5a2 100644
--- a/depmod.c
+++ b/depmod.c
@@ -585,6 +585,54 @@ static struct module *grab_basedir(const char *dirname)
 	return list;
 }
 
+static void sort_modules(const char *dirname, struct module **listp)
+{
+	struct module *list = *listp, *tlist = NULL, **tpos = &tlist;
+	FILE *modorder;
+	int dir_len = strlen(dirname) + 1;
+	char file_name[dir_len + strlen("modules.order") + 1];
+	char line[10240];
+
+	sprintf(file_name, "%s/%s", dirname, "modules.order");
+
+	modorder = fopen(file_name, "r");
+	if (!modorder) {
+		/* Older kernels don't generate modules.order.  Just
+		   return if the file doesn't exist. */
+		if (errno == ENOENT)
+			return;
+		fatal("Could not open '%s': %s\n", file_name, strerror(errno));
+	}
+
+	sprintf(line, "%s/", dirname);
+
+	/* move modules listed in modorder file to tlist in order */
+	while (fgets(line, sizeof(line), modorder)) {
+		struct module **pos, *mod;
+		int len = strlen(line);
+
+		if (line[len - 1] == '\n')
+			line[len - 1] = '\0';
+
+		for (pos = &list; (mod = *pos); pos = &(*pos)->next) {
+			if (strcmp(line, mod->pathname + dir_len) == 0) {
+				*pos = mod->next;
+				mod->next = NULL;
+				*tpos = mod;
+				tpos = &mod->next;
+				break;
+			}
+		}
+	}
+
+	/* append the rest */
+	*tpos = list;
+
+	fclose(modorder);
+
+	*listp = tlist;
+}
+
 static void parse_modules(struct module *list)
 {
 	struct module *i;
@@ -857,6 +905,7 @@ int main(int argc, char *argv[])
 	} else {
 		list = grab_basedir(dirname);
 	}
+	sort_modules(dirname, &list);
 	parse_modules(list);
 
 	for (i = 0; i < sizeof(depfiles)/sizeof(depfiles[0]); i++) {

Re: [PATCH] kbuild: implement modules.order, take #2

[Sam Ravnborg]

On Fri, Dec 07, 2007 at 09:04:30PM +0900, Tejun Heo wrote:
> When multiple built-in modules (especially drivers) provide the same
> capability, they're prioritized by link order specified by the order
> listed in Makefile.  This implicit ordering is lost for loadable
> modules.
> 
> When driver modules are loaded by udev, what comes first in
> modules.alias file is selected.  However, the order in this file is
> indeterministic (depends on filesystem listing order of installed
> modules).  This causes confusion.
> 
> The solution is two-parted.  This patch updates kbuild such that it
> generates and installs modules.order which contains the name of
> modules ordered according to Makefile.  The second part is update to
> depmod such that it generates output files according to this file.
> 
> Note that both obj-y and obj-m subdirs can contain modules and
> ordering information between those two are lost from beginning.
> Currently obj-y subdirs are put before obj-m subdirs.
> 
> Sam Ravnborg cleaned up Makefile modifications and suggested using awk
> to remove duplicate lines from modules.order instead of using separate
> C program.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Bill Nottingham <notting@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Kay Sievers <kay.sievers@vrfy.org>

Applied to kbuild.git.

	Sam

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Tejun Heo]

Tejun Heo wrote:
> Kbuild now generates and installs modules.order along with modules.
> This patch updates depmod such that it sorts module list according to
> the file before generating output files.  Modules which aren't on
> modules.order are put after modules which are ordered by
> modules.order.
> 
> This makes modprobe to prioritize modules according to kernel
> Makefile's just as built-in modules are link-ordered by them.
> 
> This patch is against module-init-tools 3.3-pre1.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Bill Nottingham <notting@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Kay Sievers <kay.sievers@vrfy.org>

The kernel part is in now.  Rusty Russell, what do you think about this
depmod change?

Thanks.

-- 
tejun

Re: [PATCH] kbuild: implement modules.order, take #2

[Greg KH]

On Fri, Dec 07, 2007 at 09:04:30PM +0900, Tejun Heo wrote:
> When multiple built-in modules (especially drivers) provide the same
> capability, they're prioritized by link order specified by the order
> listed in Makefile.  This implicit ordering is lost for loadable
> modules.
> 
> When driver modules are loaded by udev, what comes first in
> modules.alias file is selected.  However, the order in this file is
> indeterministic (depends on filesystem listing order of installed
> modules).  This causes confusion.
> 
> The solution is two-parted.  This patch updates kbuild such that it
> generates and installs modules.order which contains the name of
> modules ordered according to Makefile.  The second part is update to
> depmod such that it generates output files according to this file.
> 
> Note that both obj-y and obj-m subdirs can contain modules and
> ordering information between those two are lost from beginning.
> Currently obj-y subdirs are put before obj-m subdirs.
> 
> Sam Ravnborg cleaned up Makefile modifications and suggested using awk
> to remove duplicate lines from modules.order instead of using separate
> C program.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Bill Nottingham <notting@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

thanks for doing this, looks very nice.

greg k-h

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Greg KH]

On Fri, Dec 07, 2007 at 09:07:57PM +0900, Tejun Heo wrote:
> Kbuild now generates and installs modules.order along with modules.
> This patch updates depmod such that it sorts module list according to
> the file before generating output files.  Modules which aren't on
> modules.order are put after modules which are ordered by
> modules.order.
> 
> This makes modprobe to prioritize modules according to kernel
> Makefile's just as built-in modules are link-ordered by them.
> 
> This patch is against module-init-tools 3.3-pre1.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Bill Nottingham <notting@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Kay Sievers <kay.sievers@vrfy.org>

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>

thanks,

greg k-h

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Tejun Heo]

Tejun Heo wrote:
> Tejun Heo wrote:
>> Kbuild now generates and installs modules.order along with modules.
>> This patch updates depmod such that it sorts module list according to
>> the file before generating output files.  Modules which aren't on
>> modules.order are put after modules which are ordered by
>> modules.order.
>>
>> This makes modprobe to prioritize modules according to kernel
>> Makefile's just as built-in modules are link-ordered by them.
>>
>> This patch is against module-init-tools 3.3-pre1.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>> Cc: Sam Ravnborg <sam@ravnborg.org>
>> Cc: Bill Nottingham <notting@redhat.com>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Greg Kroah-Hartman <gregkh@suse.de>
>> Cc: Kay Sievers <kay.sievers@vrfy.org>
> 
> The kernel part is in now.  Rusty Russell, what do you think about this
> depmod change?

Ping?

-- 
tejun

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Rusty Russell]

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

On Wednesday 02 January 2008 22:13:52 Tejun Heo wrote:
> Tejun Heo wrote:
> > Tejun Heo wrote:
> >> Kbuild now generates and installs modules.order along with modules.
> >> This patch updates depmod such that it sorts module list according to
> >> the file before generating output files.  Modules which aren't on
> >> modules.order are put after modules which are ordered by
> >> modules.order.
> >>
> >> This makes modprobe to prioritize modules according to kernel
> >> Makefile's just as built-in modules are link-ordered by them.
> >>
> > The kernel part is in now.  Rusty Russell, what do you think about this
> > depmod change?
>
> Ping?

Oh, sorry.  Jon is now module-init-tools maintainer, and I've cc'd him and 
forwarded your original patch.

Thanks,
Rusty.




[-- Attachment #2: Tejun Heo <htejun@gmail.com>: [PATCH] depmod: sort output according to modules.order, take #2 --]
[-- Type: message/rfc822, Size: 5415 bytes --]

From: Tejun Heo <htejun@gmail.com>
To: Sam Ravnborg <sam@ravnborg.org>, Linux Kernel <linux-kernel@vger.kernel.org>, notting@redhat.com, rusty@rustcorp.com.au, kay.sievers@vrfy.org, Greg KH <greg@kroah.com>
Subject: [PATCH] depmod: sort output according to modules.order, take #2
Date: Fri, 07 Dec 2007 21:07:57 +0900
Message-ID: <4759379D.8090201@gmail.com>

Kbuild now generates and installs modules.order along with modules.
This patch updates depmod such that it sorts module list according to
the file before generating output files.  Modules which aren't on
modules.order are put after modules which are ordered by
modules.order.

This makes modprobe to prioritize modules according to kernel
Makefile's just as built-in modules are link-ordered by them.

This patch is against module-init-tools 3.3-pre1.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Bill Nottingham <notting@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Kay Sievers <kay.sievers@vrfy.org>
---
Comment added and path comparion logic slightly modified such that
dirname part of mode->pathname is ignored instead of prepending
dirname to lines read from modules.order.  Behavior-wise it's
identical to the previous version.

Thanks.

 depmod.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/depmod.c b/depmod.c
index ea7ad05..c3ae5a2 100644
--- a/depmod.c
+++ b/depmod.c
@@ -585,6 +585,54 @@ static struct module *grab_basedir(const char *dirname)
 	return list;
 }
 
+static void sort_modules(const char *dirname, struct module **listp)
+{
+	struct module *list = *listp, *tlist = NULL, **tpos = &tlist;
+	FILE *modorder;
+	int dir_len = strlen(dirname) + 1;
+	char file_name[dir_len + strlen("modules.order") + 1];
+	char line[10240];
+
+	sprintf(file_name, "%s/%s", dirname, "modules.order");
+
+	modorder = fopen(file_name, "r");
+	if (!modorder) {
+		/* Older kernels don't generate modules.order.  Just
+		   return if the file doesn't exist. */
+		if (errno == ENOENT)
+			return;
+		fatal("Could not open '%s': %s\n", file_name, strerror(errno));
+	}
+
+	sprintf(line, "%s/", dirname);
+
+	/* move modules listed in modorder file to tlist in order */
+	while (fgets(line, sizeof(line), modorder)) {
+		struct module **pos, *mod;
+		int len = strlen(line);
+
+		if (line[len - 1] == '\n')
+			line[len - 1] = '\0';
+
+		for (pos = &list; (mod = *pos); pos = &(*pos)->next) {
+			if (strcmp(line, mod->pathname + dir_len) == 0) {
+				*pos = mod->next;
+				mod->next = NULL;
+				*tpos = mod;
+				tpos = &mod->next;
+				break;
+			}
+		}
+	}
+
+	/* append the rest */
+	*tpos = list;
+
+	fclose(modorder);
+
+	*listp = tlist;
+}
+
 static void parse_modules(struct module *list)
 {
 	struct module *i;
@@ -857,6 +905,7 @@ int main(int argc, char *argv[])
 	} else {
 		list = grab_basedir(dirname);
 	}
+	sort_modules(dirname, &list);
 	parse_modules(list);
 
 	for (i = 0; i < sizeof(depfiles)/sizeof(depfiles[0]); i++) {

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Jon Masters]

On Thu, 2008-01-03 at 10:12 +1100, Rusty Russell wrote:

> Oh, sorry.  Jon is now module-init-tools maintainer, and I've cc'd him and 
> forwarded your original patch.

Ta. I was semi-offline last week due to holidays, but already saw the
original mail and have another fix for debug sections - will sort it.

Jon.

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Tejun Heo]

Jon Masters wrote:
> On Thu, 2008-01-03 at 10:12 +1100, Rusty Russell wrote:
> 
>> Oh, sorry.  Jon is now module-init-tools maintainer, and I've cc'd him and 
>> forwarded your original patch.
> 
> Ta. I was semi-offline last week due to holidays, but already saw the
> original mail and have another fix for debug sections - will sort it.

Jon, this patch hasn't made into module-init-tools after 10 months.  Ping?

Thanks.

-- 
tejun

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Jon Masters]

On Thu, 2008-11-13 at 20:36 +0900, Tejun Heo wrote:
> Jon Masters wrote:
> > On Thu, 2008-01-03 at 10:12 +1100, Rusty Russell wrote:
> > 
> >> Oh, sorry.  Jon is now module-init-tools maintainer, and I've cc'd him and 
> >> forwarded your original patch.
> > 
> > Ta. I was semi-offline last week due to holidays, but already saw the
> > original mail and have another fix for debug sections - will sort it.
> 
> Jon, this patch hasn't made into module-init-tools after 10 months.  Ping?

Oops. I missed that one. Also reminds me I should post a patch to clean
up some of these Modules files on a kernel make clean, I think the
markers file was still kicking around last time I built out of tree
modules...should check that.

Jon.

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Jon Masters]

On Thu, 2008-11-13 at 20:36 +0900, Tejun Heo wrote:

> Jon, this patch hasn't made into module-init-tools

I'm redoing it to cope with the binary sorted output files we have now -
i.e. we can't just rely upon the order of modules.dep any more, what
you're actually trying to do with modules.order is ensure that e.g.
modules providing conflicting aliases get handled in "order".

I'll followup.

Jon.

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Tejun Heo]

Jon Masters wrote:
> On Thu, 2008-11-13 at 20:36 +0900, Tejun Heo wrote:
> 
>> Jon, this patch hasn't made into module-init-tools
> 
> I'm redoing it to cope with the binary sorted output files we have now -
> i.e. we can't just rely upon the order of modules.dep any more, what
> you're actually trying to do with modules.order is ensure that e.g.
> modules providing conflicting aliases get handled in "order".

Yes, that's right.  Ah... so, now modules.dep is sorted to speed up
module look up?  Is this code already in module-init-tools?

Thanks.

-- 
tejun

Re: [PATCH] depmod: sort output according to modules.order, take #2

[Kay Sievers]

On Sun, Nov 16, 2008 at 06:25, Tejun Heo <htejun@gmail.com> wrote:
> Jon Masters wrote:
>> On Thu, 2008-11-13 at 20:36 +0900, Tejun Heo wrote:
>>
>>> Jon, this patch hasn't made into module-init-tools
>>
>> I'm redoing it to cope with the binary sorted output files we have now -
>> i.e. we can't just rely upon the order of modules.dep any more, what
>> you're actually trying to do with modules.order is ensure that e.g.
>> modules providing conflicting aliases get handled in "order".
>
> Yes, that's right.  Ah... so, now modules.dep is sorted to speed up
> module look up?

It's a match tree, stored in a binary file format, not a specific ordering.

> Is this code already in module-init-tools?

http://git.kernel.org/?p=utils/kernel/module-init-tools/module-init-tools.git;a=summary

Kay