All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Kay Sievers <kay.sievers@vrfy.org>
Cc: Andreas Robinson <andr345@gmail.com>,
	sam@ravnborg.org, linux-kernel@vger.kernel.org,
	Jon Masters <jonathan@jonmasters.org>,
	heiko.carstens@de.ibm.com
Subject: Re: [RFC PATCH 0/6] module, kbuild: Faster boot with custom kernel.
Date: Fri, 20 Feb 2009 11:28:55 +1030	[thread overview]
Message-ID: <200902201128.55959.rusty@rustcorp.com.au> (raw)
In-Reply-To: <ac3eb2510902191359o3e32c424h830e78cd0ace446a@mail.gmail.com>

On Friday 20 February 2009 08:29:48 Kay Sievers wrote:
> Further testing revealed, if I only comment out the stop_machine()
> preparation, which is used in an error case, I get almost the same
> improvement, even with the original mutex in place. Without the mutex
> it's still a bit better, maybe it would be much better if we have more
> CPUs, but all the long delays are gone only with removing the
> stop_machine() preparation.

Hmm, interesting.  The reason that reducing the lock coverage had this effect
is because stop_machine_create() just bumps a refcount if someone is already
between ...create() and ...destroy().

So, now we've found the problem, let's fix it, then re-visit mutex reduction.

module: don't use stop_machine on module load

Kay Sievers <kay.sievers@vrfy.org> discovered that boot times are slowed
by about half a second because all the stop_machine_create() calls,
and he only probes about 40 modules (I have 125 loaded on this laptop).

We only do stop_machine_create() so we can unlink the module if
something goes wrong, but it's overkill (and buggy anyway: if
stop_machine_create() fails we still call stop_machine_destroy()).

Since we are only protecting against kallsyms (esp. oops) walking the
list, synchronize_sched() is sufficient (synchronize_rcu() is probably
sufficient, but we're not in a hurry).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/kernel/module.c b/kernel/module.c
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1881,12 +1881,6 @@ static noinline struct module *load_modu
 	if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
 		return ERR_PTR(-ENOMEM);
 
-	/* Create stop_machine threads since the error path relies on
-	 * a non-failing stop_machine call. */
-	err = stop_machine_create();
-	if (err)
-		goto free_hdr;
-
 	if (copy_from_user(hdr, umod, len) != 0) {
 		err = -EFAULT;
 		goto free_hdr;
@@ -2271,12 +2265,13 @@ static noinline struct module *load_modu
 	/* Get rid of temporary copy */
 	vfree(hdr);
 
-	stop_machine_destroy();
 	/* Done! */
 	return mod;
 
  unlink:
-	stop_machine(__unlink_module, mod, NULL);
+	/* Unlink carefully: kallsyms could be walking list. */
+	__list_del(&mod->list);
+	synchronize_sched();
 	module_arch_cleanup(mod);
  cleanup:
 	kobject_del(&mod->mkobj.kobj);
@@ -2297,7 +2292,6 @@ static noinline struct module *load_modu
 	kfree(args);
  free_hdr:
 	vfree(hdr);
-	stop_machine_destroy();
 	return ERR_PTR(err);
 
  truncated:

  reply	other threads:[~2009-02-20  0:59 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-15 18:20 [RFC PATCH 0/6] module, kbuild: Faster boot with custom kernel Andreas Robinson
2009-02-15 18:20 ` [RFC PATCH 1/6] New option: Static linking of external modules Andreas Robinson
2009-02-15 18:20 ` [RFC PATCH 2/6] module: add module ELF section with module_init() pointer Andreas Robinson
2009-02-15 18:20 ` [RFC PATCH 3/6] module: always prefix module parameters with the module name Andreas Robinson
2009-02-15 18:20 ` [RFC PATCH 4/6] kbuild: allow linking of an external object into vmlinux Andreas Robinson
2009-02-15 18:20 ` [RFC PATCH 5/6] scripts: new module preprocessor for static linking Andreas Robinson
2009-02-15 18:20 ` [RFC PATCH 6/6] kbuild: enable relinking of vmlinux without full kernel tree Andreas Robinson
2009-02-16 22:51 ` [RFC PATCH 0/6] module, kbuild: Faster boot with custom kernel Rusty Russell
2009-02-17 10:42   ` Andreas Robinson
2009-02-17 11:53     ` Kay Sievers
2009-02-18  4:58       ` Rusty Russell
2009-02-18  9:15         ` Kay Sievers
2009-02-18 10:25           ` Andreas Robinson
2009-02-20  0:37             ` Andreas Robinson
2009-02-20  1:55               ` Kay Sievers
2009-02-21 11:43                 ` Andreas Robinson
2009-03-02 14:32                   ` Andreas Robinson
2009-03-02 15:59                     ` Kay Sievers
2009-03-02 16:20                     ` Arjan van de Ven
2009-03-02 16:29                       ` Kay Sievers
2009-03-02 18:27                         ` Arjan van de Ven
2009-03-02 21:41                           ` Andreas Robinson
2009-03-04 18:47                           ` Andreas Robinson
2009-03-06  0:18                             ` Arjan van de Ven
2009-03-06 15:15                               ` Andreas Robinson
2009-03-06 15:45                                 ` Arjan van de Ven
2009-03-08 10:47                                   ` Andreas Robinson
2009-03-08 16:01                                     ` Arjan van de Ven
2009-03-08 20:13                                       ` [PATCH] sata_nv: add a module parameter to enable async scanning Andreas Robinson
2009-03-09 17:12                                       ` [RFC PATCH 0/6] module, kbuild: Faster boot with custom kernel Will Newton
2009-03-06  7:05                       ` fastboot kernel parameter Sitsofe Wheeler
2009-03-06 11:23                         ` Arjan van de Ven
2009-02-24  1:27               ` [RFC PATCH 0/6] module, kbuild: Faster boot with custom kernel Rusty Russell
2009-02-18 11:57           ` Rusty Russell
2009-02-18 13:57             ` Kay Sievers
2009-02-19 11:15               ` Rusty Russell
2009-02-19 11:41                 ` Kay Sievers
2009-02-19 20:48                   ` Kay Sievers
2009-02-19 21:59                     ` Kay Sievers
2009-02-20  0:58                       ` Rusty Russell [this message]
2009-02-20  1:33                         ` Kay Sievers
2009-02-24  1:39                           ` Rusty Russell
2009-02-20 11:32                         ` Rusty Russell
2009-02-23 16:42                           ` Kay Sievers
2009-02-25  7:03                             ` Rusty Russell
2009-02-25 18:12                               ` Kay Sievers

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=200902201128.55959.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=andr345@gmail.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=jonathan@jonmasters.org \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sam@ravnborg.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.