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

Mon Apr 5 15:24:46 UTC 2010

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 at 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 at decadent.org.uk>
> Cc: Wouter van Heyst <larstiq at larstiq.dyndns.org>
> Cc: <linux-mmc at vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> (cherry-picked from commit bd68e0838fe85794b06892054772fa013a8d1986 upstream)
> Signed-off-by: Stefan Bader <stefan.bader at canonical.com>

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

Acked-by: Leann Ogasawara <leann.ogasawara at 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
> 
> 