mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	Brandon Philips <brandon@ifup.org>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	LKML <linux-kernel@vger.kernel.org>,
	Jon Masters <jonathan@jonmasters.org>,
	Tejun Heo <htejun@gmail.com>,
	Masami Hiramatsu <mhiramat@redhat.com>,
	Kay Sievers <kay.sievers@vrfy.org>
Subject: [PATCH 1/2] Make the module 'usage' lists be two-way
Date: Mon, 31 May 2010 13:16:23 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.00.1005311315460.3637@i5.linux-foundation.org> (raw)
In-Reply-To: <alpine.LFD.2.00.1005311311490.3637@i5.linux-foundation.org>


From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mon, 31 May 2010 12:19:37 -0700
Subject: [PATCH 1/2] Make the module 'usage' lists be two-way

When adding a module that depends on another one, we used to create a
one-way list of "modules_which_use_me", so that module unloading could
see who needs a module.

It's actually quite simple to make that list go both ways: so that we
not only can see "who uses me", but also see a list of modules that are
"used by me".

In fact, we always wanted that list in "module_unload_free()": when we
unload a module, we want to also release all the other modules that are
used by that module.  But because we didn't have that list, we used to
first iterate over all modules, and then iterate over each "used by me"
list of that module.

By making the list two-way, we simplify module_unload_free(), and it
allows for some trivial fixes later too.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 include/linux/module.h |    4 ++-
 kernel/module.c        |   86 +++++++++++++++++++++++++++--------------------
 2 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 6914fca..680db9e 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -359,7 +359,9 @@ struct module
 
 #ifdef CONFIG_MODULE_UNLOAD
 	/* What modules depend on me? */
-	struct list_head modules_which_use_me;
+	struct list_head source_list;
+	/* What modules do I depend on? */
+	struct list_head target_list;
 
 	/* Who is waiting for us to be unloaded */
 	struct task_struct *waiter;
