All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	alan@lxorguk.ukuu.org.uk, Prarit Bhargava <prarit@redhat.com>,
	Rusty Russell <rusty@rustcorp.com.au>
Subject: [ 36/46] module: put modules in list much earlier.
Date: Thu, 24 Jan 2013 13:13:14 -0800	[thread overview]
Message-ID: <20130124211149.244643977@linuxfoundation.org> (raw)
In-Reply-To: <20130124211135.862755794@linuxfoundation.org>

3.7-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Rusty Russell <rusty@rustcorp.com.au>

commit 1fb9341ac34825aa40354e74d9a2c69df7d2c304 upstream.

Prarit's excellent bug report:
> In recent Fedora releases (F17 & F18) some users have reported seeing
> messages similar to
>
> [   15.478160] kvm: Could not allocate 304 bytes percpu data
> [   15.478174] PERCPU: allocation failed, size=304 align=32, alloc from
> reserved chunk failed
>
> during system boot.  In some cases, users have also reported seeing this
> message along with a failed load of other modules.
>
> What is happening is systemd is loading an instance of the kvm module for
> each cpu found (see commit e9bda3b).  When the module load occurs the kernel
> currently allocates the modules percpu data area prior to checking to see
> if the module is already loaded or is in the process of being loaded.  If
> the module is already loaded, or finishes load, the module loading code
> releases the current instance's module's percpu data.

Now we have a new state MODULE_STATE_UNFORMED, we can insert the
module into the list (and thus guarantee its uniqueness) before we
allocate the per-cpu region.

Reported-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Tested-by: Prarit Bhargava <prarit@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 kernel/module.c |   90 ++++++++++++++++++++++++++++++--------------------------
 lib/bug.c       |    1 
 2 files changed, 50 insertions(+), 41 deletions(-)

