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.
Signed-off-by: Ben Hutchings <[email protected]>
---
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 d98b0e2..010c964 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 1c68783..d20b7bc 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -64,7 +64,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.5.2
On Wed, 11 Nov 2009 04:44:36 +0000
Ben Hutchings <[email protected]> wrote:
> 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.
>
The description really doesn't give me enough info to work out what's
happening here and why this is being proposed. But it smells nasty.
> 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 d98b0e2..010c964 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.
> + */
So we have a module parameter which nobody knows about. If they don't
set this parameter which they don't know about, the kernel will trash
their filesystem??
> +#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");
> +
On Mon, 2009-11-16 at 12:23 -0800, Andrew Morton wrote:
> On Wed, 11 Nov 2009 04:44:36 +0000
> Ben Hutchings <[email protected]> wrote:
>
> > 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.
> >
>
> The description really doesn't give me enough info to work out what's
> happening here and why this is being proposed. But it smells nasty.
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.
[...]
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index d98b0e2..010c964 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.
> > + */
>
> So we have a module parameter which nobody knows about. If they don't
> set this parameter which they don't know about, the kernel will trash
> their filesystem??
[...]
No, because it's set to 1 by default. There is no change in the default
behaviour.
Ben.
--
Ben Hutchings
The two most common things in the universe are hydrogen and stupidity.
Ben Hutchings wrote:
> 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.
That's true for virtually all storage devices, not just MMC.
> 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.
Can't the kernel flush the write buffer at suspend time, so that you can
remove this choice for good?
--
Stefan Richter
-=====-==--= =-== =---=
http://arcgraph.de/sr/
On Mon, Nov 16, 2009 at 10:31:49PM +0000, Ben Hutchings wrote:
> On Mon, 2009-11-16 at 12:23 -0800, Andrew Morton wrote:
> > On Wed, 11 Nov 2009 04:44:36 +0000
> > Ben Hutchings <[email protected]> wrote:
> >
> > > 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.
> > >
> >
> > The description really doesn't give me enough info to work out what's
> > happening here and why this is being proposed. But it smells nasty.
>
> 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.
I'm running 2.6.32-rc7 with this patch applied and CONFIG_MMC_UNSAFE_RESUME=y
That works as desired for my non-removable case. Is it desired that I
test if 'removable=1' will thrash my filesystem?
Wouter van Heyst
On Sun, 2009-11-22 at 12:42 +0100, Wouter van Heyst wrote:
> On Mon, Nov 16, 2009 at 10:31:49PM +0000, Ben Hutchings wrote:
[...]
> > 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.
>
> I'm running 2.6.32-rc7 with this patch applied and CONFIG_MMC_UNSAFE_RESUME=y
> That works as desired for my non-removable case. Is it desired that I
> test if 'removable=1' will thrash my filesystem?
Please test with CONFIG_MMC_UNSAFE_RESUME=n (which Debian will continue
to use) and removable=0.
Ben.
--
Ben Hutchings
Unix is many things to many people,
but it's never been everything to anybody.
Ben Hutchings wrote:
> 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.
>
> Signed-off-by: Ben Hutchings <[email protected]>
> ---
You do not cater for having more than one slot e.g. N900 has two:
one internal non-removable and one external that is removable.
What about a sysfs entry instead e.g.
/sys/class/mmc-host/mmc*/nonremovable
> 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 d98b0e2..010c964 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 1c68783..d20b7bc 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -64,7 +64,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.
> */
On Tue, 17 Nov 2009 08:53:00 +0100
Stefan Richter <[email protected]> wrote:
> Ben Hutchings wrote:
> > 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.
>
> That's true for virtually all storage devices, not just MMC.
>
> > 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.
>
> Can't the kernel flush the write buffer at suspend time, so that you can
> remove this choice for good?
I'm afraid that's insufficient. What it would need to do is to is
flush everything (to make sure what's on disk matches what's in
memory), but also read back the filesystem on resume to verify that
nothing else modified it (i.e. making sure what's on disk still matches
what's in memory).
Another way of putting it is that the kernel needs to umount/mount
around suspend in a way that's transparent to users of the filesystem.
Until we have such a system in place then everything will be hacks
which only shift around the problem.
Rgds
--
-- Pierre Ossman
WARNING: This correspondence is being monitored by FRA, a
Swedish intelligence agency. Make sure your server uses
encryption for SMTP traffic and consider using PGP for
end-to-end encryption.
> You do not cater for having more than one slot e.g. N900 has two:
> one internal non-removable and one external that is removable.
>
> What about a sysfs entry instead e.g.
>
> /sys/class/mmc-host/mmc*/nonremovable
That continues the assumption that the user will somehow ever know about
this stuff and configure it. Almost nobody will.
Not only does it need to be per-port there needs to be a credible
description of how it will automatically happen - who configures it,
when, how and is the data needed available.
> I'm afraid that's insufficient. What it would need to do is to is
> flush everything (to make sure what's on disk matches what's in
> memory), but also read back the filesystem on resume to verify that
> nothing else modified it (i.e. making sure what's on disk still matches
> what's in memory).
For most file systems it is sufficient to check the superblock related
information. So we'd need an fs->ops->validate_media() or somesuch but it
wouldn't be that horrific or need to do much I/O in most cases.
You could defeat that by being really stupid, but the purpose of the
check isn't a stupidity filter but to stop accidents happening in normal
use.
> Another way of putting it is that the kernel needs to umount/mount
> around suspend in a way that's transparent to users of the filesystem.
No. The kernel needs to push stuff to media on suspend (which is good
manners anyway), and validate on resume. if the validate fails you mark
the media as changed and the block layer will already see to it that
everything gets aborted as it already does with a truely removable device.
In fact if you did this by media serial numbers and idents you don't even
need the fs hook, although it would certainly be safer that way.
Alan
On Mon, 30 Nov 2009 12:54:05 +0000
Alan Cox <[email protected]> wrote:
>
> For most file systems it is sufficient to check the superblock related
> information. So we'd need an fs->ops->validate_media() or somesuch but it
> wouldn't be that horrific or need to do much I/O in most cases.
>
> You could defeat that by being really stupid, but the purpose of the
> check isn't a stupidity filter but to stop accidents happening in normal
> use.
>
Agreed. Something like that would more or less solve the issue. Someone
just needs to write the code for all (or most) filesystems.
> > Another way of putting it is that the kernel needs to umount/mount
> > around suspend in a way that's transparent to users of the filesystem.
>
> No. The kernel needs to push stuff to media on suspend (which is good
> manners anyway), and validate on resume. if the validate fails you mark
> the media as changed and the block layer will already see to it that
> everything gets aborted as it already does with a truely removable device.
>
> In fact if you did this by media serial numbers and idents you don't even
> need the fs hook, although it would certainly be safer that way.
>
The hardware driver layer can only check if it's the same device being
plugged in, not if someone has done something with it during suspend,
so I see no other way than solving this in the filesystem layer.
Rgds
--
-- Pierre Ossman
WARNING: This correspondence is being monitored by FRA, a
Swedish intelligence agency. Make sure your server uses
encryption for SMTP traffic and consider using PGP for
end-to-end encryption.
On Mon, 2009-11-30 at 13:39 +0100, Pierre Ossman wrote:
> On Tue, 17 Nov 2009 08:53:00 +0100
> Stefan Richter <[email protected]> wrote:
>
> > Ben Hutchings wrote:
> > > 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.
> >
> > That's true for virtually all storage devices, not just MMC.
> >
> > > 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.
> >
> > Can't the kernel flush the write buffer at suspend time, so that you can
> > remove this choice for good?
>
> I'm afraid that's insufficient. What it would need to do is to is
> flush everything (to make sure what's on disk matches what's in
> memory), but also read back the filesystem on resume to verify that
> nothing else modified it (i.e. making sure what's on disk still matches
> what's in memory).
I had just an idea.
Before we do suspend, pick few random sectors from the media, run that
through some hash function, thus creating some sort of watermark.
On resume do that again.
This should catch all unintentional card swappings.
Of course this is only half of the problem, because user could edit the
card meanwhile, but still solving half the problem is better that
nothing?
What do you think ?
Best regards,
Maxim Levitsky
> Before we do suspend, pick few random sectors from the media, run that
> through some hash function, thus creating some sort of watermark.
Statistically speaking the chances are you'll catch zero sectors and
lose. You'll also not detect the suspend, move to other box, use, put
back error. That is one users make and we need to be at least vaguely
robust against.
Hence you need the fs checking here.
Alan
On Mon, 2009-11-30 at 13:51 +0000, Alan Cox wrote:
> > Before we do suspend, pick few random sectors from the media, run that
> > through some hash function, thus creating some sort of watermark.
>
> Statistically speaking the chances are you'll catch zero sectors and
> lose. You'll also not detect the suspend, move to other box, use, put
> back error. That is one users make and we need to be at least vaguely
> robust against.
>
> Hence you need the fs checking here.
>
> Alan
I have to agree with you about that one.
An FS checking is really only solution.
Then I think such check can be added gradually to existing filesystems
(starting with fat), and allow these filesystems to persist across low
power states regardsless of CONFIG_$system_UNSAFE_RESUME
For fat, simple checksum of the 'fat' table will catch most attempts.
Also directory modification times can be compared, at least for root
directory.
Best regards,
Maxim Levitsky
On Sun, Nov 22, 2009 at 12:32:16PM +0000, Ben Hutchings wrote:
> On Sun, 2009-11-22 at 12:42 +0100, Wouter van Heyst wrote:
...
> > I'm running 2.6.32-rc7 with this patch applied and CONFIG_MMC_UNSAFE_RESUME=y
> > That works as desired for my non-removable case. Is it desired that I
> > test if 'removable=1' will thrash my filesystem?
>
> Please test with CONFIG_MMC_UNSAFE_RESUME=n (which Debian will continue
> to use) and removable=0.
It took a while to get around to it correctly, but yes this works.
Without setting removable=0 my Acer Aspire One hangs while trying to
resume if the mmc card is mounted.
Wouter van Heyst