diff mbox

[Lucid] SRU: Allow mmc cards to be used with unsafe resume

Message ID 1270288207-10441-1-git-send-email-stefan.bader@canonical.com
State Accepted
Delegated to: Andy Whitcroft
Headers show

Commit Message

Stefan Bader April 3, 2010, 9:50 a.m. UTC
SRU Justification:

Impact: On suspend, mmc cards are ejected on suspend and rediscovered at
resume. There seems to be a bug in that in Lucid as a mounted mmc card
causes the suspend to hang. Also ejecting and rediscovering is actually
annoying if the card contains extensions to the home directory or a root
filesystem.

Fix: This is not a real fix, but a workaround. In the Lucid code there is
a compile time only option to allow the mmc/sd card to be mounted and just
get reused on resume. Contrary to the unsafe title there arctually is some
safeguarding code to check the cards id before blindly using it.
The following patch leaves the default as it is now, but allows users to
change the mode with a boot parameter or by writing into the modules parameter
in sysfs.

Testcase: Suspend with mounted SD card hangs on its way down. With removable=0
it will go down and come up without apparent problems.


From faf4b61efc07ba61195e97b42200331aea030b0d Mon Sep 17 00:00:00 2001
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 14 Dec 2009 18:01:29 -0800
Subject: [PATCH] mmc: add module parameter to set whether cards are assumed removable

Some people run general-purpose distribution kernels on netbooks with
a card that is physically non-removable or logically non-removable
(e.g. used for /home) and cannot be cleanly unmounted during suspend.
Add a module parameter to set whether cards are assumed removable or
non-removable, with the default set by CONFIG_MMC_UNSAFE_RESUME.

In general, it is not possible to tell whether a card present in an MMC
slot after resume is the same that was there before suspend.  So there are
two possible behaviours, each of which will cause data loss in some cases:

CONFIG_MMC_UNSAFE_RESUME=n (default): Cards are assumed to be removed
during suspend.  Any filesystem on them must be unmounted before suspend;
otherwise, buffered writes will be lost.

CONFIG_MMC_UNSAFE_RESUME=y: Cards are assumed to remain present during
suspend.  They must not be swapped during suspend; otherwise, buffered
writes will be flushed to the wrong card.

Currently the choice is made at compile time and this allows that to be
overridden at module load time.

BugLink: http://bugs.launchpad.net/bugs/477106

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: Wouter van Heyst <larstiq@larstiq.dyndns.org>
Cc: <linux-mmc@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 drivers/mmc/core/Kconfig |    4 +++-
 drivers/mmc/core/core.c  |   16 ++++++++++++++++
 drivers/mmc/core/core.h  |    2 ++
 drivers/mmc/core/mmc.c   |   23 +----------------------
 drivers/mmc/core/sd.c    |   21 +--------------------
 5 files changed, 23 insertions(+), 43 deletions(-)

Comments

