Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1249366 > unrolled thread

[PATCH 0/14] init: deps: dependency based (parallelized) init

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/14] init: deps: dependency based (parallelized) init Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
      Re: [PATCH 04/14] init: deps: order network interfaces by link order Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-17 20:30 +0200
        Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 20:40 +0200
          Re: [PATCH 04/14] init: deps: order network interfaces by link order Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-17 21:00 +0200
            Re: [PATCH 04/14] init: deps: order network interfaces by link order Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-17 21:10 +0200
              Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 21:20 +0200
                Re: [PATCH 04/14] init: deps: order network interfaces by link order Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-17 21:40 +0200
                Re: [PATCH 04/14] init: deps: order network interfaces by link order Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-17 21:40 +0200
                  Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 22:00 +0200
                    Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 23:30 +0200
              Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 23:40 +0200
            Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 21:10 +0200
          Re: [PATCH 04/14] init: deps: order network interfaces by link order Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-17 21:00 +0200
          Re: [PATCH 04/14] init: deps: order network interfaces by link order Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-17 21:10 +0200
            Re: [PATCH 04/14] init: deps: order network interfaces by link order Alexander Holler <holler@ahsoftware.de> - 2015-10-17 21:10 +0200
    [PATCH 03/14] init: deps: dt: use (HW-specific) dependencies provided by the DT too Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 06/14] dtc: deps: Automatically add new property 'dependencies' which contains a list of referenced phandles Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 07/14] dtc: deps: introduce new (virtual) property no-dependencies Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 09/14] dtc: deps: Add option to print dependency graph as dot (Graphviz) Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 02/14] init: deps: use annotated initcalls for a dependency based (optionally parallelized) init Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 01/14] init: deps: introduce annotated initcalls Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:20 +0200
    [PATCH 14/14] dt: dts: deps: omap: beagle: make some remote-endpoints non-dependencies Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:30 +0200
    [PATCH 13/14] dt: dts: deps: imx6q: make some remote-endpoints non-dependencies Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:30 +0200
    [PATCH 12/14] dt: dts: deps: kirkwood: dockstar: add dependency ehci -> usb power regulator Alexander Holler <holler@ahsoftware.de> - 2015-10-17 19:30 +0200
    Re: [PATCH 0/14] init: deps: dependency based (parallelized) init Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-17 19:50 +0200
      Re: [PATCH 0/14] init: deps: dependency based (parallelized) init Alexander Holler <holler@ahsoftware.de> - 2015-10-17 20:30 +0200
        Re: [PATCH 0/14] init: deps: dependency based (parallelized) init Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-17 20:40 +0200
          Re: [PATCH 0/14] init: deps: dependency based (parallelized) init Alexander Holler <holler@ahsoftware.de> - 2015-10-17 21:50 +0200
            Re: [PATCH 0/14] init: deps: dependency based (parallelized) init Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-17 22:30 +0200
              Re: [PATCH 0/14] init: deps: dependency based (parallelized) init Alexander Holler <holler@ahsoftware.de> - 2015-10-17 22:40 +0200
    Re: [PATCH 11/14] init: deps: annotate various initcalls Linus Torvalds <torvalds@linux-foundation.org> - 2015-10-17 20:50 +0200
      Re: [PATCH 11/14] init: deps: annotate various initcalls Alexander Holler <holler@ahsoftware.de> - 2015-10-17 21:10 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1249372 — [PATCH 02/14] init: deps: use annotated initcalls for a dependency based (optionally parallelized) init

Based on the dependencies provided by annotated initcalls, this patch
introduces a topological sort to sort initcalls and (optionally) uses
multiple threads to call initcalls.

If the feature is disabled, nothing changes.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 include/linux/init.h |   6 +
 init/.gitignore      |   1 +
 init/Kconfig.deps    |  38 +++++
 init/Makefile        |  15 ++
 init/dependencies.c  | 394 +++++++++++++++++++++++++++++++++++++++++++++++++++
 init/main.c          |  10 +-
 lib/Kconfig.debug    |   1 +
 7 files changed, 464 insertions(+), 1 deletion(-)
 create mode 100644 init/.gitignore
 create mode 100644 init/Kconfig.deps
 create mode 100644 init/dependencies.c

diff --git a/include/linux/init.h b/include/linux/init.h
index 758fd18..264f83f 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -160,6 +160,12 @@ extern void (*late_time_init)(void);
 
 extern bool initcall_debug;
 
+/* Defined in init/dependencies.c */
+void __init do_annotated_initcalls(void);
+
+/* id_dependency will be initialized before id */
+int __init add_initcall_dependency(unsigned id, unsigned id_dependency);
+
 #endif
   
 #ifndef MODULE
diff --git a/init/.gitignore b/init/.gitignore
new file mode 100644
index 0000000..38e6d06
--- /dev/null
+++ b/init/.gitignore
@@ -0,0 +1 @@
+driver_names.c
diff --git a/init/Kconfig.deps b/init/Kconfig.deps
new file mode 100644
index 0000000..9ced0d4
--- /dev/null
+++ b/init/Kconfig.deps
@@ -0,0 +1,38 @@
+config DEPENDENCIES
+	bool "Use dependency based initialization sequence (DO NOT USE)"
+	select ANNOTATED_INITCALLS
+	help
+	  This will likely crash your kernel at startup. You have been warned.
+	  That means you should make sure you have a working backup kernel
+	  you can boot from in case the kernel with this feature turned on
+	  crashes.
+	  In order to benefit from this feature, statically linked drivers
+	  have to provide dependencies.
+
+config DEPENDENCIES_PRINT_INIT_ORDER
+	bool "Print dependency based initialization order"
+	depends on DEPENDENCIES
+	help
+	  Used for debugging purposes.
+
+config DEPENDENCIES_PRINT_CALLS
+	bool "Show when annotated initcalls are actually called"
+	depends on DEPENDENCIES
+	help
+	  Used for debugging purposes.
+
+config DEPENDENCIES_PARALLEL
+	bool "Call annotated initcalls in parallel"
+	depends on DEPENDENCIES
+	help
+	  Calculates which (annotated) initcalls can be called in parallel
+	  and calls them using multiple threads.
+
+config DEPENDENCIES_THREADS
+	int "Number of threads to use for parallel initialization"
+	depends on DEPENDENCIES_PARALLEL
+	default 0
+	help
+	  0 means the number of threads used for parallel initialization
+	  of drivers equals the number of online CPUs.
+	  1 means the threaded initialization is disabled.
diff --git a/init/Makefile b/init/Makefile
index 7bc47ee..6a8c22c 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -9,6 +9,7 @@ else
 obj-$(CONFIG_BLK_DEV_INITRD)   += initramfs.o
 endif
 obj-$(CONFIG_GENERIC_CALIBRATE_DELAY) += calibrate.o
+obj-$(CONFIG_DEPENDENCIES) += dependencies.o
 
 ifneq ($(CONFIG_ARCH_INIT_TASK),y)
 obj-y                          += init_task.o
@@ -19,6 +20,20 @@ mounts-$(CONFIG_BLK_DEV_RAM)	+= do_mounts_rd.o
 mounts-$(CONFIG_BLK_DEV_INITRD)	+= do_mounts_initrd.o
 mounts-$(CONFIG_BLK_DEV_MD)	+= do_mounts_md.o
 
+quiet_cmd_make-driver_names = GEN     $@
+      cmd_make-driver_names = sed $< > $@ \
+		-e 's/^\tdrvid_\(.*\),/\t"\1",/' \
+		-e 's/^\tdrvid_max$$/\t"max"/' \
+		-e 's/^enum {/static const char *driver_names[] __initdata = {/' \
+		-e '/^\#ifndef _LINUX_DRIVER_IDS_H$$/d' \
+		-e '/^\#define _LINUX_DRIVER_IDS_H$$/d' \
+		-e '/^\#endif \/\* _LINUX_DRIVER_IDS_H \*\/$$/d'
+
+$(obj)/driver_names.c: $(srctree)/include/linux/driver_ids.h
+	$(call cmd,make-driver_names)
+
+$(obj)/dependencies.o: $(obj)/driver_names.c
+
 # dependencies on generated files need to be listed explicitly
 $(obj)/version.o: include/generated/compile.h
 