--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2958,7 +2958,7 @@ static bool finished_loading(const char
 	bool ret;
 
 	mutex_lock(&module_mutex);
-	mod = find_module(name);
+	mod = find_module_all(name, true);
 	ret = !mod || mod->state == MODULE_STATE_LIVE
 		|| mod->state == MODULE_STATE_GOING;
 	mutex_unlock(&module_mutex);
@@ -2991,6 +2991,32 @@ static struct module *load_module(void _
 		goto free_copy;
 	}
 
+	/*
+	 * We try to place it in the list now to make sure it's unique
+	 * before we dedicate too many resources.  In particular,
+	 * temporary percpu memory exhaustion.
+	 */
+	mod->state = MODULE_STATE_UNFORMED;
+again:
+	mutex_lock(&module_mutex);
+	if ((old = find_module_all(mod->name, true)) != NULL) {
+		if (old->state == MODULE_STATE_COMING
+		    || old->state == MODULE_STATE_UNFORMED) {
+			/* Wait in case it fails to load. */
+			mutex_unlock(&module_mutex);
+			err = wait_event_interruptible(module_wq,
+					       finished_loading(mod->name));
+			if (err)
+				goto free_module;
+			goto again;
+		}
+		err = -EEXIST;
+		mutex_unlock(&module_mutex);
+		goto free_module;
+	}
+	list_add_rcu(&mod->list, &modules);
+	mutex_unlock(&module_mutex);
+
 #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info.sig_ok;
 	if (!mod->sig_ok)
@@ -3000,7 +3026,7 @@ static struct module *load_module(void _
 	/* Now module is in final location, initialize linked lists, etc. */
 	err = module_unload_init(mod);
 	if (err)
-		goto free_module;
+		goto unlink_mod;
 
 	/* Now we've got everything in the final locations, we can
 	 * find optional sections. */
@@ -3035,54 +3061,33 @@ static struct module *load_module(void _
 		goto free_arch_cleanup;
 	}
 
-	/* Mark state as coming so strong_try_module_get() ignores us. */
-	mod->state = MODULE_STATE_COMING;
-
-	/* Now sew it into the lists so we can get lockdep and oops
-	 * info during argument parsing.  No one should access us, since
-	 * strong_try_module_get() will fail.
-	 * lockdep/oops can run asynchronous, so use the RCU list insertion
-	 * function to insert in a way safe to concurrent readers.
-	 * The mutex protects against concurrent writers.
-	 */
-again:
-	mutex_lock(&module_mutex);
-	if ((old = find_module(mod->name)) != NULL) {
-		if (old->state == MODULE_STATE_COMING) {
-			/* Wait in case it fails to load. */
-			mutex_unlock(&module_mutex);
-			err = wait_event_interruptible(module_wq,
-					       finished_loading(mod->name));
-			if (err)
-				goto free_arch_cleanup;
-			goto again;
-		}
-		err = -EEXIST;
-		goto unlock;
-	}
-
-	/* This has to be done once we're sure module name is unique. */
 	dynamic_debug_setup(info.debug, info.num_debug);
 
-	/* Find duplicate symbols */
+	mutex_lock(&module_mutex);
+	/* Find duplicate symbols (must be called under lock). */
 	err = verify_export_symbols(mod);
 	if (err < 0)
-		goto ddebug;
+		goto ddebug_cleanup;
 
+	/* This relies on module_mutex for list integrity. */
 	module_bug_finalize(info.hdr, info.sechdrs, mod);
-	list_add_rcu(&mod->list, &modules);
+
+	/* Mark state as coming so strong_try_module_get() ignores us,
+	 * but kallsyms etc. can see us. */
+	mod->state = MODULE_STATE_COMING;
+
 	mutex_unlock(&module_mutex);
 
 	/* Module is ready to execute: parsing args may do that. */
 	err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
 			 -32768, 32767, &ddebug_dyndbg_module_param_cb);
 	if (err < 0)
-		goto unlink;
+		goto bug_cleanup;
 
 	/* Link in to syfs. */
 	err = mod_sysfs_setup(mod, &info, mod->kp, mod->num_kp);
 	if (err < 0)
-		goto unlink;
+		goto bug_cleanup;
 
 	/* Get rid of temporary copy. */
 	free_copy(&info);
@@ -3091,16 +3096,13 @@ again:
 	trace_module_load(mod);
 	return mod;
 
- unlink:
+ bug_cleanup:
+	/* module_bug_cleanup needs module_mutex protection */
 	mutex_lock(&module_mutex);
-	/* Unlink carefully: kallsyms could be walking list. */
-	list_del_rcu(&mod->list);
 	module_bug_cleanup(mod);
-	wake_up_all(&module_wq);
- ddebug:
-	dynamic_debug_remove(info.debug);
- unlock:
 	mutex_unlock(&module_mutex);
+ ddebug_cleanup:
+	dynamic_debug_remove(info.debug);
 	synchronize_sched();
 	kfree(mod->args);
  free_arch_cleanup:
@@ -3109,6 +3111,12 @@ again:
 	free_modinfo(mod);
  free_unload:
 	module_unload_free(mod);
+ unlink_mod:
+	mutex_lock(&module_mutex);
+	/* Unlink carefully: kallsyms could be walking list. */
+	list_del_rcu(&mod->list);
+	wake_up_all(&module_wq);
+	mutex_unlock(&module_mutex);
  free_module:
 	module_deallocate(mod, &info);
  free_copy:
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -55,6 +55,7 @@ static inline unsigned long bug_addr(con
 }
 
 #ifdef CONFIG_MODULES
+/* Updates are protected by module mutex */
 static LIST_HEAD(module_bug_list);
 
 static const struct bug_entry *module_find_bug(unsigned long bugaddr)



  parent reply	other threads:[~2013-01-24 21:31 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-24 21:12 [ 00/46] 3.7.5-stable review Greg Kroah-Hartman
2013-01-24 21:12 ` [ 01/46] make sure that /linuxrc has std{in,out,err} Greg Kroah-Hartman
2013-01-24 21:12 ` [ 02/46] Ensure that kernel_init_freeable() is not inlined into non __init code Greg Kroah-Hartman
2013-01-24 21:12 ` [ 03/46] drm/i915: Invalidate the relocation presumed_offsets along the slow path Greg Kroah-Hartman
2013-01-24 21:12 ` [ 04/46] libata: ahci: Fix lack of command retry after a success error handler Greg Kroah-Hartman
2013-01-24 21:12 ` [ 05/46] libata: ahci: Add support for Enmotus Bobcat device Greg Kroah-Hartman
2013-01-24 21:12 ` [ 06/46] libata: replace sata_settings with devslp_timing Greg Kroah-Hartman
2013-01-24 21:12 ` [ 07/46] ftrace: Be first to run code modification on modules Greg Kroah-Hartman
2013-01-24 21:12 ` [ 08/46] evm: checking if removexattr is not a NULL Greg Kroah-Hartman
2013-01-24 21:12 ` [ 09/46] virtio-blk: Dont free ida when disk is in use Greg Kroah-Hartman
2013-01-24 21:12 ` [ 10/46] async: fix __lowest_in_progress() Greg Kroah-Hartman
2013-01-24 21:12 ` [ 11/46] vfio-pci: Fix buffer overfill Greg Kroah-Hartman
2013-01-24 21:12 ` [ 12/46] perf x86: revert 20b279 - require exclude_guest to use PEBS - kernel side Greg Kroah-Hartman
2013-01-24 21:12 ` [ 13/46] ptrace: introduce signal_wake_up_state() and ptrace_signal_wake_up() Greg Kroah-Hartman
2013-01-24 21:12 ` [ 14/46] ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL Greg Kroah-Hartman
2013-01-24 21:12 ` [ 15/46] wake_up_process() should be never used to wakeup a TASK_STOPPED/TRACED task Greg Kroah-Hartman
2013-01-24 21:12 ` [ 16/46] ALSA: hda - Fix mute led for another HP machine Greg Kroah-Hartman
2013-01-24 21:12 ` [ 17/46] ALSA: hda - Add Conexant CX20755/20756/20757 codec IDs Greg Kroah-Hartman
2013-01-24 21:12 ` [ 18/46] arm64: makefile: fix uname munging when setting ARCH on native machine Greg Kroah-Hartman
2013-01-24 21:12 ` [ 19/46] arm64: elf: fix core dumping to match what glibc expects Greg Kroah-Hartman
2013-01-24 21:12 ` [ 20/46] PCI/AER: pci_get_domain_bus_and_slot() call missing required pci_dev_put() Greg Kroah-Hartman
2013-01-24 21:12 ` [ 21/46] PCI: Allow pcie_aspm=force even when FADT indicates it is unsupported Greg Kroah-Hartman
2013-01-24 21:13 ` [ 22/46] PCI: pciehp: Use per-slot workqueues to avoid deadlock Greg Kroah-Hartman
2013-01-24 21:13 ` [ 23/46] PCI: shpchp: Handle push button event asynchronously Greg Kroah-Hartman
2013-01-24 21:13 ` [ 24/46] PCI: shpchp: Use per-slot workqueues to avoid deadlock Greg Kroah-Hartman
2013-01-24 21:13 ` [ 25/46] Revert "drivers/misc/ti-st: remove gpio handling" Greg Kroah-Hartman
2013-01-24 21:13 ` [ 26/46] media: gspca_kinect: add Kinect for Windows USB id Greg Kroah-Hartman
2013-01-24 21:13 ` [ 27/46] USB: UHCI: fix IRQ race during initialization Greg Kroah-Hartman
2013-01-24 21:13 ` [ 28/46] usb: dwc3: gadget: fix ep->maxburst for ep0 Greg Kroah-Hartman
2013-01-24 21:13 ` [ 29/46] usb: gadget: FunctionFS: Fix missing braces in parse_opts Greg Kroah-Hartman
2013-01-24 21:13 ` [ 30/46] usb: musb: cppi_dma: drop __init annotation Greg Kroah-Hartman
2013-01-24 21:13 ` [ 31/46] SCSI: sd: Reshuffle init_sd to avoid crash Greg Kroah-Hartman
2013-01-24 21:13 ` [ 32/46] drivers/firmware/dmi_scan.c: check dmi version when get system uuid Greg Kroah-Hartman
2013-01-24 21:13 ` [ 33/46] drivers/firmware/dmi_scan.c: fetch dmi version from SMBIOS if it exists Greg Kroah-Hartman
2013-01-24 21:13   ` Greg Kroah-Hartman
2013-01-24 21:13 ` [ 34/46] drm/i915: Implement WaDisableHiZPlanesWhenMSAAEnabled Greg Kroah-Hartman
2013-01-24 21:13 ` [ 35/46] module: add new state MODULE_STATE_UNFORMED Greg Kroah-Hartman
2013-01-24 21:13 ` Greg Kroah-Hartman [this message]
2013-01-24 21:13 ` [ 37/46] module: fix missing module_mutex unlock Greg Kroah-Hartman
2013-01-24 21:13 ` [ 38/46] powernow-k8: Add a kconfig dependency on acpi-cpufreq Greg Kroah-Hartman
2013-01-24 21:13 ` [ 39/46] cpufreq: Add module aliases for acpi-cpufreq Greg Kroah-Hartman
2013-01-24 21:13 ` [ 40/46] ACPI / cpuidle: Fix NULL pointer issues when cpuidle is disabled Greg Kroah-Hartman
2013-01-24 21:13 ` [ 41/46] ACPI / processor: Get power info before updating the C-states Greg Kroah-Hartman
2013-01-24 21:13 ` [ 42/46] ACPI: Check MSR valid bit before using P-state frequencies Greg Kroah-Hartman
2013-01-24 21:13 ` [ 43/46] i2c: mxs: Fix type of error code Greg Kroah-Hartman
2013-01-24 21:13 ` [ 44/46] intel_idle: Dont register CPU notifier if we are not running Greg Kroah-Hartman
2013-01-24 21:13 ` [ 45/46] ioat: Fix DMA memory sync direction correct flag Greg Kroah-Hartman
2013-01-24 21:13 ` [ 46/46] dma: tegra: implement flags parameters for cyclic transfer Greg Kroah-Hartman
2013-01-25 18:06 ` [ 00/46] 3.7.5-stable review Shuah Khan
2013-01-27  2:15 ` Satoru Takeuchi

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=20130124211149.244643977@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=prarit@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=stable@vger.kernel.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.