Leann Ogasawara April 5, 2010, 3:24 p.m. UTC | #1
On Sat, 2010-04-03 at 11:50 +0200, Stefan Bader wrote:
> SRU Justification:
> 
> Impact: On suspend, mmc cards are ejected on suspend and rediscovered at
> resume. There seems to be a bug in that in Lucid as a mounted mmc card
> causes the suspend to hang. Also ejecting and rediscovering is actually
> annoying if the card contains extensions to the home directory or a root
> filesystem.
> 
> Fix: This is not a real fix, but a workaround. In the Lucid code there is
> a compile time only option to allow the mmc/sd card to be mounted and just
> get reused on resume. Contrary to the unsafe title there arctually is some
> safeguarding code to check the cards id before blindly using it.
> The following patch leaves the default as it is now, but allows users to
> change the mode with a boot parameter or by writing into the modules parameter
> in sysfs.
> 
> Testcase: Suspend with mounted SD card hangs on its way down. With removable=0
> it will go down and come up without apparent problems.
> 
> 
> From faf4b61efc07ba61195e97b42200331aea030b0d Mon Sep 17 00:00:00 2001
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Mon, 14 Dec 2009 18:01:29 -0800
> Subject: [PATCH] mmc: add module parameter to set whether cards are assumed removable
> 
> Some people run general-purpose distribution kernels on netbooks with
> a card that is physically non-removable or logically non-removable
> (e.g. used for /home) and cannot be cleanly unmounted during suspend.
> Add a module parameter to set whether cards are assumed removable or
> non-removable, with the default set by CONFIG_MMC_UNSAFE_RESUME.
> 
> In general, it is not possible to tell whether a card present in an MMC
> slot after resume is the same that was there before suspend.  So there are
> two possible behaviours, each of which will cause data loss in some cases:
> 
> CONFIG_MMC_UNSAFE_RESUME=n (default): Cards are assumed to be removed
> during suspend.  Any filesystem on them must be unmounted before suspend;
> otherwise, buffered writes will be lost.
> 
> CONFIG_MMC_UNSAFE_RESUME=y: Cards are assumed to remain present during
> suspend.  They must not be swapped during suspend; otherwise, buffered
> writes will be flushed to the wrong card.
> 
> Currently the choice is made at compile time and this allows that to be
> overridden at module load time.
> 
> BugLink: http://bugs.launchpad.net/bugs/477106
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Cc: Wouter van Heyst <larstiq@larstiq.dyndns.org>
> Cc: <linux-mmc@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> (cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>

Looks ok to me.  Stefan, will you be building test kernels for bug
477106?

Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>

> ---
>  drivers/mmc/core/Kconfig |    4 +++-
>  drivers/mmc/core/core.c  |   16 ++++++++++++++++
>  drivers/mmc/core/core.h  |    2 ++
>  drivers/mmc/core/mmc.c   |   23 +----------------------
>  drivers/mmc/core/sd.c    |   21 +--------------------
>  5 files changed, 23 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index ab37a6d..bb22ffd 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -3,7 +3,7 @@
>  #
>  
>  config MMC_UNSAFE_RESUME
> -	bool "Allow unsafe resume (DANGEROUS)"
> +	bool "Assume MMC/SD cards are non-removable (DANGEROUS)"
>  	help
>  	  If you say Y here, the MMC layer will assume that all cards
>  	  stayed in their respective slots during the suspend. The
> @@ -14,3 +14,5 @@ config MMC_UNSAFE_RESUME
>  	  This option is usually just for embedded systems which use
>  	  a MMC/SD card for rootfs. Most people should say N here.
>  
> +	  This option sets a default which can be overridden by the
> +	  module parameter "removable=0" or "removable=1".
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 7dab2e5..30acd52 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -48,6 +48,22 @@ int use_spi_crc = 1;
>  module_param(use_spi_crc, bool, 0);
>  
>  /*
> + * We normally treat cards as removed during suspend if they are not
> + * known to be on a non-removable bus, to avoid the risk of writing
> + * back data to a different card after resume.  Allow this to be
> + * overridden if necessary.
> + */
> +#ifdef CONFIG_MMC_UNSAFE_RESUME
> +int mmc_assume_removable;
> +#else
> +int mmc_assume_removable = 1;
> +#endif
> +module_param_named(removable, mmc_assume_removable, bool, 0644);
> +MODULE_PARM_DESC(
> +	removable,
> +	"MMC/SD cards are removable and may be removed during suspend");
> +
> +/*
>   * Internal function. Schedule delayed work in the MMC work queue.
>   */
>  static int mmc_schedule_delayed_work(struct delayed_work *work,
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 67ae6ab..a811c52 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -54,7 +54,9 @@ int mmc_attach_mmc(struct mmc_host *host, u32 ocr);
>  int mmc_attach_sd(struct mmc_host *host, u32 ocr);
>  int mmc_attach_sdio(struct mmc_host *host, u32 ocr);
>  
> +/* Module parameters */
>  extern int use_spi_crc;
> +extern int mmc_assume_removable;
>  
>  /* Debugfs information for hosts and cards */
>  void mmc_add_host_debugfs(struct mmc_host *host);
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index bfefce3..c111894 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -602,25 +602,6 @@ static int mmc_awake(struct mmc_host *host)
>  	return err;
>  }
>  
> -#ifdef CONFIG_MMC_UNSAFE_RESUME
> -
> -static const struct mmc_bus_ops mmc_ops = {
> -	.awake = mmc_awake,
> -	.sleep = mmc_sleep,
> -	.remove = mmc_remove,
> -	.detect = mmc_detect,
> -	.suspend = mmc_suspend,
> -	.resume = mmc_resume,
> -	.power_restore = mmc_power_restore,
> -};
> -
> -static void mmc_attach_bus_ops(struct mmc_host *host)
> -{
> -	mmc_attach_bus(host, &mmc_ops);
> -}
> -
> -#else
> -
>  static const struct mmc_bus_ops mmc_ops = {
>  	.awake = mmc_awake,
>  	.sleep = mmc_sleep,
> @@ -645,15 +626,13 @@ static void mmc_attach_bus_ops(struct mmc_host *host)
>  {
>  	const struct mmc_bus_ops *bus_ops;
>  
> -	if (host->caps & MMC_CAP_NONREMOVABLE)
> +	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
>  		bus_ops = &mmc_ops_unsafe;
>  	else
>  		bus_ops = &mmc_ops;
>  	mmc_attach_bus(host, bus_ops);
>  }
>  
> -#endif
> -
>  /*
>   * Starting point for MMC card init.
>   */
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 10b2a4d..fdd414e 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -606,23 +606,6 @@ static void mmc_sd_power_restore(struct mmc_host *host)
>  	mmc_release_host(host);
>  }
>  
> -#ifdef CONFIG_MMC_UNSAFE_RESUME
> -
> -static const struct mmc_bus_ops mmc_sd_ops = {
> -	.remove = mmc_sd_remove,
> -	.detect = mmc_sd_detect,
> -	.suspend = mmc_sd_suspend,
> -	.resume = mmc_sd_resume,
> -	.power_restore = mmc_sd_power_restore,
> -};
> -
> -static void mmc_sd_attach_bus_ops(struct mmc_host *host)
> -{
> -	mmc_attach_bus(host, &mmc_sd_ops);
> -}
> -
> -#else
> -
>  static const struct mmc_bus_ops mmc_sd_ops = {
>  	.remove = mmc_sd_remove,
>  	.detect = mmc_sd_detect,
> @@ -643,15 +626,13 @@ static void mmc_sd_attach_bus_ops(struct mmc_host *host)
>  {
>  	const struct mmc_bus_ops *bus_ops;
>  
> -	if (host->caps & MMC_CAP_NONREMOVABLE)
> +	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
>  		bus_ops = &mmc_sd_ops_unsafe;
>  	else
>  		bus_ops = &mmc_sd_ops;
>  	mmc_attach_bus(host, bus_ops);
>  }
>  
> -#endif
> -
>  /*
>   * Starting point for SD card init.
>   */
> -- 
> 1.6.3.3
> 
>
Stefan Bader April 5, 2010, 3:47 p.m. UTC | #2
Leann Ogasawara wrote:
> On Sat, 2010-04-03 at 11:50 +0200, Stefan Bader wrote:
>> SRU Justification:
>>
>> Impact: On suspend, mmc cards are ejected on suspend and rediscovered at
>> resume. There seems to be a bug in that in Lucid as a mounted mmc card
>> causes the suspend to hang. Also ejecting and rediscovering is actually
>> annoying if the card contains extensions to the home directory or a root
>> filesystem.
>>
>> Fix: This is not a real fix, but a workaround. In the Lucid code there is
>> a compile time only option to allow the mmc/sd card to be mounted and just
>> get reused on resume. Contrary to the unsafe title there arctually is some
>> safeguarding code to check the cards id before blindly using it.
>> The following patch leaves the default as it is now, but allows users to
>> change the mode with a boot parameter or by writing into the modules parameter
>> in sysfs.
>>
>> Testcase: Suspend with mounted SD card hangs on its way down. With removable=0
>> it will go down and come up without apparent problems.
>>
>>
>> From faf4b61efc07ba61195e97b42200331aea030b0d Mon Sep 17 00:00:00 2001
>> From: Ben Hutchings <ben@decadent.org.uk>
>> Date: Mon, 14 Dec 2009 18:01:29 -0800
>> Subject: [PATCH] mmc: add module parameter to set whether cards are assumed removable
>>
>> Some people run general-purpose distribution kernels on netbooks with
>> a card that is physically non-removable or logically non-removable
>> (e.g. used for /home) and cannot be cleanly unmounted during suspend.
>> Add a module parameter to set whether cards are assumed removable or
>> non-removable, with the default set by CONFIG_MMC_UNSAFE_RESUME.
>>
>> In general, it is not possible to tell whether a card present in an MMC
>> slot after resume is the same that was there before suspend.  So there are
>> two possible behaviours, each of which will cause data loss in some cases:
>>
>> CONFIG_MMC_UNSAFE_RESUME=n (default): Cards are assumed to be removed
>> during suspend.  Any filesystem on them must be unmounted before suspend;
>> otherwise, buffered writes will be lost.
>>
>> CONFIG_MMC_UNSAFE_RESUME=y: Cards are assumed to remain present during
>> suspend.  They must not be swapped during suspend; otherwise, buffered
>> writes will be flushed to the wrong card.
>>
>> Currently the choice is made at compile time and this allows that to be
>> overridden at module load time.
>>
>> BugLink: http://bugs.launchpad.net/bugs/477106
>>
>> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>> Cc: Wouter van Heyst <larstiq@larstiq.dyndns.org>
>> Cc: <linux-mmc@vger.kernel.org>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>> (cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> 
> Looks ok to me.  Stefan, will you be building test kernels for bug
> 477106?
> 
Ah, actually I have but only (selfishly) for myself and only for i386. But I
will let it build on ronne and shove over the kernels tomorrow.
[As my Aspire One exactly shows that hang I can at least confirm it seems to
work for me :)]

Stefan
Stefan Bader April 6, 2010, 7:49 a.m. UTC | #3
Stefan Bader wrote:
> Leann Ogasawara wrote:
>> On Sat, 2010-04-03 at 11:50 +0200, Stefan Bader wrote:
>>> SRU Justification:
>>>
>>> Impact: On suspend, mmc cards are ejected on suspend and rediscovered at
>>> resume. There seems to be a bug in that in Lucid as a mounted mmc card
>>> causes the suspend to hang. Also ejecting and rediscovering is actually
>>> annoying if the card contains extensions to the home directory or a root
>>> filesystem.
>>>
>>> Fix: This is not a real fix, but a workaround. In the Lucid code there is
>>> a compile time only option to allow the mmc/sd card to be mounted and just
>>> get reused on resume. Contrary to the unsafe title there arctually is some
>>> safeguarding code to check the cards id before blindly using it.
>>> The following patch leaves the default as it is now, but allows users to
>>> change the mode with a boot parameter or by writing into the modules parameter
>>> in sysfs.
>>>
>>> Testcase: Suspend with mounted SD card hangs on its way down. With removable=0
>>> it will go down and come up without apparent problems.
>>>
>>>
>>> From faf4b61efc07ba61195e97b42200331aea030b0d Mon Sep 17 00:00:00 2001
>>> From: Ben Hutchings <ben@decadent.org.uk>
>>> Date: Mon, 14 Dec 2009 18:01:29 -0800
>>> Subject: [PATCH] mmc: add module parameter to set whether cards are assumed removable
>>>
>>> Some people run general-purpose distribution kernels on netbooks with
>>> a card that is physically non-removable or logically non-removable
>>> (e.g. used for /home) and cannot be cleanly unmounted during suspend.
>>> Add a module parameter to set whether cards are assumed removable or
>>> non-removable, with the default set by CONFIG_MMC_UNSAFE_RESUME.
>>>
>>> In general, it is not possible to tell whether a card present in an MMC
>>> slot after resume is the same that was there before suspend.  So there are
>>> two possible behaviours, each of which will cause data loss in some cases:
>>>
>>> CONFIG_MMC_UNSAFE_RESUME=n (default): Cards are assumed to be removed
>>> during suspend.  Any filesystem on them must be unmounted before suspend;
>>> otherwise, buffered writes will be lost.
>>>
>>> CONFIG_MMC_UNSAFE_RESUME=y: Cards are assumed to remain present during
>>> suspend.  They must not be swapped during suspend; otherwise, buffered
>>> writes will be flushed to the wrong card.
>>>
>>> Currently the choice is made at compile time and this allows that to be
>>> overridden at module load time.
>>>
>>> BugLink: http://bugs.launchpad.net/bugs/477106
>>>
>>> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>>> Cc: Wouter van Heyst <larstiq@larstiq.dyndns.org>
>>> Cc: <linux-mmc@vger.kernel.org>
>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>>> (cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
>>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
>> Looks ok to me.  Stefan, will you be building test kernels for bug
>> 477106?
>>
> Ah, actually I have but only (selfishly) for myself and only for i386. But I
> will let it build on ronne and shove over the kernels tomorrow.
> [As my Aspire One exactly shows that hang I can at least confirm it seems to
> work for me :)]
> 
> Stefan
> 
Ok, its there now and the report has been updated.

Stefan
Robert Hooker April 6, 2010, 11:44 a.m. UTC | #4
On Tue, Apr 6, 2010 at 3:49 AM, Stefan Bader <stefan.bader@canonical.com> wrote:
> Stefan Bader wrote:
>> Leann Ogasawara wrote:
>>> On Sat, 2010-04-03 at 11:50 +0200, Stefan Bader wrote:
>>>> SRU Justification:
>>>>
>>>> Impact: On suspend, mmc cards are ejected on suspend and rediscovered at
>>>> resume. There seems to be a bug in that in Lucid as a mounted mmc card
>>>> causes the suspend to hang. Also ejecting and rediscovering is actually
>>>> annoying if the card contains extensions to the home directory or a root
>>>> filesystem.
>>>>
>>>> Fix: This is not a real fix, but a workaround. In the Lucid code there is
>>>> a compile time only option to allow the mmc/sd card to be mounted and just
>>>> get reused on resume. Contrary to the unsafe title there arctually is some
>>>> safeguarding code to check the cards id before blindly using it.
>>>> The following patch leaves the default as it is now, but allows users to
>>>> change the mode with a boot parameter or by writing into the modules parameter
>>>> in sysfs.
>>>>
>>>> Testcase: Suspend with mounted SD card hangs on its way down. With removable=0
>>>> it will go down and come up without apparent problems.
>>>>
>>>>
>>>> From faf4b61efc07ba61195e97b42200331aea030b0d Mon Sep 17 00:00:00 2001
>>>> From: Ben Hutchings <ben@decadent.org.uk>
>>>> Date: Mon, 14 Dec 2009 18:01:29 -0800
>>>> Subject: [PATCH] mmc: add module parameter to set whether cards are assumed removable
>>>>
>>>> Some people run general-purpose distribution kernels on netbooks with
>>>> a card that is physically non-removable or logically non-removable
>>>> (e.g. used for /home) and cannot be cleanly unmounted during suspend.
>>>> Add a module parameter to set whether cards are assumed removable or
>>>> non-removable, with the default set by CONFIG_MMC_UNSAFE_RESUME.
>>>>
>>>> In general, it is not possible to tell whether a card present in an MMC
>>>> slot after resume is the same that was there before suspend.  So there are
>>>> two possible behaviours, each of which will cause data loss in some cases:
>>>>
>>>> CONFIG_MMC_UNSAFE_RESUME=n (default): Cards are assumed to be removed
>>>> during suspend.  Any filesystem on them must be unmounted before suspend;
>>>> otherwise, buffered writes will be lost.
>>>>
>>>> CONFIG_MMC_UNSAFE_RESUME=y: Cards are assumed to remain present during
>>>> suspend.  They must not be swapped during suspend; otherwise, buffered
>>>> writes will be flushed to the wrong card.
>>>>
>>>> Currently the choice is made at compile time and this allows that to be
>>>> overridden at module load time.
>>>>
>>>> BugLink: http://bugs.launchpad.net/bugs/477106
>>>>
>>>> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
>>>> Cc: Wouter van Heyst <larstiq@larstiq.dyndns.org>
>>>> Cc: <linux-mmc@vger.kernel.org>
>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
>>>> (cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
>>>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
>>> Looks ok to me.  Stefan, will you be building test kernels for bug
>>> 477106?
>>>
>> Ah, actually I have but only (selfishly) for myself and only for i386. But I
>> will let it build on ronne and shove over the kernels tomorrow.
>> [As my Aspire One exactly shows that hang I can at least confirm it seems to
>> work for me :)]
>>
>> Stefan
>>
> Ok, its there now and the report has been updated.
>
> Stefan
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>


Hmm, unfortunately this doesn't seem to work for me. On my own kernels
I build mmc/sdhci/sdhci-pci into the kernel instead of having them
modular and in that case even booting with mmc.removable=1 (aka no
unsafe resume) suspend/resume works correctly. I tried your 2.6.32-19
kernel out booting with mmc.removable=0 (to enable unsafe resume) and
unfortunately it still hangs with the image still on the screen when I
try to suspend with the SD mounted on my Acer Aspire One AOA150. What
bios revision are you using on your AOA110 where it is working? Are
you using any pciehp or sdhci module options? Are you using the left
SD slot? What filesystem is your SD using? I tried my normal ext4 with
no journal and fat32.


my /var/log/pm-suspend.log from the failed one with mmc.removable=0:

Tue Apr  6 07:03:46 EDT 2010: performing suspend
Initial commandline parameters:
Tue Apr  6 07:05:34 EDT 2010: Running hooks for suspend.
/usr/lib/pm-utils/sleep.d/000kernel-change suspend suspend:success.
/usr/lib/pm-utils/sleep.d/00logging suspend suspend:Linux asuka
2.6.32-19-generic #28+lp477106v1 SMP Mon Apr 5 19:40:35 UTC 2010 i686
GNU/Linux
Module                  Size  Used by
binfmt_misc             6587  1
microcode              10076  0
joydev                  8708  0
snd_hda_codec_realtek   203073  1
snd_hda_intel          21781  2
snd_hda_codec          74201  2 snd_hda_codec_realtek,snd_hda_intel
snd_hwdep               5412  1 snd_hda_codec
snd_pcm_oss            35308  0
snd_mixer_oss          13746  1 snd_pcm_oss
snd_pcm                70662  3 snd_hda_intel,snd_hda_codec,snd_pcm_oss
snd_seq_dummy           1338  0
snd_seq_oss            26726  0
snd_seq_midi            4557  0
arc4                    1153  2
snd_rawmidi            19056  1 snd_seq_midi
snd_seq_midi_event      6003  2 snd_seq_oss,snd_seq_midi
mmc_block               8258  2
snd_seq                47295  6
snd_seq_dummy,snd_seq_oss,snd_seq_midi,snd_seq_midi_event
snd_timer              19098  2 snd_pcm,snd_seq
snd_seq_device          5700  5
snd_seq_dummy,snd_seq_oss,snd_seq_midi,snd_rawmidi,snd_seq
b43                   157250  0
uvcvideo               56990  0
snd                    54148  16
snd_hda_codec_realtek,snd_hda_intel,snd_hda_codec,snd_hwdep,snd_pcm_oss,snd_mixer_oss,snd_pcm,snd_seq_oss,snd
mac80211              204566  1 b43
acerhdf                 6691  0
jmb38x_ms               7311  0
sdhci_pci               5470  0
videodev               34361  1 uvcvideo
v4l1_compat            13251  2 uvcvideo,videodev
psmouse                62957  0
serio_raw               3978  0
cfg80211              126581  2 b43,mac80211
sdhci                  15462  1 sdhci_pci
led_class               2864  2 b43,sdhci
memstick                8237  1 jmb38x_ms
soundcore               6620  1 snd
snd_page_alloc          7204  2 snd_hda_intel,snd_pcm
fbcon                  35102  71
tileblit                2031  1 fbcon
font                    7557  1 fbcon
bitblit                 4707  1 fbcon
softcursor              1189  1 bitblit
i915                  282014  3
drm_kms_helper         29297  1 i915
drm                   162631  4 i915,drm_kms_helper
i2c_algo_bit            5028  1 i915
intel_agp              24181  1
r8169                  34172  0
video                  17375  1 i915
output                  1871  1 video
mii                     4381  1 r8169
ssb                    37592  1 b43
agpgart                31724  2 drm,intel_agp
             total       used       free     shared    buffers     cached
Mem:       1532292     400116    1132176          0      89820     167740
-/+ buffers/cache:     142556    1389736
Swap:            0          0          0
success.
/usr/lib/pm-utils/sleep.d/00powersave suspend suspend:success.
/usr/lib/pm-utils/sleep.d/01PulseAudio suspend suspend:success.
/etc/pm/sleep.d/10_grub-common suspend suspend:success.
/etc/pm/sleep.d/10_unattended-upgrades-hibernate suspend suspend:success.
/usr/lib/pm-utils/sleep.d/49bluetooth suspend suspend:not applicable.
/usr/lib/pm-utils/sleep.d/55NetworkManager suspend suspend:not applicable.
/usr/lib/pm-utils/sleep.d/75modules suspend suspend:success.
/usr/lib/pm-utils/sleep.d/90clock suspend suspend:not applicable.
/usr/lib/pm-utils/sleep.d/94cpufreq suspend suspend:success.
/usr/lib/pm-utils/sleep.d/95anacron suspend suspend:stop: Unknown instance:
success.
/usr/lib/pm-utils/sleep.d/95led suspend suspend:not applicable.
/usr/lib/pm-utils/sleep.d/95packagekit suspend suspend:success.
/usr/lib/pm-utils/sleep.d/98-video-quirk-db-handler suspend suspend:success.
/usr/lib/pm-utils/sleep.d/99video suspend suspend:kernel.acpi_video_flags = 0
success.
/etc/pm/sleep.d/action_wpa suspend suspend:success.
Tue Apr  6 07:05:35 EDT 2010: performing suspend
Andy Whitcroft April 8, 2010, 4:42 p.m. UTC | #5
On Sat, Apr 03, 2010 at 11:50:07AM +0200, Stefan Bader wrote:
> SRU Justification:
> 
> Impact: On suspend, mmc cards are ejected on suspend and rediscovered at
> resume. There seems to be a bug in that in Lucid as a mounted mmc card
> causes the suspend to hang. Also ejecting and rediscovering is actually
> annoying if the card contains extensions to the home directory or a root
> filesystem.
> 
> Fix: This is not a real fix, but a workaround. In the Lucid code there is
> a compile time only option to allow the mmc/sd card to be mounted and just
> get reused on resume. Contrary to the unsafe title there arctually is some
> safeguarding code to check the cards id before blindly using it.
> The following patch leaves the default as it is now, but allows users to
> change the mode with a boot parameter or by writing into the modules parameter
> in sysfs.
> 
> Testcase: Suspend with mounted SD card hangs on its way down. With removable=0
> it will go down and come up without apparent problems.
> 
> 
> From faf4b61efc07ba61195e97b42200331aea030b0d Mon Sep 17 00:00:00 2001
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Mon, 14 Dec 2009 18:01:29 -0800
> Subject: [PATCH] mmc: add module parameter to set whether cards are assumed removable
> 
> Some people run general-purpose distribution kernels on netbooks with
> a card that is physically non-removable or logically non-removable
> (e.g. used for /home) and cannot be cleanly unmounted during suspend.
> Add a module parameter to set whether cards are assumed removable or
> non-removable, with the default set by CONFIG_MMC_UNSAFE_RESUME.
> 
> In general, it is not possible to tell whether a card present in an MMC
> slot after resume is the same that was there before suspend.  So there are
> two possible behaviours, each of which will cause data loss in some cases:
> 
> CONFIG_MMC_UNSAFE_RESUME=n (default): Cards are assumed to be removed
> during suspend.  Any filesystem on them must be unmounted before suspend;
> otherwise, buffered writes will be lost.
> 
> CONFIG_MMC_UNSAFE_RESUME=y: Cards are assumed to remain present during
> suspend.  They must not be swapped during suspend; otherwise, buffered
> writes will be flushed to the wrong card.
> 
> Currently the choice is made at compile time and this allows that to be
> overridden at module load time.
> 
> BugLink: http://bugs.launchpad.net/bugs/477106
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Cc: Wouter van Heyst <larstiq@larstiq.dyndns.org>
> Cc: <linux-mmc@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> (cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  drivers/mmc/core/Kconfig |    4 +++-
>  drivers/mmc/core/core.c  |   16 ++++++++++++++++
>  drivers/mmc/core/core.h  |    2 ++
>  drivers/mmc/core/mmc.c   |   23 +----------------------
>  drivers/mmc/core/sd.c    |   21 +--------------------
>  5 files changed, 23 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
> index ab37a6d..bb22ffd 100644
> --- a/drivers/mmc/core/Kconfig
> +++ b/drivers/mmc/core/Kconfig
> @@ -3,7 +3,7 @@
>  #
>  
>  config MMC_UNSAFE_RESUME
> -	bool "Allow unsafe resume (DANGEROUS)"
> +	bool "Assume MMC/SD cards are non-removable (DANGEROUS)"
>  	help
>  	  If you say Y here, the MMC layer will assume that all cards
>  	  stayed in their respective slots during the suspend. The
> @@ -14,3 +14,5 @@ config MMC_UNSAFE_RESUME
>  	  This option is usually just for embedded systems which use
>  	  a MMC/SD card for rootfs. Most people should say N here.
>  
> +	  This option sets a default which can be overridden by the
> +	  module parameter "removable=0" or "removable=1".
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 7dab2e5..30acd52 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -48,6 +48,22 @@ int use_spi_crc = 1;
>  module_param(use_spi_crc, bool, 0);
>  
>  /*
> + * We normally treat cards as removed during suspend if they are not
> + * known to be on a non-removable bus, to avoid the risk of writing
> + * back data to a different card after resume.  Allow this to be
> + * overridden if necessary.
> + */
> +#ifdef CONFIG_MMC_UNSAFE_RESUME
> +int mmc_assume_removable;
> +#else
> +int mmc_assume_removable = 1;
> +#endif
> +module_param_named(removable, mmc_assume_removable, bool, 0644);
> +MODULE_PARM_DESC(
> +	removable,
> +	"MMC/SD cards are removable and may be removed during suspend");
> +
> +/*
>   * Internal function. Schedule delayed work in the MMC work queue.
>   */
>  static int mmc_schedule_delayed_work(struct delayed_work *work,
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index 67ae6ab..a811c52 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -54,7 +54,9 @@ int mmc_attach_mmc(struct mmc_host *host, u32 ocr);
>  int mmc_attach_sd(struct mmc_host *host, u32 ocr);
>  int mmc_attach_sdio(struct mmc_host *host, u32 ocr);
>  
> +/* Module parameters */
>  extern int use_spi_crc;
> +extern int mmc_assume_removable;
>  
>  /* Debugfs information for hosts and cards */
>  void mmc_add_host_debugfs(struct mmc_host *host);
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index bfefce3..c111894 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -602,25 +602,6 @@ static int mmc_awake(struct mmc_host *host)
>  	return err;
>  }
>  
> -#ifdef CONFIG_MMC_UNSAFE_RESUME
> -
> -static const struct mmc_bus_ops mmc_ops = {
> -	.awake = mmc_awake,
> -	.sleep = mmc_sleep,
> -	.remove = mmc_remove,
> -	.detect = mmc_detect,
> -	.suspend = mmc_suspend,
> -	.resume = mmc_resume,
> -	.power_restore = mmc_power_restore,
> -};
> -
> -static void mmc_attach_bus_ops(struct mmc_host *host)
> -{
> -	mmc_attach_bus(host, &mmc_ops);
> -}
> -
> -#else
> -
>  static const struct mmc_bus_ops mmc_ops = {
>  	.awake = mmc_awake,
>  	.sleep = mmc_sleep,
> @@ -645,15 +626,13 @@ static void mmc_attach_bus_ops(struct mmc_host *host)
>  {
>  	const struct mmc_bus_ops *bus_ops;
>  
> -	if (host->caps & MMC_CAP_NONREMOVABLE)
> +	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
>  		bus_ops = &mmc_ops_unsafe;
>  	else
>  		bus_ops = &mmc_ops;
>  	mmc_attach_bus(host, bus_ops);
>  }
>  
> -#endif
> -
>  /*
>   * Starting point for MMC card init.
>   */
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 10b2a4d..fdd414e 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -606,23 +606,6 @@ static void mmc_sd_power_restore(struct mmc_host *host)
>  	mmc_release_host(host);
>  }
>  
> -#ifdef CONFIG_MMC_UNSAFE_RESUME
> -
> -static const struct mmc_bus_ops mmc_sd_ops = {
> -	.remove = mmc_sd_remove,
> -	.detect = mmc_sd_detect,
> -	.suspend = mmc_sd_suspend,
> -	.resume = mmc_sd_resume,
> -	.power_restore = mmc_sd_power_restore,
> -};
> -
> -static void mmc_sd_attach_bus_ops(struct mmc_host *host)
> -{
> -	mmc_attach_bus(host, &mmc_sd_ops);
> -}
> -
> -#else
> -
>  static const struct mmc_bus_ops mmc_sd_ops = {
>  	.remove = mmc_sd_remove,
>  	.detect = mmc_sd_detect,
> @@ -643,15 +626,13 @@ static void mmc_sd_attach_bus_ops(struct mmc_host *host)
>  {
>  	const struct mmc_bus_ops *bus_ops;
>  
> -	if (host->caps & MMC_CAP_NONREMOVABLE)
> +	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
>  		bus_ops = &mmc_sd_ops_unsafe;
>  	else
>  		bus_ops = &mmc_sd_ops;
>  	mmc_attach_bus(host, bus_ops);
>  }
>  
> -#endif
> -
>  /*
>   * Starting point for SD card init.
>   */

Default behaviour should be the same, new options to override.  Seems
sane.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Andy Whitcroft April 8, 2010, 4:48 p.m. UTC | #6
Applied to Lucid.

-apw
diff mbox

Patch

diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index ab37a6d..bb22ffd 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -3,7 +3,7 @@ 
 #
 
 config MMC_UNSAFE_RESUME
-	bool "Allow unsafe resume (DANGEROUS)"
+	bool "Assume MMC/SD cards are non-removable (DANGEROUS)"
 	help
 	  If you say Y here, the MMC layer will assume that all cards
 	  stayed in their respective slots during the suspend. The
@@ -14,3 +14,5 @@  config MMC_UNSAFE_RESUME
 	  This option is usually just for embedded systems which use
 	  a MMC/SD card for rootfs. Most people should say N here.
 
+	  This option sets a default which can be overridden by the
+	  module parameter "removable=0" or "removable=1".
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7dab2e5..30acd52 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -48,6 +48,22 @@  int use_spi_crc = 1;
 module_param(use_spi_crc, bool, 0);
 
 /*
+ * We normally treat cards as removed during suspend if they are not
+ * known to be on a non-removable bus, to avoid the risk of writing
+ * back data to a different card after resume.  Allow this to be
+ * overridden if necessary.
+ */
+#ifdef CONFIG_MMC_UNSAFE_RESUME
+int mmc_assume_removable;
+#else
+int mmc_assume_removable = 1;
+#endif
+module_param_named(removable, mmc_assume_removable, bool, 0644);
+MODULE_PARM_DESC(
+	removable,
+	"MMC/SD cards are removable and may be removed during suspend");
+
+/*
  * Internal function. Schedule delayed work in the MMC work queue.
  */
 static int mmc_schedule_delayed_work(struct delayed_work *work,
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 67ae6ab..a811c52 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -54,7 +54,9 @@  int mmc_attach_mmc(struct mmc_host *host, u32 ocr);
 int mmc_attach_sd(struct mmc_host *host, u32 ocr);
 int mmc_attach_sdio(struct mmc_host *host, u32 ocr);
 
+/* Module parameters */
 extern int use_spi_crc;
+extern int mmc_assume_removable;
 
 /* Debugfs information for hosts and cards */
 void mmc_add_host_debugfs(struct mmc_host *host);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index bfefce3..c111894 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -602,25 +602,6 @@  static int mmc_awake(struct mmc_host *host)
 	return err;
 }
 
-#ifdef CONFIG_MMC_UNSAFE_RESUME
-
-static const struct mmc_bus_ops mmc_ops = {
-	.awake = mmc_awake,
-	.sleep = mmc_sleep,
-	.remove = mmc_remove,
-	.detect = mmc_detect,
-	.suspend = mmc_suspend,
-	.resume = mmc_resume,
-	.power_restore = mmc_power_restore,
-};
-
-static void mmc_attach_bus_ops(struct mmc_host *host)
-{
-	mmc_attach_bus(host, &mmc_ops);
-}
-
-#else
-
 static const struct mmc_bus_ops mmc_ops = {
 	.awake = mmc_awake,
 	.sleep = mmc_sleep,
@@ -645,15 +626,13 @@  static void mmc_attach_bus_ops(struct mmc_host *host)
 {
 	const struct mmc_bus_ops *bus_ops;
 
-	if (host->caps & MMC_CAP_NONREMOVABLE)
+	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
 		bus_ops = &mmc_ops_unsafe;
 	else
 		bus_ops = &mmc_ops;
 	mmc_attach_bus(host, bus_ops);
 }
 
-#endif
-
 /*
  * Starting point for MMC card init.
  */
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index 10b2a4d..fdd414e 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -606,23 +606,6 @@  static void mmc_sd_power_restore(struct mmc_host *host)
 	mmc_release_host(host);
 }
 
-#ifdef CONFIG_MMC_UNSAFE_RESUME
-
-static const struct mmc_bus_ops mmc_sd_ops = {
-	.remove = mmc_sd_remove,
-	.detect = mmc_sd_detect,
-	.suspend = mmc_sd_suspend,
-	.resume = mmc_sd_resume,
-	.power_restore = mmc_sd_power_restore,
-};
-
-static void mmc_sd_attach_bus_ops(struct mmc_host *host)
-{
-	mmc_attach_bus(host, &mmc_sd_ops);
-}
-
-#else
-
 static const struct mmc_bus_ops mmc_sd_ops = {
 	.remove = mmc_sd_remove,
 	.detect = mmc_sd_detect,
@@ -643,15 +626,13 @@  static void mmc_sd_attach_bus_ops(struct mmc_host *host)
 {
 	const struct mmc_bus_ops *bus_ops;
 
-	if (host->caps & MMC_CAP_NONREMOVABLE)
+	if (host->caps & MMC_CAP_NONREMOVABLE || !mmc_assume_removable)
 		bus_ops = &mmc_sd_ops_unsafe;
 	else
 		bus_ops = &mmc_sd_ops;
 	mmc_attach_bus(host, bus_ops);
 }
 
-#endif
-
 /*
  * Starting point for SD card init.
  */