On Mon, 5 Jan 2015, Mike Snitzer wrote:
> On Fri, Jan 02 2015 at 5:56pm -0500,
> Mikulas Patocka <mpatocka@xxxxxxxxxx> wrote:
>
> > Hi, I'm back.
> >
> > I looked at current internal suspend code in 3.19.
> >
> > One thing that seems strange is this:
> >
> > static void __dm_internal_suspend(struct mapped_device *md, unsigned suspend_flags)
> > {
> > struct dm_table *map = NULL;
> >
> > if (dm_suspended_internally_md(md))
> > return; /* nested internal suspend */
> > ....
> > static void __dm_internal_resume(struct mapped_device *md)
> > {
> > if (!dm_suspended_internally_md(md))
> > return; /* resume from nested internal suspend */
> >
> > - the comments imply that the code supports nested internal suspend, while
> > in fact it doesn't. If the caller calls:
> > * dm_internal_suspend_noflush
> > * dm_internal_suspend_noflush
> > * dm_internal_resume
> > the device is not suspended while it should be.
>
> Yes, very true, it should. But IIRC in practice it doesn't hurt us to
> not be bothered about being so precise given the current caller.
If the device is not suspended while it should be, it could lead to
various subtle bugs. We should either disallow multiple internal suspends
(print warning when that happens) or handle it correctly. Here I'm sending
a patch to fix it.
Mikulas
From: Mikulas Patocka <mpatocka@xxxxxxxxxx>
The current code attempts to handle multiple internal suspends on the same
device, but it does that incorrectly. When these functions are called in
this order on the same device
dm_internal_suspend_noflush
dm_internal_suspend_noflush
dm_internal_resume
the device is not suspended, while it should be. This patch fixes the bug
by maintaining a counter of suspends, and resuming the device when the
counter drops to zero.
Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
---
drivers/md/dm.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
Index: linux-3.19-rc3/drivers/md/dm.c
===================================================================
--- linux-3.19-rc3.orig/drivers/md/dm.c 2015-01-08 22:04:23.000000000 +0100
+++ linux-3.19-rc3/drivers/md/dm.c 2015-01-08 22:22:14.000000000 +0100
@@ -206,6 +206,9 @@ struct mapped_device {
/* zero-length flush that will be cloned and submitted to targets */
struct bio flush_bio;
+ /* the number of internal suspends */
+ unsigned internal_suspend_count;
+
struct dm_stats stats;
};
@@ -3023,7 +3026,7 @@ static void __dm_internal_suspend(struct
{
struct dm_table *map = NULL;
- if (dm_suspended_internally_md(md))
+ if (md->internal_suspend_count++)
return; /* nested internal suspend */
if (dm_suspended_md(md)) {
@@ -3048,7 +3051,9 @@ static void __dm_internal_suspend(struct
static void __dm_internal_resume(struct mapped_device *md)
{
- if (!dm_suspended_internally_md(md))
+ BUG_ON(!md->internal_suspend_count);
+
+ if (--md->internal_suspend_count)
return; /* resume from nested internal suspend */
if (dm_suspended_md(md))
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