diff --git a/init/dependencies.c b/init/dependencies.c
new file mode 100644
index 0000000..c47817c
--- /dev/null
+++ b/init/dependencies.c
@@ -0,0 +1,394 @@
+/*
+ * Code for building a deterministic initialization order
+ * based on dependencies.
+ *
+ * Copyright (C) 2014 Alexander Holler <holler@ahsoftware.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+/* #define DEBUG */
+
+#include <linux/kthread.h>
+#include <linux/sort.h>
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/init.h>
+
+#if defined(CONFIG_DEPENDENCIES_PRINT_INIT_ORDER) \
+	|| defined(CONFIG_DEPENDENCIES_PRINT_CALLS)
+#include "driver_names.c"
+#endif
+
+#define MAX_VERTICES drvid_max /* maximum number of vertices */
+#define MAX_EDGES (MAX_VERTICES*5) /* maximum number of edges (dependencies) */
+
+struct edgenode {
+	unsigned y; /* initcall ID */
+#ifdef CONFIG_DEPENDENCIES_PARALLEL
+	unsigned x;
+#endif
+	struct edgenode *next; /* next edge in list */
+};
+
+/* Vertex numbers correspond to initcall IDs. */
+static struct edgenode edge_slots[MAX_EDGES] __initdata; /* avoid kmalloc */
+static struct edgenode *edges[MAX_VERTICES] __initdata; /* adjacency info */
+static unsigned nedges __initdata; /* number of edges */
+static unsigned nvertices __initdata; /* number of vertices */
+static bool processed[MAX_VERTICES] __initdata;
+static bool include_node[MAX_VERTICES] __initdata;
+static bool discovered[MAX_VERTICES] __initdata;
+
+static unsigned order[MAX_VERTICES] __initdata;
+static unsigned norder __initdata;
+static const struct _annotated_initcall
+		*annotated_initcall_by_drvid[MAX_VERTICES] __initdata;
+
+int __init add_initcall_dependency(unsigned id, unsigned id_dependency)
+{
+	struct edgenode *p;
+
+	if (!id || !id_dependency)
+		return 0; /* ignore root */
+	if (unlikely(nedges >= MAX_EDGES)) {
+		pr_err("init: maximum number of edges (%u) reached!\n",
+			MAX_EDGES);
+		return -EINVAL;
+	}
+	if (unlikely(id == id_dependency))
+		return 0;
+	if (!include_node[id] || !include_node[id_dependency])
+		return 0; /* ignore edges for initcalls not included */
+	p = &edge_slots[nedges++];
+	p->y = id_dependency;
+#ifdef CONFIG_DEPENDENCIES_PARALLEL
+	p->x = id;
+#endif
+	/* insert at head of list */
+	p->next = edges[id];
+	edges[id] = p;
+
+	return 0;
+}
+
+static int __init depth_first_search(unsigned v)
+{
+	struct edgenode *p;
+	unsigned y; /* successor vertex */
+
+	discovered[v] = 1;
+	p = edges[v];
+	while (p) {
+		y = p->y;
+		if (unlikely(discovered[y] && !processed[y])) {
+			pr_err("init: cycle found %u <-> %u!\n", v, y);
+			return -EINVAL;
+		}
+		if (!discovered[y] && depth_first_search(y))
+			return -EINVAL;
+		p = p->next;
+	}
+	order[norder++] = v;
+	processed[v] = 1;
+	return 0;
+}
+
+static int __init topological_sort(void)
+{
+	unsigned i;
+
+	for (i = 1; i <= nvertices; ++i)
+		if (!discovered[i] && include_node[i])
+			if (depth_first_search(i))
+				return -EINVAL;
+	return 0;
+}
+
+#ifdef CONFIG_DEPENDENCIES_PARALLEL
+/*
+ * The algorithm I've used below to calculate the max. distance for
+ * nodes to the root node likely isn't the fasted. But based on the
+ * already done implementation of the topological sort, this is an
+ * easy way to achieve this. Instead of first doing an topological
+ * sort and then using the stuff below to calculate the distances,
+ * using an algorithm which does spit out distances directly would
+ * be likely faster (also we are talking here about a few ms).
+ * If you want to spend the time, you could have a look e.g. at the
+ * topic 'layered graph drawing'.
+ */
+/* max. distance from a node to root */
+static unsigned distance[MAX_VERTICES] __initdata;
+static struct {
+	unsigned start;
+	unsigned length;
+} tgroup[20] __initdata;
+static unsigned count_groups __initdata;
+static __initdata DECLARE_COMPLETION(initcall_thread_done);
+static atomic_t shared_counter __initdata;
+static atomic_t count_initcall_threads __initdata;
+static atomic_t ostart __initdata;
+static atomic_t ocount __initdata;
+static atomic_t current_group __initdata;
+static unsigned num_threads __initdata;
+static __initdata DECLARE_WAIT_QUEUE_HEAD(group_waitqueue);
+
+static void __init calc_max_distance(uint32_t v)
+{
+	unsigned i;
+	unsigned max_dist = 0;
+
+	for (i = 0; i < nedges; ++i)
+		if (edge_slots[i].x == v)
+			max_dist = max(max_dist,
+				distance[edge_slots[i].y] + 1);
+	distance[v] = max_dist;
+}
+
+static void __init calc_distances(void)
+{
+	unsigned i;
+
+	for (i = 0; i < norder; ++i)
+		calc_max_distance(order[i]);
+}
+
+static int __init compare_by_distance(const void *lhs, const void *rhs)
+{
+	if (distance[*(unsigned *)lhs] < distance[*(unsigned *)rhs])
+		return -1;
+	if (distance[*(unsigned *)lhs] > distance[*(unsigned *)rhs])
+		return 1;
+	return 0;
+}
+
+static void __init build_order_by_distance(void)
+{
+	calc_distances();
+	sort(order, norder, sizeof(unsigned), &compare_by_distance, NULL);
+}
+
+static void __init build_tgroups(void)
+{
+	unsigned i;
+	unsigned dist = 0;
+
+	for (i = 0; i < norder; ++i) {
+		if (distance[order[i]] != dist) {
+			dist = distance[order[i]];
+			count_groups++;
+			tgroup[count_groups].start = i;
+		}
+		tgroup[count_groups].length++;
+	}
+	count_groups++;
+#ifdef DEBUG
+	for (i = 0; i < count_groups; ++i)
+		pr_info("init: group %u length %u (start %u)\n", i,
+				tgroup[i].length, tgroup[i].start);
+#endif
+}
+
+static int __init initcall_thread(void *thread_nr)
+{
+	int i;
+	unsigned group;
+	int start, count;
+	const struct _annotated_initcall *ac;
+	DEFINE_WAIT(wait);
+
+	while ((group = atomic_read(&current_group)) < count_groups) {
+		start = atomic_read(&ostart);
+		count = atomic_read(&ocount);
+		while ((i = atomic_dec_return(&shared_counter)) >= 0) {
+			ac = annotated_initcall_by_drvid[
+					order[start + count - 1 - i]];
+#ifdef CONFIG_DEPENDENCIES_PRINT_CALLS
+			pr_info("init: thread %lu calling initcall for driver %s (ID %u)\n",
+				(unsigned long)thread_nr,
+				driver_names[ac->id], ac->id);
+#endif
+			do_one_initcall(*ac->initcall);
+		}
+		prepare_to_wait(&group_waitqueue, &wait, TASK_UNINTERRUPTIBLE);
+		if (!atomic_dec_and_test(&count_initcall_threads)) {
+			/*
+			 * The current group was processed, sleep until the
+			 * last thread finished work on this group, changes
+			 * the group and wakes up all threads.
+			 */
+			schedule();
+			finish_wait(&group_waitqueue, &wait);
+			continue;
+		}
+		atomic_inc(&current_group);
+		atomic_set(&count_initcall_threads, num_threads);
+		if (++group >= count_groups) {
+			/*
+			 * All groups processed and all threads finished.
+			 * Prepare to process unordered annotated
+			 * initcalls and wake up other threads to call
+			 * them too.
+			 */
+			atomic_set(&shared_counter,
+				__annotated_initcall_end -
+					__annotated_initcall_start);
+			wake_up_all(&group_waitqueue);
+			finish_wait(&group_waitqueue, &wait);
+			break;
+		}
+		/*
+		 * Finalize the switch to the next group and wake up other
+		 * threads to process the new group too.
+		 */
+		pr_debug("init: thread %lu changes group\n",
+			(unsigned long)thread_nr);
+		atomic_set(&ostart, tgroup[group].start);
+		atomic_set(&ocount, tgroup[group].length);
+		atomic_set(&shared_counter, tgroup[group].length);
+		wake_up_all(&group_waitqueue);
+		finish_wait(&group_waitqueue, &wait);
+	}
+	if (atomic_dec_and_test(&count_initcall_threads))
+		complete(&initcall_thread_done);
+	do_exit(0);
+	return 0;
+}
+#else
+#define build_order_by_distance()
+#define build_tgroups()
+#endif /* CONFIG_DEPENDENCIES_PARALLEL */
+
+static void __init init_drivers_non_threaded(void)
+{
+	unsigned i;
+	const struct _annotated_initcall *ac;
+
+	for (i = 0; i < norder; ++i) {
+		ac = annotated_initcall_by_drvid[order[i]];
+#ifdef CONFIG_DEPENDENCIES_PRINT_CALLS
+		pr_info("init: calling initcall for driver %s (ID %u)\n",
+			driver_names[ac->id], ac->id);
+#endif
+		do_one_initcall(*ac->initcall);
+	}
+}
+
+static int __init add_dependencies(void)
+{
+	int rc;
+	const struct _annotated_initcall *ac;
+	const unsigned *dep;
+	unsigned i;
+
+	ac = __annotated_initcall_start;
+	for (; ac < __annotated_initcall_end; ++ac) {
+		dep = ac->dependencies;
+		if (dep)
+			for (i = 0; dep[i]; ++i) {
+				rc = add_initcall_dependency(ac->id, dep[i]);
+				if (unlikely(rc))
+					return rc;
+			}
+	}
+	return 0;
+}
+
+static void __init build_inventory(void)
+{
+	const struct _annotated_initcall *ac;
+
+	ac = __annotated_initcall_start;
+	for (; ac < __annotated_initcall_end; ++ac) {
+		include_node[ac->id] = true;
+		annotated_initcall_by_drvid[ac->id] = ac;
+		nvertices = max(nvertices, ac->id);
+	}
+}
+
+#ifdef CONFIG_DEPENDENCIES_PRINT_INIT_ORDER
+static void __init print_order(void)
+{
+	unsigned i;
+
+	pr_info("init: initialization order:\n");
+	for (i = 0; i < norder; ++i) {
+#ifdef CONFIG_DEPENDENCIES_PARALLEL
+		pr_info("init: %u (group %u) %s (ID %u)\n", i,
+			distance[order[i]], driver_names[order[i]], order[i]);
+#else
+		pr_info("init: %u %s (ID %u)\n", i,
+			driver_names[order[i]], order[i]);
+#endif
+	}
+}
+#else
+#define print_order()
+#endif
+
+static int __init build_order(void)
+{
+	int rc = 0;
+
+	build_inventory();
+	add_dependencies();
+	if (topological_sort())
+		return -EINVAL; /* cycle found */
+	pr_debug("init: vertices: %u edges %u count %u\n",
+					nvertices, nedges, norder);
+	build_order_by_distance();
+	build_tgroups();
+	print_order();
+	return rc;
+}
+
+void __init do_annotated_initcalls(void)
+{
+	unsigned i;
+
+	i = __annotated_initcall_end - __annotated_initcall_start;
+	if (!i)
+		return;
+
+	if (build_order()) {
+		/*
+		 * Building order failed (likely because of a dependency
+		 * circle). Try to boot anyway by calling all annotated
+		 * initcalls unordered.
+		 */
+		const struct _annotated_initcall *ac;
+
+		ac = __annotated_initcall_start;
+		for (; ac < __annotated_initcall_end; ++ac)
+			do_one_initcall(*ac->initcall);
+		return;
+	}
+
+#ifndef CONFIG_DEPENDENCIES_PARALLEL
+	init_drivers_non_threaded();
+#else
+	if (CONFIG_DEPENDENCIES_THREADS == 0)
+		num_threads = num_online_cpus();
+	else
+		num_threads = CONFIG_DEPENDENCIES_THREADS;
+	if (num_threads < 2) {
+		init_drivers_non_threaded();
+		return;
+	}
+	pr_debug("init: using %u threads to call annotated initcalls\n",
+				num_threads);
+	atomic_set(&count_initcall_threads, num_threads);
+	atomic_set(&ostart, tgroup[0].start);
+	atomic_set(&ocount, tgroup[0].length);
+	atomic_set(&shared_counter, tgroup[0].length);
+	atomic_set(&current_group, 0);
+	for (i = 0; i < num_threads; ++i)
+		kthread_run(initcall_thread, (void *)(unsigned long)i,
+			"initcalls");
+	wait_for_completion(&initcall_thread_done);
+	pr_debug("init: all threads done\n");
+#endif
+}
diff --git a/init/main.c b/init/main.c
index 5650655..f873c08 100644
--- a/init/main.c
+++ b/init/main.c
@@ -863,8 +863,16 @@ static void __init do_initcalls(void)
 {
 	int level;
 
-	for (level = 0; level < ARRAY_SIZE(initcall_levels) - 1; level++)
+	for (level = 0; level < ARRAY_SIZE(initcall_levels) - 3; level++)
 		do_initcall_level(level);
+#ifdef CONFIG_DEPENDENCIES
+	/* call annotated drivers (sorted) */
+	do_annotated_initcalls();
+#endif
+	/* call normal and not annoted drivers (not sorted) */
+	do_initcall_level(level++);
+	/* call late drivers */
+	do_initcall_level(level);
 }
 
 /*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index e2894b2..4815b15 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1844,3 +1844,4 @@ source "samples/Kconfig"
 
 source "lib/Kconfig.kgdb"
 
+source "init/Kconfig.deps"
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249373 — [PATCH 01/14] init: deps: introduce annotated initcalls

Make it possible to identify initcalls before calling them by adding an
ID, an optional pointer to a list of IDs the initcalls depends on and
an optional pointer to a struct device_driver.

This is e.g. necessary in order to sort initcalls by whatever means
before calling them.

To annotate an initcall, the following changes are necessary on
drivers which want to offer that feature:

now		annotated
------------------------------------------------------------------------
pure_initcall(fn)
		annotated_initcall(pure, fn, id, dependencies) or
		annotated_initcall_drv(pure, fn, id, dependencies, drv)
core_initcall(fn)
		annotated_initcall(core, fn, id, dependencies) or
		annotated_initcall_drv(core, fn, id, dependencies, drv)
core_initcall_sync(fn)
		annotated_initcall_sync(core, fn, id, dependencies) or
		annotated_initcall_sync_drv(core, fn, id, dependencies,
								drv)
(...)
late_initcall(fn)
		annotated_initcall(late, fn, id, dependencies)
module_init(fn)
		annotated_module_init(fn, id, dependencies)

module_platform_driver(drv)
		annotated_module_platform_driver(drv, id, dependencies)
module_platform_driver_probe(drv, probe)
		annotated:module_platform_driver_probe(drv, probe, id,
							 dependencies)
module_i2c_driver(i2c_drv)
		annotated_module_i2c_driver(i2c_drv, id, dependencies)
module_usb_driver(usb_drv)
		annotated_module_usb_driver(usb_drv, id, dependencies)
module_phy_driver(__phy_drivers)
		annotated_module_phy_driver(__phy_drivers, id,
						dependencies)
module_pci_driver(pci_driver)
		annotated_module_pci_driver(pci_driver, id,
						dependencies)
module_serio_driver(serio_driver)
		annotated_module_serio_driver(serio_driver, id,
						dependencies)
module_acpi_driver(acpi_driver)
		annotated_module_acpi_driver(acpi_driver, id,
						dependencies)

E.g. to make the driver sram offering an annotated initcall the
following patch is necessary:

----
-postcore_initcall(sram_init);
+annotated_initcall_drv(postcore, sram_init, drvid_sram, NULL,
+			sram_driver.driver);
----

The change for a module with dependencies might look like:

----
-module_platform_driver(gpio_led_driver);
+static const unsigned dependencies[] __initdata __maybe_unused = {
+	drvid_leds,
+	0
+};
+
+annotated_module_platform_driver(gpio_led_driver, drvid_gpio_led,
+					dependencies);
----

These changes can be done without any fear. If the feature is disabled,
which is the default, the new macros will just map to the old ones and
nothing is changed at all.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 arch/arm/kernel/vmlinux.lds.S     |  1 +
 arch/arm/mach-omap2/soc.h         | 10 ++++++-
 drivers/usb/storage/usb.h         | 14 ++++++++++
 include/acpi/acpi_bus.h           | 13 +++++++++
 include/asm-generic/vmlinux.lds.h |  7 +++++
 include/linux/device.h            | 14 ++++++++++
 include/linux/driver_ids.h        | 21 ++++++++++++++
 include/linux/i2c.h               |  4 +++
 include/linux/init.h              | 58 +++++++++++++++++++++++++++++++++++++++
 include/linux/module.h            |  7 +++++
 include/linux/pci.h               |  4 +++
 include/linux/phy.h               | 16 +++++++++++
 include/linux/platform_device.h   | 41 +++++++++++++++++++++++++--
 include/linux/serio.h             |  5 ++++
 include/linux/usb.h               | 12 ++++++++
 init/Kconfig                      |  3 ++
 16 files changed, 226 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/driver_ids.h

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde..c22784e 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -213,6 +213,7 @@ SECTIONS
 #endif
 		INIT_SETUP(16)
 		INIT_CALLS
+		ANNOTATED_INITCALLS
 		CON_INITCALL
 		SECURITY_INITCALL
 		INIT_RAM_FS
diff --git a/arch/arm/mach-omap2/soc.h b/arch/arm/mach-omap2/soc.h
index f97654d..75f7012 100644
--- a/arch/arm/mach-omap2/soc.h
+++ b/arch/arm/mach-omap2/soc.h
@@ -554,5 +554,13 @@ level(__##fn);
 #define omap_late_initcall(fn)		omap_initcall(late_initcall, fn)
 #define omap_late_initcall_sync(fn)	omap_initcall(late_initcall_sync, fn)
 
-#endif	/* __ASSEMBLY__ */
+#define annotated_omap_initcall(level, fn, id, deps)	\
+static int __init __used __##fn(void)		\
+{						\
+	if (!soc_is_omap())			\
+		return 0;			\
+	return fn();				\
+}						\
+annotated_initcall(level, __##fn, id, deps)
 
+#endif	/* __ASSEMBLY__ */
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index da0ad32..f2ed368 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -218,4 +218,18 @@ static void __exit __driver##_exit(void) \
 } \
 module_exit(__driver##_exit)
 
+#define annotated_module_usb_stor_driver(__driver, __sht, __name, \
+					 __id, __deps) \
+static int __init __driver##_init(void) \
+{ \
+	usb_stor_host_template_init(&(__sht), __name, THIS_MODULE); \
+	return usb_register(&(__driver)); \
+} \
+annotated_module_init(__driver##_init, __id, __deps); \
+static void __exit __driver##_exit(void) \
+{ \
+	usb_deregister(&(__driver)); \
+} \
+module_exit(__driver##_exit)
+
 #endif
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 83061ca..42d0612 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -541,6 +541,19 @@ static inline bool acpi_device_enumerated(struct acpi_device *adev)
 	module_driver(__acpi_driver, acpi_bus_register_driver, \
 		      acpi_bus_unregister_driver)
 
+#define annotated_module_acpi_driver(__acpi_driver, __id, __deps) \
+static int __init __acpi_driver##_init(void) \
+{ \
+	return acpi_bus_register_driver(&(__acpi_driver)); \
+} \
+annotated_module_init_drv(__acpi_driver##_init, __id, __deps, \
+			  __acpi_driver.drv); \
+static void __exit __acpi_driver##_exit(void) \
+{ \
+	acpi_bus_unregister_driver(&(__acpi_driver)); \
+} \
+module_exit(__acpi_driver##_exit)
+
 /*
  * Bind physical devices with ACPI devices
  */
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8bd374d..ad6cace 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -660,6 +660,12 @@
 		INIT_CALLS_LEVEL(7)					\
 		VMLINUX_SYMBOL(__initcall_end) = .;
 
+#define ANNOTATED_INITCALLS						\
+		. = ALIGN(32);						\
+		VMLINUX_SYMBOL(__annotated_initcall_start) = .;		\
+		*(.annotated_initcall.init)				\
+		VMLINUX_SYMBOL(__annotated_initcall_end) = .;
+
 #define CON_INITCALL							\
 		VMLINUX_SYMBOL(__con_initcall_start) = .;		\
 		*(.con_initcall.init)					\
@@ -816,6 +822,7 @@
 		INIT_DATA						\
 		INIT_SETUP(initsetup_align)				\
 		INIT_CALLS						\
+		ANNOTATED_INITCALLS					\
 		CON_INITCALL						\
 		SECURITY_INITCALL					\
 		INIT_RAM_FS						\
diff --git a/include/linux/device.h b/include/linux/device.h
index a2b4ea7..7320dc9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1321,4 +1321,18 @@ static int __init __driver##_init(void) \
 } \
 device_initcall(__driver##_init);
 
+#define annotated_module_driver(__driver, __register, __unregister, \
+				__id, __deps, ...) \
+static int __init __driver##_init(void) \
+{ \
+	return __register(&(__driver), ##__VA_ARGS__); \
+} \
+annotated_module_init_drv(__driver##_init, __id, __deps, __driver.driver); \
+static void __exit __driver##_exit(void) \
+{ \
+	__unregister(&(__driver), ##__VA_ARGS__); \
+} \
+module_exit(__driver##_exit)
+
+
 #endif /* _DEVICE_H_ */
diff --git a/include/linux/driver_ids.h b/include/linux/driver_ids.h
new file mode 100644
index 0000000..60964fe
--- /dev/null
+++ b/include/linux/driver_ids.h
@@ -0,0 +1,21 @@
+#ifndef _LINUX_DRIVER_IDS_H
+#define _LINUX_DRIVER_IDS_H
+
+/*
+ * In fact, the IDs listed here are IDs for initcalls, and not for
+ * drivers. But most of the time, a driver or subsystem has only one
+ * initcall, and talking about IDs for drivers makes more sense than
+ * talking about initcalls, something many people have no clear
+ * understanding about.
+ *
+ * Please use the name of the module as the name for the ID if
+ * something can be build as a module.
+ */
+
+enum {
+	drvid_unused,
+	/* To be filled */
+	drvid_max
+};
+
+#endif /* _LINUX_DRIVER_IDS_H */
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e83a738..efe0e7d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -629,6 +629,10 @@ static inline int i2c_adapter_id(struct i2c_adapter *adap)
 	module_driver(__i2c_driver, i2c_add_driver, \
 			i2c_del_driver)
 
+#define annotated_module_i2c_driver(__i2c_driver, __id, __deps) \
+	annotated_module_driver(__i2c_driver, i2c_add_driver, \
+			i2c_del_driver, __id, __deps)
+
 #endif /* I2C */
 
 #if IS_ENABLED(CONFIG_OF)
diff --git a/include/linux/init.h b/include/linux/init.h
index b449f37..758fd18 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -3,6 +3,9 @@
 
 #include <linux/compiler.h>
 #include <linux/types.h>
+#ifndef __ASSEMBLY__
+#include <linux/driver_ids.h>
+#endif
 
 /* These macros are used to mark some functions or 
  * initialized data (doesn't apply to uninitialized data)
@@ -124,6 +127,17 @@
 typedef int (*initcall_t)(void);
 typedef void (*exitcall_t)(void);
 
+struct device_driver;
+
+struct _annotated_initcall {
+	const initcall_t initcall;
+	const unsigned id; /* from driver_ids.h */
+	const unsigned *dependencies;
+	const struct device_driver *driver;
+};
+extern const struct _annotated_initcall __annotated_initcall_start[],
+				  __annotated_initcall_end[];
+
 extern initcall_t __con_initcall_start[], __con_initcall_end[];
 extern initcall_t __security_initcall_start[], __security_initcall_end[];
 
@@ -184,6 +198,18 @@ extern bool initcall_debug;
 	__attribute__((__section__(".initcall" #id ".init"))) = fn; \
 	LTO_REFERENCE_INITCALL(__initcall_##fn##id)
 
+#define __define_annotated_initcall(fn, __id, deps) \
+	static struct _annotated_initcall __annotated_initcall_##fn __used \
+	__attribute__((__section__(".annotated_initcall.init"))) = \
+		{ .initcall = fn, .id = __id, .dependencies = deps, \
+		  .driver = NULL }
+
+#define __define_annotated_initcall_drv(fn, __id, deps, drv) \
+	static struct _annotated_initcall __annotated_initcall_##fn __used \
+	__attribute__((__section__(".annotated_initcall.init"))) = \
+		{ .initcall = fn, .id = __id, .dependencies = deps, \
+		  .driver = &(drv) }
+
 /*
  * Early initcalls run before initializing SMP.
  *
@@ -216,6 +242,38 @@ extern bool initcall_debug;
 #define late_initcall(fn)		__define_initcall(fn, 7)
 #define late_initcall_sync(fn)		__define_initcall(fn, 7s)
 
+/*
+ * Annotated initcalls are accompanied by a struct device_driver.
+ * This makes initcalls identifiable and is used to order initcalls.
+ *
+ * If disabled, nothing is changed and the classic level based
+ * initialization sequence is in use.
+ */
+#ifdef CONFIG_ANNOTATED_INITCALLS
+#define annotated_module_init(fn, id, deps) \
+	__define_annotated_initcall(fn, id, deps)
+#define annotated_module_init_drv(fn, id, deps, drv) \
+	__define_annotated_initcall_drv(fn, id, deps, drv)
+#define annotated_initcall(level, fn, id, deps) \
+	__define_annotated_initcall(fn, id, deps)
+#define annotated_initcall_sync(level, fn, id, deps) \
+	__define_annotated_initcall(fn, id, deps)
+#define annotated_initcall_drv(level, fn, id, deps, drv) \
+	__define_annotated_initcall_drv(fn, id, deps, drv)
+#define annotated_initcall_drv_sync(level, fn, id, deps, drv) \
+	__define_annotated_initcall_drv(fn, id, deps, drv)
+#else
+#define annotated_module_init(fn, id, deps)	module_init(fn)
+#define annotated_module_init_drv(fn, id, deps, drv)	module_init(fn)
+#define annotated_initcall(level, fn, id, deps)	level ## _initcall(fn)
+#define annotated_initcall_sync(level, fn, id, deps) \
+	level ## _initcall_sync(fn)
+#define annotated_initcall_drv(level, fn, id, deps, drv) \
+	level ## _initcall(fn)
+#define annotated_initcall_drv_sync(level, fn, id, deps, drv) \
+	level ## _initcall_sync(fn)
+#endif
+
 #define __initcall(fn) device_initcall(fn)
 
 #define __exitcall(fn) \
diff --git a/include/linux/module.h b/include/linux/module.h
index 3a19c79..880fdda 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -120,6 +120,13 @@ extern void cleanup_module(void);
 #define late_initcall(fn)		module_init(fn)
 #define late_initcall_sync(fn)		module_init(fn)
 
+#define annotated_initcall(level, fn, id, deps)	module_init(fn)
+#define annotated_initcall_sync(level, fn, id, deps)	module_init(fn)
+#define annotated_initcall_drv(level, fn, id, deps, drv)	module_init(fn)
+#define annotated_initcall_drv_sync(level, fn, id, deps, drv)	module_init(fn)
+#define annotated_module_init(fn, id, deps)	module_init(fn)
+#define annotated_module_init_drv(fn, id, deps, drv)	module_init(fn)
+
 #define console_initcall(fn)		module_init(fn)
 #define security_initcall(fn)		module_init(fn)
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1d4eb60..8093898 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1162,6 +1162,10 @@ void pci_unregister_driver(struct pci_driver *dev);
 	module_driver(__pci_driver, pci_register_driver, \
 		       pci_unregister_driver)
 
+#define annotated_module_pci_driver(__pci_driver, __id, __dependencies) \
+	annotated_module_driver(__pci_driver, pci_register_driver, \
+		       pci_unregister_driver, __id, __dependencies)
+
 struct pci_driver *pci_dev_driver(const struct pci_dev *dev);
 int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int vendor, unsigned int device,
diff --git a/include/linux/phy.h b/include/linux/phy.h
index a26c3f8..e5377fb 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -824,4 +824,20 @@ module_exit(phy_module_exit)
 #define module_phy_driver(__phy_drivers)				\
 	phy_module_driver(__phy_drivers, ARRAY_SIZE(__phy_drivers))
 
+#define annotated_phy_module_driver(__phy_drivers, __count, id, deps)	\
+static int __init phy_module_init(void)					\
+{									\
+	return phy_drivers_register(__phy_drivers, __count);		\
+}									\
+annotated_module_init(phy_module_init, id, deps);			\
+static void __exit phy_module_exit(void)				\
+{									\
+	phy_drivers_unregister(__phy_drivers, __count);			\
+}									\
+module_exit(phy_module_exit)
+
+#define annotated_module_phy_driver(__phy_drivers, id, deps)		\
+	annotated_phy_module_driver(__phy_drivers,			\
+				    ARRAY_SIZE(__phy_drivers), id, deps)
+
 #endif /* __PHY_H */
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index bba08f4..10452cb 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -218,9 +218,29 @@ static inline void platform_set_drvdata(struct platform_device *pdev,
  * boilerplate.  Each module may only use this macro once, and
  * calling it replaces module_init() and module_exit()
  */
-#define module_platform_driver(__platform_driver) \
-	module_driver(__platform_driver, platform_driver_register, \
-			platform_driver_unregister)
+#define module_platform_driver(__driver) \
+static int __init __driver##_init(void) \
+{ \
+	return platform_driver_register(&(__driver)); \
+} \
+module_init(__driver##_init); \
+static void __exit __driver##_exit(void) \
+{ \
+	platform_driver_unregister(&(__driver)); \
+} \
+module_exit(__driver##_exit)
+
+#define annotated_module_platform_driver(__driver, __id, __deps) \
+static int __init __driver##_init(void) \
+{ \
+	return platform_driver_register(&(__driver)); \
+} \
+annotated_module_init_drv(__driver##_init, __id, __deps, __driver.driver); \
+static void __exit __driver##_exit(void) \
+{ \
+	platform_driver_unregister(&(__driver)); \
+} \
+module_exit(__driver##_exit)
 
 /* builtin_platform_driver() - Helper macro for builtin drivers that
  * don't do anything special in driver init.  This eliminates some
@@ -249,6 +269,21 @@ static void __exit __platform_driver##_exit(void) \
 } \
 module_exit(__platform_driver##_exit);
 
+#define annotated_module_platform_driver_probe(__platform_driver, \
+				__platform_probe, __id, __deps) \
+static int __init __platform_driver##_init(void) \
+{ \
+	return platform_driver_probe(&(__platform_driver), \
+				     __platform_probe);    \
+} \
+annotated_module_init_drv(__platform_driver##_init, __id, __deps, \
+			  __platform_driver.driver); \
+static void __exit __platform_driver##_exit(void) \
+{ \
+	platform_driver_unregister(&(__platform_driver)); \
+} \
+module_exit(__platform_driver##_exit)
+
 /* builtin_platform_driver_probe() - Helper macro for drivers that don't do
  * anything special in device init.  This eliminates some boilerplate.  Each
  * driver may only use this macro once, and using it replaces device_initcall.
diff --git a/include/linux/serio.h b/include/linux/serio.h
index 9f779c7..e5a9ef6 100644
--- a/include/linux/serio.h
+++ b/include/linux/serio.h
@@ -105,6 +105,11 @@ void serio_unregister_driver(struct serio_driver *drv);
 	module_driver(__serio_driver, serio_register_driver, \
 		       serio_unregister_driver)
 
+#define annotated_module_serio_driver(__serio_driver, id, deps) \
+	annotated_module_driver(__serio_driver, serio_register_driver, \
+		       serio_unregister_driver, id, deps)
+
+
 static inline int serio_write(struct serio *serio, unsigned char data)
 {
 	if (serio->write)
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 447fe29..f8d7a15 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1187,6 +1187,18 @@ extern void usb_deregister(struct usb_driver *);
 	module_driver(__usb_driver, usb_register, \
 		       usb_deregister)
 
+#define annotated_module_usb_driver(__usb_driver, __id, __deps) \
+static int __init __usb_driver##_init(void) \
+{ \
+	return usb_register(&(__usb_driver)); \
+} \
+annotated_module_init(__usb_driver##_init, __id, __deps); \
+static void __exit __usb_driver##_exit(void) \
+{ \
+	usb_deregister(&(__usb_driver)); \
+} \
+module_exit(__usb_driver##_exit)
+
 extern int usb_register_device_driver(struct usb_device_driver *,
 			struct module *);
 extern void usb_deregister_device_driver(struct usb_device_driver *);
diff --git a/init/Kconfig b/init/Kconfig
index af09b4f..5cfd7c4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -26,6 +26,9 @@ config IRQ_WORK
 config BUILDTIME_EXTABLE_SORT
 	bool
 
+config ANNOTATED_INITCALLS
+	bool
+
 menu "General setup"
 
 config BROKEN
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249380 — [PATCH 14/14] dt: dts: deps: omap: beagle: make some remote-endpoints non-dependencies

This is necessary to remove dependency cycles introduced by
'remote-endpoints'.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 arch/arm/boot/dts/omap3-beagle.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index a547411..78ba39e 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -101,6 +101,7 @@
 
 				tfp410_in: endpoint@0 {
 					remote-endpoint = <&dpi_out>;
+					no-dependencies = <&dpi_out>;
 				};
 			};
 
@@ -109,6 +110,7 @@
 
 				tfp410_out: endpoint@0 {
 					remote-endpoint = <&dvi_connector_in>;
+					no-dependencies = <&dvi_connector_in>;
 				};
 			};
 		};
@@ -150,6 +152,7 @@
 			etb_in: endpoint {
 				slave-mode;
 				remote-endpoint = <&etm_out>;
+				no-dependencies = <&etm_out>;
 			};
 		};
 	};
@@ -373,6 +376,7 @@
 	port {
 		venc_out: endpoint {
 			remote-endpoint = <&tv_connector_in>;
+			no-dependencies = <&tv_connector_in>;
 			ti,channels = <2>;
 		};
 	};
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249390 — [PATCH 13/14] dt: dts: deps: imx6q: make some remote-endpoints non-dependencies

This is necessary to remove dependency cycles introduced by
'remote-endpoints'.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 arch/arm/boot/dts/imx6q.dtsi   | 2 ++
 arch/arm/boot/dts/imx6qdl.dtsi | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q.dtsi b/arch/arm/boot/dts/imx6q.dtsi
index 399103b..8db7f25 100644
--- a/arch/arm/boot/dts/imx6q.dtsi
+++ b/arch/arm/boot/dts/imx6q.dtsi
@@ -184,6 +184,7 @@
 
 				ipu2_di0_hdmi: endpoint@1 {
 					remote-endpoint = <&hdmi_mux_2>;
+					no-dependencies = <&hdmi_mux_2>;
 				};
 
 				ipu2_di0_mipi: endpoint@2 {
@@ -205,6 +206,7 @@
 
 				ipu2_di1_hdmi: endpoint@1 {
 					remote-endpoint = <&hdmi_mux_3>;
+					no-dependencies = <&hdmi_mux_3>;
 				};
 
 				ipu2_di1_mipi: endpoint@2 {
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index b57033e..db3d0d0 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -1150,10 +1150,12 @@
 
 				ipu1_di0_hdmi: endpoint@1 {
 					remote-endpoint = <&hdmi_mux_0>;
+					no-dependencies = <&hdmi_mux_0>;
 				};
 
 				ipu1_di0_mipi: endpoint@2 {
 					remote-endpoint = <&mipi_mux_0>;
+					no-dependencies = <&mipi_mux_0>;
 				};
 
 				ipu1_di0_lvds0: endpoint@3 {
@@ -1175,10 +1177,12 @@
 
 				ipu1_di1_hdmi: endpoint@1 {
 					remote-endpoint = <&hdmi_mux_1>;
+					no-dependencies = <&hdmi_mux_1>;
 				};
 
 				ipu1_di1_mipi: endpoint@2 {
 					remote-endpoint = <&mipi_mux_1>;
+					no-dependencies = <&mipi_mux_1>;
 				};
 
 				ipu1_di1_lvds0: endpoint@3 {
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249396 — [PATCH 12/14] dt: dts: deps: kirkwood: dockstar: add dependency ehci -> usb power regulator

This serves as an example how easy it is to fix an initialization order
if the order depends on the DT. No source code changes will be necessary.

If you look at the dependency graph for the dockstar, you will see that
there is no dependency between ehci and the usb power regulator. This
ends up with the fact that the regulator will be initialized after ehci.

Fix this by adding one dependency to the .dts.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 arch/arm/boot/dts/kirkwood-dockstar.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/kirkwood-dockstar.dts b/arch/arm/boot/dts/kirkwood-dockstar.dts
index 8497363..426d8840 100644
--- a/arch/arm/boot/dts/kirkwood-dockstar.dts
+++ b/arch/arm/boot/dts/kirkwood-dockstar.dts
@@ -107,3 +107,7 @@
 		phy-handle = <&ethphy0>;
 	};
 };
+
+&usb0 {
+	dependencies = <&usb_power>;
+};
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249403

On Sat, Oct 17, 2015 at 07:14:13PM +0200, Alexander Holler wrote:
> Hello,
> 
> here is the newest version of my patches to use a dependency based
> initialization order. It now works without DT too.
> 
> Background:
> 
> Currently initcalls are ordered by some levels and the link order. This
> means whenever a file is renamed, changes directory or a Makefile is
> modified the order with which initcalls are called might change. This
> might result in problems. Furthermore, the required dependencies are
> often not documented, sometimes there are comments in the source or in a
> commit message, but most often the knowledge why a specific initcall
> belongs to a specific initcall level isn't obvious without carefully
> examing he source. And initcalls are used by drivers and subsystems, and
> the count of both have grown quiet a lot in the last years. So it's
> rather difficult to maintain a proper link order.

Files move around very rarely, is this really an issue?

> Another problem is that the link order can't be modified dynamically at
> boot time to include dependencies dictated by the hardware. To circumvent
> this, a brute-force trial-and-error mechanism called deferred probes has
> been introduced, but this approach, while beeing KISS, has its own
> problems.

What problems does deferred probing have?  Why not just fix that if
there is issues with it, as it was supposed to solve this issue without
needing to annotate anything.

> To solve these problems I've written patches to use a topological sort at
> boot time which uses dependencies to calculate the order with which
> initcalls are called.
> 
> Why? What are the benefits (assuming correct dependencies are available)?
> 
> - It offers a clear in-source documentation for dependencies between
>   initcalls.
> - It is robust in regard to file or directory name changes and changes in
>   a Makefile.
> - If enabled, the order with which drivers for interfaces are called
>   (e.g. network interfaces, hard disks), can be defined independent of
>   the link order. These might result in more stable interface names or
>   numbers.
> - If enabled, it makes the the deferred probes obsolete, which might
>   result in faster boot times.
> - If enabled, it is possible to call initcalls in parallel. E.g. the
>   shipped kernel for Fedora 21 (4.1.7-100.fc21.x86_64) contains around
>   560 initcalls. These are all called in series. Also some of them use
>   asynchronous stuff by themself, most don't do.

But that shipped kernel boots to X in less than 2 seconds, so there
isn't really a speed issue here, right?

> Drawbacks:
> 
> - It requires a small amount of time to calculate the order a boot time.
>   But this time is most often smaller than the time saved by using
>   multiple cores to call initcalls or by not needing deferred probes.

How much time is needed?

> - Dependencies are required. For everything which can be build as a
>   module, looking at modules.dep might give some pointers. Looking at
>   the help from menuconfig also might give some pointers. But in the
>   end, the most preferable way would be if maintainers or other people
>   which have a deeper knowledge about the source and functionality
>   would add the dependencies.

How will a "normal" driver author figure out what those dependancies are
in order to be able to write them down?  That's my biggest objection
here, I have no idea how to add these, nor how to properly review such a
submission.  What about systems that have different ordering/dependancy
requirements for the same drivers due to different ways the hardware is
hooked up?  That is not going to work well here, unless I'm missing
something.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249410

Am 17.10.2015 um 19:44 schrieb Greg Kroah-Hartman:
> On Sat, Oct 17, 2015 at 07:14:13PM +0200, Alexander Holler wrote:
>> Hello,
>>
>> here is the newest version of my patches to use a dependency based
>> initialization order. It now works without DT too.
>>
>> Background:
>>
>> Currently initcalls are ordered by some levels and the link order. This
>> means whenever a file is renamed, changes directory or a Makefile is
>> modified the order with which initcalls are called might change. This
>> might result in problems. Furthermore, the required dependencies are
>> often not documented, sometimes there are comments in the source or in a
>> commit message, but most often the knowledge why a specific initcall
>> belongs to a specific initcall level isn't obvious without carefully
>> examing he source. And initcalls are used by drivers and subsystems, and
>> the count of both have grown quiet a lot in the last years. So it's
>> rather difficult to maintain a proper link order.
>
> Files move around very rarely, is this really an issue?

No idea. You are maintaining the staging area. ;)

>
>> Another problem is that the link order can't be modified dynamically at
>> boot time to include dependencies dictated by the hardware. To circumvent
>> this, a brute-force trial-and-error mechanism called deferred probes has
>> been introduced, but this approach, while beeing KISS, has its own
>> problems.
>
> What problems does deferred probing have?  Why not just fix that if
> there is issues with it, as it was supposed to solve this issue without
> needing to annotate anything.

I've not looked why deferred probes are sometimes causing such a large 
delay. But giving that it's brutforce and non-deterministic, some 
drivers might be probed a dozen times, or important drivers might be 
probed very late, forcing all previous probed drivers to be probed again 
(late). Just think at the case the link order is a, b, c but drivers 
have to be called in the order c, b, a.

>> To solve these problems I've written patches to use a topological sort at
>> boot time which uses dependencies to calculate the order with which
>> initcalls are called.
>>
>> Why? What are the benefits (assuming correct dependencies are available)?
>>
>> - It offers a clear in-source documentation for dependencies between
>>    initcalls.
>> - It is robust in regard to file or directory name changes and changes in
>>    a Makefile.
>> - If enabled, the order with which drivers for interfaces are called
>>    (e.g. network interfaces, hard disks), can be defined independent of
>>    the link order. These might result in more stable interface names or
>>    numbers.
>> - If enabled, it makes the the deferred probes obsolete, which might
>>    result in faster boot times.
>> - If enabled, it is possible to call initcalls in parallel. E.g. the
>>    shipped kernel for Fedora 21 (4.1.7-100.fc21.x86_64) contains around
>>    560 initcalls. These are all called in series. Also some of them use
>>    asynchronous stuff by themself, most don't do.
>
> But that shipped kernel boots to X in less than 2 seconds, so there
> isn't really a speed issue here, right?

It's noticeable if your phone, or any other thing you want to use (like 
your route planner, clock or whatever boots in one second instead of two 
when you turn it on.

>> Drawbacks:
>>
>> - It requires a small amount of time to calculate the order a boot time.
>>    But this time is most often smaller than the time saved by using
>>    multiple cores to call initcalls or by not needing deferred probes.
>
> How much time is needed?

I've measured 3ms on a slow ARM box.

>
>> - Dependencies are required. For everything which can be build as a
>>    module, looking at modules.dep might give some pointers. Looking at
>>    the help from menuconfig also might give some pointers. But in the
>>    end, the most preferable way would be if maintainers or other people
>>    which have a deeper knowledge about the source and functionality
>>    would add the dependencies.
>
> How will a "normal" driver author figure out what those dependancies are
> in order to be able to write them down?  That's my biggest objection

Most drivers are done c&p from an existing driver. And if someone adds 
new code, he should know what these new code is for and on what it depends.

> here, I have no idea how to add these, nor how to properly review such a
> submission.  What about systems that have different ordering/dependancy
> requirements for the same drivers due to different ways the hardware is
> hooked up?  That is not going to work well here, unless I'm missing
> something.

Hmm, how is that solved now?

If you have dependencies dictated by a special HW, this dependencies 
should come in by the HW description.

The deferred probe mechanism exists just since 1 or 2 years. And once 
you had to search the source of non-displayed error in a set of several 
dozens possible sources (because many errors are now non-errors but 
-517) you might learn to hate deferred probes as much as I do. I'm sorry 
for the harsh words in regard to deferred probes.

The way to use dependencies doesn't add new requirements, it just moves 
them away from the link order. Besides that these dependencies are 
making it possible to parallelize the stuff.

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249414

On Sat, Oct 17, 2015 at 08:19:09PM +0200, Alexander Holler wrote:
> >>Another problem is that the link order can't be modified dynamically at
> >>boot time to include dependencies dictated by the hardware. To circumvent
> >>this, a brute-force trial-and-error mechanism called deferred probes has
> >>been introduced, but this approach, while beeing KISS, has its own
> >>problems.
> >
> >What problems does deferred probing have?  Why not just fix that if
> >there is issues with it, as it was supposed to solve this issue without
> >needing to annotate anything.
> 
> I've not looked why deferred probes are sometimes causing such a large
> delay. But giving that it's brutforce and non-deterministic, some drivers
> might be probed a dozen times, or important drivers might be probed very
> late, forcing all previous probed drivers to be probed again (late). Just
> think at the case the link order is a, b, c but drivers have to be called in
> the order c, b, a.

So how long does that really take to call all probe functions in all
possible order?  Real numbers please.  We have the tools to determine
where at boot time delays are happening, please use them to find the
problem drivers.

> >>To solve these problems I've written patches to use a topological sort at
> >>boot time which uses dependencies to calculate the order with which
> >>initcalls are called.
> >>
> >>Why? What are the benefits (assuming correct dependencies are available)?
> >>
> >>- It offers a clear in-source documentation for dependencies between
> >>   initcalls.
> >>- It is robust in regard to file or directory name changes and changes in
> >>   a Makefile.
> >>- If enabled, the order with which drivers for interfaces are called
> >>   (e.g. network interfaces, hard disks), can be defined independent of
> >>   the link order. These might result in more stable interface names or
> >>   numbers.
> >>- If enabled, it makes the the deferred probes obsolete, which might
> >>   result in faster boot times.
> >>- If enabled, it is possible to call initcalls in parallel. E.g. the
> >>   shipped kernel for Fedora 21 (4.1.7-100.fc21.x86_64) contains around
> >>   560 initcalls. These are all called in series. Also some of them use
> >>   asynchronous stuff by themself, most don't do.
> >
> >But that shipped kernel boots to X in less than 2 seconds, so there
> >isn't really a speed issue here, right?
> 
> It's noticeable if your phone, or any other thing you want to use (like your
> route planner, clock or whatever boots in one second instead of two when you
> turn it on.

My phone's kernel boots in less time than I can notice it, it's
userspace that takes forever to start up.  And there are other solutions
for that, look at what Sony has done for years with the cameras they
ship with Linux on them and boot insanely quick.

Again, real numbers please, show us where in the current scheme we are
taking too much time and we can work to resolve that.

> >>Drawbacks:
> >>
> >>- It requires a small amount of time to calculate the order a boot time.
> >>   But this time is most often smaller than the time saved by using
> >>   multiple cores to call initcalls or by not needing deferred probes.
> >
> >How much time is needed?
> 
> I've measured 3ms on a slow ARM box.

And how much time does deferred probe take in comparison?

> >>- Dependencies are required. For everything which can be build as a
> >>   module, looking at modules.dep might give some pointers. Looking at
> >>   the help from menuconfig also might give some pointers. But in the
> >>   end, the most preferable way would be if maintainers or other people
> >>   which have a deeper knowledge about the source and functionality
> >>   would add the dependencies.
> >
> >How will a "normal" driver author figure out what those dependancies are
> >in order to be able to write them down?  That's my biggest objection
> 
> Most drivers are done c&p from an existing driver. And if someone adds new
> code, he should know what these new code is for and on what it depends.

Trust me, as someone who reviews more new drivers than anyone else,
people don't know, they blindly cut-and-paste from other drivers and
don't stop to think what they are doing.

> >here, I have no idea how to add these, nor how to properly review such a
> >submission.  What about systems that have different ordering/dependancy
> >requirements for the same drivers due to different ways the hardware is
> >hooked up?  That is not going to work well here, unless I'm missing
> >something.
> 
> Hmm, how is that solved now?
> 
> If you have dependencies dictated by a special HW, this dependencies should
> come in by the HW description.

Yes, device tree shows this.

> The deferred probe mechanism exists just since 1 or 2 years. And once you
> had to search the source of non-displayed error in a set of several dozens
> possible sources (because many errors are now non-errors but -517) you might
> learn to hate deferred probes as much as I do. I'm sorry for the harsh words
> in regard to deferred probes.

I don't like it much either, but it seems to work.  If lots of error
messages are annoying, and are what is really taking a long time (serial
output can be a real delay), then let's turn off the log messages to
make things go faster.

> The way to use dependencies doesn't add new requirements, it just moves them
> away from the link order. Besides that these dependencies are making it
> possible to parallelize the stuff.

We can paralleize probing today, that's been around for a very long
time, nothing is preventing you from enabling that for the drivers you
know will work properly that way right now.  Moving to a "all probing in
parallel" model has been proven to not really speed things up at all,
and in fact, slows things down due to cache and multiple process issues.
See the lkml archives for the work done many years ago around this issue
for the details.

Again, link order is a crude way to handle dependancies for built-in
drivers, I know that, which is why I accepted the deferred probing which
ensures that it will always work eventually.  You are moving from having
things in link-order to be manually specified in another manner,
preventing systems that have different dependancy requirements to never
be able to work.  And this last issue is the big one, deferred probing
is the only solution that will always work for all systems.

Again, work to speed up the problem drivers that you see taking too long
at probe time, that's the real solution here and is what others have
spent a lot of time doing, resulting in my sub-second desktop boot
times.  If embedded systems want to also achieve that, then fix the
drivers, reordering the probe order is not going to solve the boot speed
issue.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249440

Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:

> So how long does that really take to call all probe functions in all
> possible order?  Real numbers please.  We have the tools to determine
> where at boot time delays are happening, please use them to find the
> problem drivers.

No idea. You might ask Tomeu Vizoso (I've added him to cc) for details 
(or search for the thread "On-demand device registration" where he 
complained that his chromebook boots slow). I've just measured, that 
most my ARM boxes booted faster when I've used the ordering, instead of 
slower through the introduce overhead to order initcalls (without having 
parallelized the initcalls).

Posting times doesn't make much sense, as they heavily depend on the 
configuration. Instead I've posted patches so you can test it yourself.

But if you want a real time, my Netbook with a single core but HT Atom 
N270 boots in one second instead of two to "dmesg | grep Freeing".

Regards,

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249446

On Sat, Oct 17, 2015 at 09:43:17PM +0200, Alexander Holler wrote:
> Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:
> 
> >So how long does that really take to call all probe functions in all
> >possible order?  Real numbers please.  We have the tools to determine
> >where at boot time delays are happening, please use them to find the
> >problem drivers.
> 
> No idea. You might ask Tomeu Vizoso (I've added him to cc) for details (or
> search for the thread "On-demand device registration" where he complained
> that his chromebook boots slow). I've just measured, that most my ARM boxes
> booted faster when I've used the ordering, instead of slower through the
> introduce overhead to order initcalls (without having parallelized the
> initcalls).

I've already asked him, I don't like his patch series to try to resolve
this issue either :)

> Posting times doesn't make much sense, as they heavily depend on the
> configuration. Instead I've posted patches so you can test it yourself.
> 
> But if you want a real time, my Netbook with a single core but HT Atom N270
> boots in one second instead of two to "dmesg | grep Freeing".

Try running the boot time graphic tool to determine where that time is
spent, odds are you just need to enable a single driver to async it's
probe function and you should be fine.

Again, fix up the broken driver(s), don't paper over the issues with
core changes that are not necessary.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249457

Am 17.10.2015 um 22:20 schrieb Greg Kroah-Hartman:
> On Sat, Oct 17, 2015 at 09:43:17PM +0200, Alexander Holler wrote:
>> Am 17.10.2015 um 20:38 schrieb Greg Kroah-Hartman:
>>
>>> So how long does that really take to call all probe functions in all
>>> possible order?  Real numbers please.  We have the tools to determine
>>> where at boot time delays are happening, please use them to find the
>>> problem drivers.
>>
>> No idea. You might ask Tomeu Vizoso (I've added him to cc) for details (or
>> search for the thread "On-demand device registration" where he complained
>> that his chromebook boots slow). I've just measured, that most my ARM boxes
>> booted faster when I've used the ordering, instead of slower through the
>> introduce overhead to order initcalls (without having parallelized the
>> initcalls).
>
> I've already asked him, I don't like his patch series to try to resolve
> this issue either :)
>
>> Posting times doesn't make much sense, as they heavily depend on the
>> configuration. Instead I've posted patches so you can test it yourself.
>>
>> But if you want a real time, my Netbook with a single core but HT Atom N270
>> boots in one second instead of two to "dmesg | grep Freeing".
>
> Try running the boot time graphic tool to determine where that time is
> spent, odds are you just need to enable a single driver to async it's
> probe function and you should be fine.
>
> Again, fix up the broken driver(s), don't paper over the issues with
> core changes that are not necessary.

I assume you are aware of the 80/20 rule. So you might see the 
parallelize feature of topological sort as an attempt to do some of the 
work in the last 20 percent left.

Sorry, but I've no idea why you are now trying to pin the stuff I'm 
talking about to a specific issue.

Or to talk in more clear words:

The current initcall ordering is, in my humble opinion, a whole and 
mostly undocumented mess. And I'm pretty sure it will become worse, I 
just have to wait and see. And if every attempt to fix that will be 
killed as fast as my one ...

Anyway, thanks for comments.

Regards,

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249418 — Re: [PATCH 11/14] init: deps: annotate various initcalls

On Sat, Oct 17, 2015 at 10:14 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>
> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
> index 873dbfc..d5d2459 100644
> --- a/arch/arm/common/edma.c
> +++ b/arch/arm/common/edma.c
> @@ -1872,5 +1872,4 @@ static int __init edma_init(void)
>  {
>         return platform_driver_probe(&edma_driver, edma_probe);
>  }
> -arch_initcall(edma_init);
> -
> +annotated_initcall_drv(arch, edma_init, drvid_edma, NULL, edma_driver.driver);
> diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
> index b445a5d..d9bcb89 100644
> --- a/arch/arm/crypto/aes-ce-glue.c
> +++ b/arch/arm/crypto/aes-ce-glue.c
> @@ -520,5 +520,10 @@ static void __exit aes_exit(void)
>         crypto_unregister_algs(aes_algs, ARRAY_SIZE(aes_algs));
>  }
>
> -module_init(aes_init);
> +static const unsigned dependencies[] __initconst __maybe_unused = {
> +       drvid_cryptomgr,
> +       0
> +};
> +
> +annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);
>  module_exit(aes_exit);

So I think this is kind of a sign of the same disease I mentioned
earlier: making dependencies "separate" from the init levels, now
means that you do the initialization of the dependencies *instead* of
the init level. And that smells bad and wrong, and causes this kind of
patch that is not only huge, but si unreadable and the end result
looks like crap too.

We've actually been quite good at having the module attributes all be
*separate* things that work together. So the code had

  module_init(aes_init);
  module_exit(aes_exit);

but also things like

  MODULE_DESCRIPTION("AES-ECB/CBC/CTR/XTS using ARMv8 Crypto Extensions");
  MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
  MODULE_LICENSE("GPL v2");

and that all helps improve readablity and keep things sane.

In contrast, turds like these are just pure and utter crap:

   static const unsigned dependencies[] __initconst __maybe_unused = {
          drvid_cryptomgr,
          0
   };
   annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);

and yes, I know that we have things like this for the driver ID lists
etc, but that doesn't make it better.

No, I think any dependency model should strive to make this really
really easy and separate, and do things like

   module_depends(cryptomgr);

and then just use that to fill in a link section or something like
that. And no, there's no way we will ever maintain a "list of
dependency identifiers". This is stuff that should be all about
scripting, or - better yet - just make the link section contain
strings so that you don't *need* any C level identifiers.

That would be trivial to do by just making the "module_depends()"
macro be something like

  #define _dependency(x,y) \
         static const  struct module_dependency_attribute \
         __used __attribute__ ((__section__ ("__dependencies")))  \
        * __dependency_attr =  { x,y }

  #define module_depends(x) \
        _dependency(#x, KBUILD_NAME)

  #define module_provides(x) \
        _dependency(KBUILD_NAME, #x)

And if a module depends on multiple other things, then you just have
multiple of those "module_depends()" things. There's some gcc trick to
generating numbered (per compilation unit) C identifiers (so that you
can have multiple of those "__dependency_attr" variables in the same
file), but I forget it right now.

And this is also where I think those "module_init()" vs
"subsys_init()" things come in. "module_init()" means that it's a
driver level thing, which would mean that module_init() implies

  module_depends(level7);
  module_provides(level7_end);

so that the module would automatically be sorted wrt the "driver" level.

Another advantage (apart from legibility of the source, and
integrating with the *existing* level-based dependencies) is that
using something like "module_depends()" and "module_provides()" means
that it should be easy to parse even outside of a C compiler, so you
could - if you want to - make all the dependencies be done not as part
of compiling the source, but as a separate scripting thing. That could
be useful for things like statistics and visualization tools that
don't want to actually build the kernel, but want to just show the
dependencies between different modules.

So no. I do *not* think big patches like this are acceptable. This
kind of patch - along with the patch that just adds the random
dependency identifier C enums - is exactly what we do *not* want. If
we do dependencies, they should all be small and local things, and
they should not *replace* the existing "module_init()" vs
"arch_init()" system, they should add on top of it.

                      Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249429 — Re: [PATCH 11/14] init: deps: annotate various initcalls

Am 17.10.2015 um 20:47 schrieb Linus Torvalds:
> On Sat, Oct 17, 2015 at 10:14 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>>
>> diff --git a/arch/arm/common/edma.c b/arch/arm/common/edma.c
>> index 873dbfc..d5d2459 100644
>> --- a/arch/arm/common/edma.c
>> +++ b/arch/arm/common/edma.c
>> @@ -1872,5 +1872,4 @@ static int __init edma_init(void)
>>   {
>>          return platform_driver_probe(&edma_driver, edma_probe);
>>   }
>> -arch_initcall(edma_init);
>> -
>> +annotated_initcall_drv(arch, edma_init, drvid_edma, NULL, edma_driver.driver);
>> diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
>> index b445a5d..d9bcb89 100644
>> --- a/arch/arm/crypto/aes-ce-glue.c
>> +++ b/arch/arm/crypto/aes-ce-glue.c
>> @@ -520,5 +520,10 @@ static void __exit aes_exit(void)
>>          crypto_unregister_algs(aes_algs, ARRAY_SIZE(aes_algs));
>>   }
>>
>> -module_init(aes_init);
>> +static const unsigned dependencies[] __initconst __maybe_unused = {
>> +       drvid_cryptomgr,
>> +       0
>> +};
>> +
>> +annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);
>>   module_exit(aes_exit);
>
> So I think this is kind of a sign of the same disease I mentioned
> earlier: making dependencies "separate" from the init levels, now
> means that you do the initialization of the dependencies *instead* of
> the init level. And that smells bad and wrong, and causes this kind of
> patch that is not only huge, but si unreadable and the end result
> looks like crap too.
>
> We've actually been quite good at having the module attributes all be
> *separate* things that work together. So the code had
>
>    module_init(aes_init);
>    module_exit(aes_exit);
>
> but also things like
>
>    MODULE_DESCRIPTION("AES-ECB/CBC/CTR/XTS using ARMv8 Crypto Extensions");
>    MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
>    MODULE_LICENSE("GPL v2");
>
> and that all helps improve readablity and keep things sane.
>
> In contrast, turds like these are just pure and utter crap:
>
>     static const unsigned dependencies[] __initconst __maybe_unused = {
>            drvid_cryptomgr,
>            0
>     };
>     annotated_module_init(aes_init, drvid_aes_ce_arm, dependencies);
>
> and yes, I know that we have things like this for the driver ID lists
> etc, but that doesn't make it better.
>
> No, I think any dependency model should strive to make this really
> really easy and separate, and do things like
>
>     module_depends(cryptomgr);
>
> and then just use that to fill in a link section or something like
> that. And no, there's no way we will ever maintain a "list of
> dependency identifiers". This is stuff that should be all about
> scripting, or - better yet - just make the link section contain
> strings so that you don't *need* any C level identifiers.
>
> That would be trivial to do by just making the "module_depends()"
> macro be something like
>
>    #define _dependency(x,y) \
>           static const  struct module_dependency_attribute \
>           __used __attribute__ ((__section__ ("__dependencies")))  \
>          * __dependency_attr =  { x,y }
>
>    #define module_depends(x) \
>          _dependency(#x, KBUILD_NAME)
>
>    #define module_provides(x) \
>          _dependency(KBUILD_NAME, #x)
>
> And if a module depends on multiple other things, then you just have
> multiple of those "module_depends()" things. There's some gcc trick to
> generating numbered (per compilation unit) C identifiers (so that you
> can have multiple of those "__dependency_attr" variables in the same
> file), but I forget it right now.
>
> And this is also where I think those "module_init()" vs
> "subsys_init()" things come in. "module_init()" means that it's a
> driver level thing, which would mean that module_init() implies
>
>    module_depends(level7);
>    module_provides(level7_end);
>
> so that the module would automatically be sorted wrt the "driver" level.
>
> Another advantage (apart from legibility of the source, and
> integrating with the *existing* level-based dependencies) is that
> using something like "module_depends()" and "module_provides()" means
> that it should be easy to parse even outside of a C compiler, so you
> could - if you want to - make all the dependencies be done not as part
> of compiling the source, but as a separate scripting thing. That could
> be useful for things like statistics and visualization tools that
> don't want to actually build the kernel, but want to just show the
> dependencies between different modules.
>
> So no. I do *not* think big patches like this are acceptable. This
> kind of patch - along with the patch that just adds the random
> dependency identifier C enums - is exactly what we do *not* want. If
> we do dependencies, they should all be small and local things, and
> they should not *replace* the existing "module_init()" vs
> "arch_init()" system, they should add on top of it.

Thanks for the detailed answer and you are right.

But I had to start somehow and, unfortunately, I don't have the 
resources to fulfill your requirements.

Regards,

Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web