Message ID | 1270288207-10441-1-git-send-email-stefan.bader@canonical.com |
---|---|
State | Accepted |
Delegated to: | Andy Whitcroft |
Headers | show |
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 > >
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 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
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
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
Applied to Lucid. -apw
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. */