On Friday 17 July 2015 17:30:45 Mike Snitzer wrote:

On Fri, Jul 17 2015 at 11:22am -0400,

Mike Snitzer [off-list ref] wrote:

quoted

On Fri, Jul 17 2015 at 10:22am -0400,

Pali Rohár [off-list ref] wrote:

quoted

On Friday 17 July 2015 10:04:39 Mike Snitzer wrote:

quoted

On Sun, Jun 21 2015 at  7:20am -0400,

Pali Rohár [off-list ref] wrote:

quoted

This patch exports function dm_suspend_md() which suspend
mapped device so other kernel drivers can use it and could
suspend mapped device when needed.

Signed-off-by: Pali Rohár <redacted>
---

 drivers/md/dm.c |    6 ++++++
 drivers/md/dm.h |    5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 2caf492..03298ff 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c

@@ -3343,6 +3343,12 @@ out:
 	return r;
 
 }

+int dm_suspend_md(struct mapped_device *md)
+{
+	return dm_suspend(md, DM_SUSPEND_LOCKFS_FLAG);
+}
+EXPORT_SYMBOL_GPL(dm_suspend_md);
+

 /*
 
  * Internal suspend/resume works like userspace-driven
  suspend. It waits * until all bios finish and prevents
  issuing new bios to the target drivers.

To do this properly you should be introducing a variant of
dm_internal_suspend.  We currently have two variants:
dm_internal_suspend_fast
dm_internal_suspend_noflush

The reason to use a dm_internal_suspend variant is this suspend
was _not_ initiated by an upper level ioctl (from userspace). 
It was done internally from within the target.

You're explicitly using DM_SUSPEND_LOCKFS_FLAG above.. meaning
you're interested in flushing all pending IO (in the FS
layered on dm-crupt, if one exists).

But see the comment in __dm_internal_suspend() about
TASK_UNINTERRUPTIBLE. If you're OK with the dm-crypt initiated
suspend being TASK_UNINTERRUPTIBLE then you could just
introduce:

void dm_internal_suspend_uninterruptible_flush(struct
mapped_device *md) {

        mutex_lock(&md->suspend_lock);
        __dm_internal_suspend(md, DM_SUSPEND_LOCKFS_FLAG);
        mutex_unlock(&md->suspend_lock);

}
EXPORT_SYMBOL_GPL(dm_internal_suspend_uninterruptible_flush);

Otherwise, there is much more extensive DM core changes needed
to __dm_internal_suspend() and .presuspend to properly support
TASK_INTERRUPTIBLE.

Hi! I will look at dm_internal_suspend. Anyway use case for
suspend is from dm-crypt to do both operations: suspend + key
wipe. It means that without entering key again from userspace,
resume is not possible. So my question is: It is possible to do
internal suspend and then using resume from userspace via ioctl?

Good question: no, userspace resume would block waiting for
internal resume.

Soooo... I'll have to look at your patch 3 to understand why
dm-crypt is needing to initiate the suspend internally but then
userspace is initiating the resume... this imbalance is
concerning.

Why not introduce a new message that allows you to wipe the key after
suspend?  Both initiated from userspace.

There is already message for wiping key and it will success only if dm 
is suspended.

But this patch series is fixing another problem: wipe key before 
suspend/hibernation action happend and to have it race free it must be 
done after userspace is freezed!

-- 
Pali Rohár
pali.rohar@gmail.com