diff --git a/kernel/module.c b/kernel/module.c
index d806e00..82ae1e4 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -523,7 +523,8 @@ static void module_unload_init(struct module *mod)
 {
 	int cpu;
 
-	INIT_LIST_HEAD(&mod->modules_which_use_me);
+	INIT_LIST_HEAD(&mod->source_list);
+	INIT_LIST_HEAD(&mod->target_list);
 	for_each_possible_cpu(cpu) {
 		per_cpu_ptr(mod->refptr, cpu)->incs = 0;
 		per_cpu_ptr(mod->refptr, cpu)->decs = 0;
@@ -538,8 +539,9 @@ static void module_unload_init(struct module *mod)
 /* modules using other modules */
 struct module_use
 {
-	struct list_head list;
-	struct module *module_which_uses;
+	struct list_head source_list;
+	struct list_head target_list;
+	struct module *source, *target;
 };
 
 /* Does a already use b? */
@@ -547,8 +549,8 @@ static int already_uses(struct module *a, struct module *b)
 {
 	struct module_use *use;
 
-	list_for_each_entry(use, &b->modules_which_use_me, list) {
-		if (use->module_which_uses == a) {
+	list_for_each_entry(use, &b->source_list, source_list) {
+		if (use->source == a) {
 			DEBUGP("%s uses %s!\n", a->name, b->name);
 			return 1;
 		}
@@ -557,11 +559,38 @@ static int already_uses(struct module *a, struct module *b)
 	return 0;
 }
 
+/*
+ * Module a uses b
+ *  - we add 'a' as a "source", 'b' as a "target" of module use
+ *  - the module_use is added to the list of 'b' sources (so
+ *    'b' can walk the list to see who sourced them, and of 'a'
+ *    targets (so 'a' can see what modules it targets).
+ */
+static int add_module_usage(struct module *a, struct module *b)
+{
+	int no_warn;
+	struct module_use *use;
+
+	DEBUGP("Allocating new usage for %s.\n", a->name);
+	use = kmalloc(sizeof(*use), GFP_ATOMIC);
+	if (!use) {
+		printk("%s: out of memory loading\n", a->name);
+		module_put(b);
+		return 0;
+	}
+
+	use->source = a;
+	use->target = b;
+	list_add(&use->source_list, &b->source_list);
+	list_add(&use->target_list, &a->target_list);
+	no_warn = sysfs_create_link(b->holders_dir, &a->mkobj.kobj, a->name);
+	return 1;
+}
+
 /* Module a uses b */
 int use_module(struct module *a, struct module *b)
 {
-	struct module_use *use;
-	int no_warn, err;
+	int err;
 
 	if (b == NULL || already_uses(a, b)) return 1;
 
@@ -578,40 +607,23 @@ int use_module(struct module *a, struct module *b)
 	if (err)
 		return 0;
 
-	DEBUGP("Allocating new usage for %s.\n", a->name);
-	use = kmalloc(sizeof(*use), GFP_ATOMIC);
-	if (!use) {
-		printk("%s: out of memory loading\n", a->name);
-		module_put(b);
-		return 0;
-	}
-
-	use->module_which_uses = a;
-	list_add(&use->list, &b->modules_which_use_me);
-	no_warn = sysfs_create_link(b->holders_dir, &a->mkobj.kobj, a->name);
-	return 1;
+	return add_module_usage(a, b);
 }
 EXPORT_SYMBOL_GPL(use_module);
 
 /* Clear the unload stuff of the module. */
 static void module_unload_free(struct module *mod)
 {
-	struct module *i;
+	struct module_use *use, *tmp;
 
-	list_for_each_entry(i, &modules, list) {
-		struct module_use *use;
-
-		list_for_each_entry(use, &i->modules_which_use_me, list) {
-			if (use->module_which_uses == mod) {
-				DEBUGP("%s unusing %s\n", mod->name, i->name);
-				module_put(i);
-				list_del(&use->list);
-				kfree(use);
-				sysfs_remove_link(i->holders_dir, mod->name);
-				/* There can be at most one match. */
-				break;
-			}
-		}
+	list_for_each_entry_safe(use, tmp, &mod->target_list, target_list) {
+		struct module *i = use->target;
+		DEBUGP("%s unusing %s\n", mod->name, i->name);
+		module_put(i);
+		list_del(&use->source_list);
+		list_del(&use->target_list);
+		kfree(use);
+		sysfs_remove_link(i->holders_dir, mod->name);
 	}
 }
 
@@ -735,7 +747,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		goto out;
 	}
 
-	if (!list_empty(&mod->modules_which_use_me)) {
+	if (!list_empty(&mod->source_list)) {
 		/* Other modules depend on us: get rid of them first. */
 		ret = -EWOULDBLOCK;
 		goto out;
@@ -799,9 +811,9 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
 
 	/* Always include a trailing , so userspace can differentiate
            between this and the old multi-field proc format. */
-	list_for_each_entry(use, &mod->modules_which_use_me, list) {
+	list_for_each_entry(use, &mod->source_list, source_list) {
 		printed_something = 1;
-		seq_printf(m, "%s,", use->module_which_uses->name);
+		seq_printf(m, "%s,", use->source->name);
 	}
 
 	if (mod->init != NULL && mod->exit == NULL) {
-- 
1.7.1.227.gb676f


  reply	other threads:[~2010-05-31 20:20 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-25 21:00 [Regression] Crash in load_module() while freeing args Rafael J. Wysocki
2010-05-25 22:54 ` Rafael J. Wysocki
2010-05-25 23:47   ` Linus Torvalds
2010-05-26  8:00     ` Rusty Russell
2010-05-26 11:57       ` Rusty Russell
2010-05-26 22:56         ` Rafael J. Wysocki
2010-05-26 23:07           ` Linus Torvalds
2010-05-27  5:26           ` Rusty Russell
2010-05-27 18:46             ` Brandon Philips
2010-05-31  9:40               ` Rusty Russell
2010-05-31 12:00                 ` [PATCH 0/2] kernel/module.c locking changes Rusty Russell
2010-05-31 12:01                   ` [PATCH 1/2] module: make locking more fine-grained Rusty Russell
2010-05-31 12:02                     ` [PATCH 2/2] module: fix bne2 "gave up waiting for init of module libcrc32c" Rusty Russell
2010-05-31 16:48                       ` Andrew Morton
2010-05-31 18:19                         ` Linus Torvalds
2010-05-31 20:15                           ` Linus Torvalds
2010-05-31 20:16                             ` Linus Torvalds [this message]
2010-05-31 20:17                               ` [PATCH 2/2] module: wait for other modules after dropping the module_mutex Linus Torvalds
2010-06-01  1:37                               ` [PATCH 1/2] Make the module 'usage' lists be two-way Rusty Russell
2010-06-01  3:42                                 ` Rusty Russell
2010-06-01  4:00                                   ` Linus Torvalds
2010-06-01  4:05                                     ` Linus Torvalds
2010-06-01  2:44                               ` Américo Wang
2010-06-01  3:51                                 ` Linus Torvalds
2010-06-01  1:57                             ` [PATCH 2/2] module: fix bne2 "gave up waiting for init of module libcrc32c" Rusty Russell
2010-06-01  3:40                               ` Linus Torvalds
2010-06-01  4:27                                 ` Linus Torvalds
2010-06-01  5:19                                 ` Rusty Russell
2010-06-02  3:15                                   ` Rusty Russell
2010-06-01  1:21                           ` Rusty Russell
2010-06-01  3:24                             ` Linus Torvalds
2010-06-01  5:22                               ` Rusty Russell
2010-06-01 14:58                                 ` Linus Torvalds
2010-06-01 17:53                                   ` Linus Torvalds
2010-06-01 23:24                                     ` Brandon Philips
2010-06-01 23:51                                       ` Linus Torvalds
2010-06-02  2:10                                         ` Brandon Philips
2010-06-02  3:03                                           ` Rusty Russell
2010-06-02  4:35                                           ` Linus Torvalds
2010-06-02  4:44                                             ` Linus Torvalds
2010-06-02  6:35                                               ` Rusty Russell
2010-06-02  7:45                                                 ` Linus Torvalds
2010-06-02  8:12                                                   ` Linus Torvalds
2010-06-02  9:07                                                     ` Rusty Russell
2010-06-02  5:52                                             ` Rusty Russell
2010-06-02  7:21                                               ` Linus Torvalds
2010-06-02 14:06                                                 ` Rusty Russell
2010-06-02 14:50                                                   ` Linus Torvalds
2010-06-03 13:06                                                     ` Rusty Russell
2010-06-02 16:53                                                   ` Brandon Philips
2010-06-02 18:01                                                   ` Linus Torvalds
2010-06-03  5:20                                                     ` Rusty Russell
2010-06-03 16:24                                                       ` Linus Torvalds
2010-06-04  1:02                                                         ` Rusty Russell
2010-06-04  1:55                                                           ` Linus Torvalds
2010-06-04  5:20                                                             ` Rusty Russell
2010-06-04 22:48                                                               ` Linus Torvalds
2010-06-05  1:49                                                                 ` Rusty Russell
2010-06-02  3:09                                   ` Rusty Russell
2010-06-02  4:32                                     ` Linus Torvalds
2010-06-02  4:56                                     ` Linus Torvalds
2010-06-02  5:52                                       ` Rusty Russell
2010-06-02  6:59                                         ` Linus Torvalds
2010-06-01  1:04                         ` Rusty Russell
2010-06-01  5:38                     ` [PATCH 1/2] module: make locking more fine-grained Américo Wang
2010-06-01  5:55                       ` Rusty Russell
2010-05-27 21:57             ` [Regression] Crash in load_module() while freeing args Rafael J. Wysocki
2010-05-31  7:54               ` Rusty Russell
2010-05-31 10:23               ` [PATCH] module: fix reference to mod->percpu after freeing module Rusty Russell
2010-05-31 10:25                 ` Tejun Heo
2010-05-26 15:41       ` [Regression] Crash in load_module() while freeing args Linus Torvalds

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=alpine.LFD.2.00.1005311315460.3637@i5.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=brandon@ifup.org \
    --cc=htejun@gmail.com \
    --cc=jonathan@jonmasters.org \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=rjw@sisk.pl \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

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

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

Powered by JetHome