dm-cache not writing out cache metadata at reboot?

dm-cache not writing out cache metadata at reboot?

[Darrick J. Wong]

Hi,

So I've been watching the hit/miss counters in dmcache and I've noticed a
couple of things that look like errors to me:

First, I noticed that if I reboot the system, neither cache_postsuspend nor
cache_dtr get called.  This might simply be expected behavior, but it means
that the in-memory superblock structure doesn't get written out to disk upon
reboot.  Just to be sure, I put a printk into __commit_transaction.  It prints
out for 'dmsetup info' and 'dmsetup remove' but nothing at reboot.

Second, cache_status calls dm_cache_commit, which writes out a superblock to
the metadata device.  However, there's no call to save_stats to copy the
current values of the counters out to the disk's copy prior to calling
dm_cache_commit.  Therefore, we seem to be writing out stale copies of
superblock fields.

The second one seems fixable with the attached patch, but the first one I don't
know about.  Any ideas?

---
Subject: [PATCH] dmcache: flush superblock when retrieving status info

When userspace queries dmcache for stats info, we should ensure that all the
metadata gets flushed out of memory to disk.  The current code neglects to
update at least the hit/miss counters, so take care of everything.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 drivers/md/dm-cache-target.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 1074409..f476ada 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2451,8 +2451,7 @@ static void cache_status(struct dm_target *ti, status_type_t type,
 	case STATUSTYPE_INFO:
 		/* Commit to ensure statistics aren't out-of-date */
 		if (!(status_flags & DM_STATUS_NOFLUSH_FLAG) && !dm_suspended(ti)) {
-			r = dm_cache_commit(cache->cmd, false);
-			if (r)
+			if (!sync_metadata(cache))
 				DMERR("could not commit metadata for accurate status");
 		}
 

Re: dm-cache not writing out cache metadata at reboot?

[Mike Snitzer]

On Wed, May 08 2013 at  5:48pm -0400,
Darrick J. Wong <darrick.wong@oracle.com> wrote:

> Hi,
> 
> So I've been watching the hit/miss counters in dmcache and I've noticed a
> couple of things that look like errors to me:
> 
> First, I noticed that if I reboot the system, neither cache_postsuspend nor
> cache_dtr get called.  This might simply be expected behavior, but it means
> that the in-memory superblock structure doesn't get written out to disk upon
> reboot.  Just to be sure, I put a printk into __commit_transaction.  It prints
> out for 'dmsetup info' and 'dmsetup remove' but nothing at reboot.

We don't have reboot notifiers that auto-magically tear down an
artbitrary DM stack.  Typically the device shutdown includes unmounting
filesystems, stopping LVM (which tears down DM devices, etc).

So given that we don't have any userspace LVM2 support for dm-cache yet
I'm not surprised by this.  In fact it is expected.
 
> Second, cache_status calls dm_cache_commit, which writes out a superblock to
> the metadata device.  However, there's no call to save_stats to copy the
> current values of the counters out to the disk's copy prior to calling
> dm_cache_commit.  Therefore, we seem to be writing out stale copies of
> superblock fields.
> 
> The second one seems fixable with the attached patch

I'll defer to Joe on this but I think sync_metadata() is pretty heavy to
be doing every 'dmsetup info'.  BTW, with just dm_cache_commit() the
superblock fields aren't stale; only the on-disk hints are.

Mike

Re: dm-cache not writing out cache metadata at reboot?

[Darrick J. Wong]

On Wed, May 08, 2013 at 06:05:26PM -0400, Mike Snitzer wrote:
> On Wed, May 08 2013 at  5:48pm -0400,
> Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> > Hi,
> > 
> > So I've been watching the hit/miss counters in dmcache and I've noticed a
> > couple of things that look like errors to me:
> > 
> > First, I noticed that if I reboot the system, neither cache_postsuspend nor
> > cache_dtr get called.  This might simply be expected behavior, but it means
> > that the in-memory superblock structure doesn't get written out to disk upon
> > reboot.  Just to be sure, I put a printk into __commit_transaction.  It prints
> > out for 'dmsetup info' and 'dmsetup remove' but nothing at reboot.
> 
> We don't have reboot notifiers that auto-magically tear down an
> artbitrary DM stack.  Typically the device shutdown includes unmounting
> filesystems, stopping LVM (which tears down DM devices, etc).
> 
> So given that we don't have any userspace LVM2 support for dm-cache yet
> I'm not surprised by this.  In fact it is expected.

Hmm, I wasn't aware that the lvm2 package had any teardown scripts.  It doesn't
seem to have any in RHEL5.8 or Ubuntu...

> > Second, cache_status calls dm_cache_commit, which writes out a superblock to
> > the metadata device.  However, there's no call to save_stats to copy the
> > current values of the counters out to the disk's copy prior to calling
> > dm_cache_commit.  Therefore, we seem to be writing out stale copies of
> > superblock fields.
> > 
> > The second one seems fixable with the attached patch
> 
> I'll defer to Joe on this but I think sync_metadata() is pretty heavy to
> be doing every 'dmsetup info'.  BTW, with just dm_cache_commit() the
> superblock fields aren't stale; only the on-disk hints are.

How often does dmsetup info run?  I admit that it becomes slower with the
patch, but I didn't think it was really in anyone's hot path.  But given that
there's a comment just prior that says:

/* Commit to ensure statistics aren't out-of-date */

it feels like we ought at least to be calling save_stats() so that we update
the on-disk statistics.  Though, given that the metadata size should be about
10MB for a 100GB cache device, I don't mind flushing out 10MB of metadata to
get the device info.

Really the problem is that with both of these complaints active, the superblock
counters and tables /never/ seem to get updated, even across multiple reboots.
(I'm still digging for why I see such weird unreproduceable benchmark numbers.)

--D

Re: dm-cache not writing out cache metadata at reboot?

[Darrick J. Wong]

On Wed, May 08, 2013 at 06:05:26PM -0400, Mike Snitzer wrote:
> On Wed, May 08 2013 at  5:48pm -0400,
> Darrick J. Wong <darrick.wong@oracle.com> wrote:
> 
> > Hi,
> > 
> > So I've been watching the hit/miss counters in dmcache and I've noticed a
> > couple of things that look like errors to me:
> > 
> > First, I noticed that if I reboot the system, neither cache_postsuspend nor
> > cache_dtr get called.  This might simply be expected behavior, but it means
> > that the in-memory superblock structure doesn't get written out to disk upon
> > reboot.  Just to be sure, I put a printk into __commit_transaction.  It prints
> > out for 'dmsetup info' and 'dmsetup remove' but nothing at reboot.
> 
> We don't have reboot notifiers that auto-magically tear down an
> artbitrary DM stack.  Typically the device shutdown includes unmounting
> filesystems, stopping LVM (which tears down DM devices, etc).
> 
> So given that we don't have any userspace LVM2 support for dm-cache yet
> I'm not surprised by this.  In fact it is expected.
>  
> > Second, cache_status calls dm_cache_commit, which writes out a superblock to
> > the metadata device.  However, there's no call to save_stats to copy the
> > current values of the counters out to the disk's copy prior to calling
> > dm_cache_commit.  Therefore, we seem to be writing out stale copies of
> > superblock fields.
> > 
> > The second one seems fixable with the attached patch
> 
> I'll defer to Joe on this but I think sync_metadata() is pretty heavy to
> be doing every 'dmsetup info'.  BTW, with just dm_cache_commit() the
> superblock fields aren't stale; only the on-disk hints are.

Hrmm, how about this: dmsetup info will call save_stats so that the superblock
gets written with the freshest hit/miss counts, and I'll create a new dmsetup
message command that actually flushes everything out?

--D

[PATCH 1/2] dmcache: flush superblock stats when retrieving status info

[Darrick J. Wong]

When userspace queries dmcache for stats info, we should ensure that all the
metadata gets flushed out of memory to disk.  The current code neglects to
update the disk copy of the hit/miss counters.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 drivers/md/dm-cache-target.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 1074409..4fb7b4c 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2451,6 +2451,7 @@ static void cache_status(struct dm_target *ti, status_type_t type,
 	case STATUSTYPE_INFO:
 		/* Commit to ensure statistics aren't out-of-date */
 		if (!(status_flags & DM_STATUS_NOFLUSH_FLAG) && !dm_suspended(ti)) {
+			save_stats(cache);
 			r = dm_cache_commit(cache->cmd, false);
 			if (r)
 				DMERR("could not commit metadata for accurate status");

Re: dm-cache not writing out cache metadata at reboot?

[Mike Snitzer]

On Thu, May 09 2013 at  4:36pm -0400,
Darrick J. Wong <darrick.wong@oracle.com> wrote:

> On Wed, May 08, 2013 at 06:05:26PM -0400, Mike Snitzer wrote:
> > On Wed, May 08 2013 at  5:48pm -0400,
> > Darrick J. Wong <darrick.wong@oracle.com> wrote:
> > 
> > > Hi,
> > > 
> > > So I've been watching the hit/miss counters in dmcache and I've noticed a
> > > couple of things that look like errors to me:
> > > 
> > > First, I noticed that if I reboot the system, neither cache_postsuspend nor
> > > cache_dtr get called.  This might simply be expected behavior, but it means
> > > that the in-memory superblock structure doesn't get written out to disk upon
> > > reboot.  Just to be sure, I put a printk into __commit_transaction.  It prints
> > > out for 'dmsetup info' and 'dmsetup remove' but nothing at reboot.
> > 
> > We don't have reboot notifiers that auto-magically tear down an
> > artbitrary DM stack.  Typically the device shutdown includes unmounting
> > filesystems, stopping LVM (which tears down DM devices, etc).
> > 
> > So given that we don't have any userspace LVM2 support for dm-cache yet
> > I'm not surprised by this.  In fact it is expected.
> >  
> > > Second, cache_status calls dm_cache_commit, which writes out a superblock to
> > > the metadata device.  However, there's no call to save_stats to copy the
> > > current values of the counters out to the disk's copy prior to calling
> > > dm_cache_commit.  Therefore, we seem to be writing out stale copies of
> > > superblock fields.
> > > 
> > > The second one seems fixable with the attached patch
> > 
> > I'll defer to Joe on this but I think sync_metadata() is pretty heavy to
> > be doing every 'dmsetup info'.  BTW, with just dm_cache_commit() the
> > superblock fields aren't stale; only the on-disk hints are.
> 
> Hrmm, how about this: dmsetup info will call save_stats so that the superblock
> gets written with the freshest hit/miss counts, and I'll create a new dmsetup
> message command that actually flushes everything out?

That sounds better.

[PATCH 2/2] dmcache: Implement a flush message

[Darrick J. Wong]

Create a new 'flush' message that causes the dmcache to write all of its
metadata out to disk.  This enables us to ensure that the disk reflects
whatever's in memory without having to tear down the cache device.  This helps
me in the case where I have a cached ro fs that I can't umount and therefore
can't tear down the cache device, but want to save the cache metadata anyway.
The command syntax is as follows:

# dmsetup message mycache 0 flush now

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 drivers/md/dm-cache-target.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 4fb7b4c..e26e5d2 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -2522,6 +2522,7 @@ err:
 
 static int process_config_option(struct cache *cache, char **argv)
 {
+	bool res;
 	unsigned long tmp;
 
 	if (!strcasecmp(argv[0], "migration_threshold")) {
@@ -2530,6 +2531,9 @@ static int process_config_option(struct cache *cache, char **argv)
 
 		cache->migration_threshold = tmp;
 		return 0;
+	} else if (!strcasecmp(argv[0], "flush")) {
+		res = sync_metadata(cache);
+		return res ? 0 : -EIO;
 	}
 
 	return NOT_CORE_OPTION;

Re: [dm-devel] dm-cache not writing out cache metadata at reboot?

[Joe Thornber]

On Wed, May 08, 2013 at 06:05:26PM -0400, Mike Snitzer wrote:
> I'll defer to Joe on this but I think sync_metadata() is pretty heavy to
> be doing every 'dmsetup info'.  BTW, with just dm_cache_commit() the
> superblock fields aren't stale; only the on-disk hints are.

Agreed, the hints are v. expensive to write.

Re: [PATCH 1/2] dmcache: flush superblock stats when retrieving status info

[Joe Thornber]

On Thu, May 09, 2013 at 01:44:38PM -0700, Darrick J. Wong wrote:
> When userspace queries dmcache for stats info, we should ensure that all the
> metadata gets flushed out of memory to disk.  The current code neglects to
> update the disk copy of the hit/miss counters.

Nack, I'm afraid.  The commit is there purely to make sure the free
metadata blocks reported from the status are as high as possible.
[When a transaction is open we have to treat all blocks from the
previous transaction as allocated to allow rollback.]

Re: [dm-devel] [PATCH 2/2] dmcache: Implement a flush message

[Joe Thornber]

On Thu, May 09, 2013 at 01:47:51PM -0700, Darrick J. Wong wrote:
> Create a new 'flush' message that causes the dmcache to write all of its
> metadata out to disk.  This enables us to ensure that the disk reflects
> whatever's in memory without having to tear down the cache device.  This helps
> me in the case where I have a cached ro fs that I can't umount and therefore
> can't tear down the cache device, but want to save the cache metadata anyway.
> The command syntax is as follows:
> 
> # dmsetup message mycache 0 flush now

Nack.

[Ignoring the ugly 'now' parameter.]

I think you're in danger of hiding the real issue.  Which is if the
target's destructor and post suspend is not being called then, as far
as dm-cache is concerned this is a crash.  Any open transactions will
be lost as it automatically rolls back.

We need to understand more why this is happening.  It's actually
harmless atm for dm-cache, because we're forced to commit before using
a new migration.  But for dm-thin you can lose writes.  Why are you
never tearing down your dm devices?

- Joe

Re: [PATCH 1/2] dmcache: flush superblock stats when retrieving status info

[Mike Snitzer]

On Fri, May 10 2013 at  6:10am -0400,
Joe Thornber <thornber@redhat.com> wrote:

> On Thu, May 09, 2013 at 01:44:38PM -0700, Darrick J. Wong wrote:
> > When userspace queries dmcache for stats info, we should ensure that all the
> > metadata gets flushed out of memory to disk.  The current code neglects to
> > update the disk copy of the hit/miss counters.
> 
> Nack, I'm afraid.  The commit is there purely to make sure the free
> metadata blocks reported from the status are as high as possible.
> [When a transaction is open we have to treat all blocks from the
> previous transaction as allocated to allow rollback.]

Right the commit is about presenting a more accurate status.  Darrick's
patch doesn't change what is reported for hit/miss counts (they are
reported from in-core counters already, so accuracy isn't a concern).

So I agree with what you're saying.  Thanks Joe.

Re: [dm-devel] [PATCH 2/2] dmcache: Implement a flush message

[Darrick J. Wong]

On Fri, May 10, 2013 at 11:22:24AM +0100, Joe Thornber wrote:
> On Thu, May 09, 2013 at 01:47:51PM -0700, Darrick J. Wong wrote:
> > Create a new 'flush' message that causes the dmcache to write all of its
> > metadata out to disk.  This enables us to ensure that the disk reflects
> > whatever's in memory without having to tear down the cache device.  This helps
> > me in the case where I have a cached ro fs that I can't umount and therefore
> > can't tear down the cache device, but want to save the cache metadata anyway.
> > The command syntax is as follows:
> > 
> > # dmsetup message mycache 0 flush now
> 
> Nack.
> 
> [Ignoring the ugly 'now' parameter.]
> 
> I think you're in danger of hiding the real issue.  Which is if the
> target's destructor and post suspend is not being called then, as far
> as dm-cache is concerned this is a crash.  Any open transactions will
> be lost as it automatically rolls back.
> 
> We need to understand more why this is happening.  It's actually
> harmless atm for dm-cache, because we're forced to commit before using
> a new migration.  But for dm-thin you can lose writes.  Why are you
> never tearing down your dm devices?

afaict, there isn't anything in the initscripts that tears down dm devices
prior to invoking reboot(), and the kernel drivers don't have reboot notifiers
to flush things out either.  I've been told that lvm does this, but I don't see
anything in the Ubuntu or RHEL6 that would suggest a teardown script...

# dpkg -L lvm2 dmsetup libdevmapper1.02.1 libdevmapper-event1.02.1 | grep etc
/etc
/etc/lvm
/etc/lvm/lvm.conf
# grep -rn dmsetup /etc
/etc/lvm/lvm.conf:333:    # waiting for udev, run 'dmsetup udevcomplete_all' manually to wake them up.

# rpm -ql lvm2 lvm2-libs device-mapper device-mapper-event device-mapper-event-libs device-mapper-libs | grep /etc
/etc/lvm
/etc/lvm/archive
/etc/lvm/backup
/etc/lvm/cache
/etc/lvm/cache/.cache
/etc/lvm/lvm.conf
/etc/rc.d/init.d/lvm2-monitor
# grep -rn dmsetup /etc/rc* /etc/init*
/etc/rc0.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc0.d/S01halt:22:            if /sbin/dmsetup info "$dst" | grep -q '^Open count: *0$'; then
/etc/rc0.d/S01halt:120:	    && [ "$(dmsetup status "$dst" | cut -d ' ' -f 3)" = crypt ]; then
/etc/rc1.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc2.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc3.d/S25netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc4.d/S25netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc5.d/S25netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc6.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc6.d/S01reboot:22:            if /sbin/dmsetup info "$dst" | grep -q '^Open count: *0$'; then
/etc/rc6.d/S01reboot:120:	    && [ "$(dmsetup status "$dst" | cut -d ' ' -f 3)" = crypt ]; then
/etc/rc.d/rc6.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/rc6.d/S01reboot:22:            if /sbin/dmsetup info "$dst" | grep -q '^Open count: *0$'; then
/etc/rc.d/rc6.d/S01reboot:120:	    && [ "$(dmsetup status "$dst" | cut -d ' ' -f 3)" = crypt ]; then
/etc/rc.d/rc0.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/rc0.d/S01halt:22:            if /sbin/dmsetup info "$dst" | grep -q '^Open count: *0$'; then
/etc/rc.d/rc0.d/S01halt:120:	    && [ "$(dmsetup status "$dst" | cut -d ' ' -f 3)" = crypt ]; then
/etc/rc.d/rc.sysinit:191:		/sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p" >/dev/null
/etc/rc.d/rc5.d/S25netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/rc1.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/rc3.d/S25netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/init.d/netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/init.d/halt:22:            if /sbin/dmsetup info "$dst" | grep -q '^Open count: *0$'; then
/etc/rc.d/init.d/halt:120:	    && [ "$(dmsetup status "$dst" | cut -d ' ' -f 3)" = crypt ]; then
/etc/rc.d/rc4.d/S25netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.d/rc2.d/K75netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/rc.sysinit:191:		/sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p" >/dev/null
/etc/init.d/netfs:53:		       /sbin/dmsetup ls --target multipath --exec "/sbin/kpartx -a -p p"
/etc/init.d/halt:22:            if /sbin/dmsetup info "$dst" | grep -q '^Open count: *0$'; then
/etc/init.d/halt:120:	    && [ "$(dmsetup status "$dst" | cut -d ' ' -f 3)" = crypt ]; then

What am I missing?  My observation of Ubuntu is that at best it shuts down
services, umounts most of the filesystems, syncs, and reboots.  RHEL seems to
shut down multipath and dmcrypt, but that was all I found.  For /most/ users of
dm it seems like the system simply reboots, and nobody's the worse for the
wear.

In the meantime I've added a script to my dmcache test tools to tear things
down at the end, which works unless the umount fails. :/ I guess I could simply
suspend the devices, but the postsuspend flush only seems to get called if I
actually redefine the device to some driver that isn't cache.

(I guess I could suspend the device and replace cache with zero... yuck.)

--D

Re: [PATCH 2/2] dmcache: Implement a flush message

[Mike Snitzer]

[in the future please refrain from posting to LKML for such a narrow
topic like dm-cache... not seeing the point in adding to the LKML noise
-- dm-devel should suffice]

On Fri, May 10 2013 at  1:51pm -0400,
Darrick J. Wong <darrick.wong@oracle.com> wrote:

> On Fri, May 10, 2013 at 11:22:24AM +0100, Joe Thornber wrote:
> > On Thu, May 09, 2013 at 01:47:51PM -0700, Darrick J. Wong wrote:
> > > Create a new 'flush' message that causes the dmcache to write all of its
> > > metadata out to disk.  This enables us to ensure that the disk reflects
> > > whatever's in memory without having to tear down the cache device.  This helps
> > > me in the case where I have a cached ro fs that I can't umount and therefore
> > > can't tear down the cache device, but want to save the cache metadata anyway.
> > > The command syntax is as follows:
> > > 
> > > # dmsetup message mycache 0 flush now
> > 
> > Nack.
> > 
> > [Ignoring the ugly 'now' parameter.]
> > 
> > I think you're in danger of hiding the real issue.  Which is if the
> > target's destructor and post suspend is not being called then, as far
> > as dm-cache is concerned this is a crash.  Any open transactions will
> > be lost as it automatically rolls back.
> > 
> > We need to understand more why this is happening.  It's actually
> > harmless atm for dm-cache, because we're forced to commit before using
> > a new migration.  But for dm-thin you can lose writes.  Why are you
> > never tearing down your dm devices?
> 
> afaict, there isn't anything in the initscripts that tears down dm devices
> prior to invoking reboot(), and the kernel drivers don't have reboot notifiers
> to flush things out either.  I've been told that lvm does this, but I don't see
> anything in the Ubuntu or RHEL6 that would suggest a teardown script...

See: https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=c698ee14bbb1310cf2383c8977d14a8e29139f8c

But I'm not sure which distros have hooked blkdeactivate in (cc'ing
prajnoha for his insight).

> What am I missing?  My observation of Ubuntu is that at best it shuts down
> services, umounts most of the filesystems, syncs, and reboots.  RHEL seems to
> shut down multipath and dmcrypt, but that was all I found.  For /most/ users of
> dm it seems like the system simply reboots, and nobody's the worse for the
> wear.

DM devices should be properly torn down; as Joe said this is
particularly important for dm-thinp (otherwise it looks like a crash and
the open transaction is rolled back).

> In the meantime I've added a script to my dmcache test tools to tear things
> down at the end, which works unless the umount fails. :/ 

You should switch to using blkdeactivate.

> I guess I could simply suspend the devices, but the postsuspend flush
> only seems to get called if I actually redefine the device to some
> driver that isn't cache.
> 
> (I guess I could suspend the device and replace cache with zero... yuck.)

You _really_ shouldn't need to play these games.

postsuspend will get called regardless of whether you're changing the
table in any way.

See: do_suspend -> dm_suspend -> dm_table_postsuspend_targets -> suspend_targets

(the only way I'm seeing that the postsuspend could not get called is if
the freeze_bdev/thaw_bdev were to fail, via {lock,unlock}_fs())

Re: [PATCH 2/2] dmcache: Implement a flush message

[Peter Rajnoha]

On 11.05.2013 17:25, Mike Snitzer wrote:> On Fri, May 10 2013 at  1:51pm
-0400,
> Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
...
>> afaict, there isn't anything in the initscripts that tears down dm
devices
>> prior to invoking reboot(), and the kernel drivers don't have reboot
notifiers
>> to flush things out either.  I've been told that lvm does this, but I
don't see
>> anything in the Ubuntu or RHEL6 that would suggest a teardown script...
>
> See:
https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=c698ee14bbb1310cf2383c8977d14a8e29139f8c
>
> But I'm not sure which distros have hooked blkdeactivate in (cc'ing
> prajnoha for his insight).
>

The blk-availability initscript/systemd unit that gets called at
shutdown/reboot and which in turn calls the blkdeactivate is already
used in RHEL 6.4 onwards and also in Fedora 18 onwards. However, for
Fedora, you need to enable the systemd unit explicitly at the moment
(systemctl enable blk-availability.service). To have it enabled by
default, the distro-wide default systemd configuration needs to be
edited which is controlled by systemd-preset file (I hope F19 is going
to have this enabled by default finally).

As for any other distros, it's up to the maintainers in that distro to
make use of the new script - I haven't looked if they started using it
or not. But upstream already provides it since lvm2 v2.02.98.

Peter

Re: [dm-devel] [PATCH 2/2] dmcache: Implement a flush message

[Darrick J. Wong]

On Mon, May 13, 2013 at 02:04:08PM +0200, Peter Rajnoha wrote:
> On 11.05.2013 17:25, Mike Snitzer wrote:> On Fri, May 10 2013 at  1:51pm
> -0400,
> > Darrick J. Wong <darrick.wong@oracle.com> wrote:
> >
> ...
> >> afaict, there isn't anything in the initscripts that tears down dm
> devices
> >> prior to invoking reboot(), and the kernel drivers don't have reboot
> notifiers
> >> to flush things out either.  I've been told that lvm does this, but I
> don't see
> >> anything in the Ubuntu or RHEL6 that would suggest a teardown script...
> >
> > See:
> https://git.fedorahosted.org/cgit/lvm2.git/commit/?id=c698ee14bbb1310cf2383c8977d14a8e29139f8c
> >
> > But I'm not sure which distros have hooked blkdeactivate in (cc'ing
> > prajnoha for his insight).
> >
> 
> The blk-availability initscript/systemd unit that gets called at
> shutdown/reboot and which in turn calls the blkdeactivate is already
> used in RHEL 6.4 onwards and also in Fedora 18 onwards. However, for
> Fedora, you need to enable the systemd unit explicitly at the moment
> (systemctl enable blk-availability.service). To have it enabled by
> default, the distro-wide default systemd configuration needs to be
> edited which is controlled by systemd-preset file (I hope F19 is going
> to have this enabled by default finally).
> 
> As for any other distros, it's up to the maintainers in that distro to
> make use of the new script - I haven't looked if they started using it
> or not. But upstream already provides it since lvm2 v2.02.98.

Aha!  Thank you for providing the missing link.  Now it all makes sense. :)

(fwiw Ubuntu's latest is 2.02.95.)

--D
> 
> Peter
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel