linux-kernel - dm-cache not writing out cache metadata at reboot?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130508214845.GA7729@blackbox.djwong.org>
Date:	Wed, 8 May 2013 14:48:45 -0700
From:	"Darrick J. Wong" <darrick.wong@...cle.com>
To:	Mike Snitzer <msnitzer@...hat.com>,
	Joe Thornber <thornber@...hat.com>
Cc:	device-mapper development <dm-devel@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: dm-cache not writing out cache metadata at reboot?

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@...cle.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");
 		}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/