diff mbox

[3.16.y-ckt,stable] Patch "dm cache: share cache-metadata object across inactive and active DM tables" has been added to staging queue

Message ID 1422877853-24757-1-git-send-email-luis.henriques@canonical.com
State New
Headers show

Commit Message

Luis Henriques Feb. 2, 2015, 11:50 a.m. UTC
This is a note to let you know that I have just added a patch titled

    dm cache: share cache-metadata object across inactive and active DM tables

to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree 
which can be found at:

 http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.16.y-queue

This patch is scheduled to be released in version 3.16.7-ckt6.

If you, or anyone else, feels it should not be added to this tree, please 
reply to this email.

For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

Thanks.
-Luis

------

From e9dcb844b1b16b05a1d55825a305faf5d08c170a Mon Sep 17 00:00:00 2001
From: Joe Thornber <ejt@redhat.com>
Date: Fri, 23 Jan 2015 10:00:07 +0000
Subject: dm cache: share cache-metadata object across inactive and active DM
 tables

commit 9b1cc9f251affdd27f29fe46d0989ba76c33faf6 upstream.

If a DM table is reloaded with an inactive table when the device is not
suspended (normal procedure for LVM2), then there will be two dm-bufio
objects that can diverge.  This can lead to a situation where the
inactive table uses bufio to read metadata at the same time the active
table writes metadata -- resulting in the inactive table having stale
metadata buffers once it is promoted to the active table slot.

Fix this by using reference counting and a global list of cache metadata
objects to ensure there is only one metadata object per metadata device.

Signed-off-by: Joe Thornber <ejt@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
 drivers/md/dm-cache-metadata.c | 101 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 95 insertions(+), 6 deletions(-)

--
2.1.4

Comments

Mike Snitzer Feb. 2, 2015, 2:52 p.m. UTC | #1
On Mon, Feb 02 2015 at  6:50am -0500,
Luis Henriques <luis.henriques@canonical.com> wrote:

> This is a note to let you know that I have just added a patch titled
> 
>     dm cache: share cache-metadata object across inactive and active DM tables
> 
> to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree 
> which can be found at:
> 
>  http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.16.y-queue
> 
> This patch is scheduled to be released in version 3.16.7-ckt6.
> 
> If you, or anyone else, feels it should not be added to this tree, please 
> reply to this email.

Please make sure you pick up the follow-on stable fix for this commit,
see: 766a78882ddf7 ("dm cache: fix missing ERR_PTR returns and handling")
Luis Henriques Feb. 2, 2015, 3:29 p.m. UTC | #2
On Mon, Feb 02, 2015 at 09:52:27AM -0500, Mike Snitzer wrote:
> On Mon, Feb 02 2015 at  6:50am -0500,
> Luis Henriques <luis.henriques@canonical.com> wrote:
> 
> > This is a note to let you know that I have just added a patch titled
> > 
> >     dm cache: share cache-metadata object across inactive and active DM tables
> > 
> > to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree 
> > which can be found at:
> > 
> >  http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.16.y-queue
> > 
> > This patch is scheduled to be released in version 3.16.7-ckt6.
> > 
> > If you, or anyone else, feels it should not be added to this tree, please 
> > reply to this email.
> 
> Please make sure you pick up the follow-on stable fix for this commit,
> see: 766a78882ddf7 ("dm cache: fix missing ERR_PTR returns and handling")

Thank you Mike, I'm queuing that commit as well.

Cheers,
--
Luís
diff mbox

Patch

diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index d2899e7eb3aa..bf854ae27240 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -94,6 +94,9 @@  struct cache_disk_superblock {
 } __packed;

 struct dm_cache_metadata {
+	atomic_t ref_count;
+	struct list_head list;
+
 	struct block_device *bdev;
 	struct dm_block_manager *bm;
 	struct dm_space_map *metadata_sm;
@@ -669,10 +672,10 @@  static void unpack_value(__le64 value_le, dm_oblock_t *block, unsigned *flags)

 /*----------------------------------------------------------------*/

-struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
-						 sector_t data_block_size,
-						 bool may_format_device,
-						 size_t policy_hint_size)
+static struct dm_cache_metadata *metadata_open(struct block_device *bdev,
+					       sector_t data_block_size,
+					       bool may_format_device,
+					       size_t policy_hint_size)
 {
 	int r;
 	struct dm_cache_metadata *cmd;
@@ -683,6 +686,7 @@  struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
 		return NULL;
 	}

+	atomic_set(&cmd->ref_count, 1);
 	init_rwsem(&cmd->root_lock);
 	cmd->bdev = bdev;
 	cmd->data_block_size = data_block_size;
@@ -705,10 +709,95 @@  struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
 	return cmd;
 }

+/*
+ * We keep a little list of ref counted metadata objects to prevent two
+ * different target instances creating separate bufio instances.  This is
+ * an issue if a table is reloaded before the suspend.
+ */
+static DEFINE_MUTEX(table_lock);
+static LIST_HEAD(table);
+
+static struct dm_cache_metadata *lookup(struct block_device *bdev)
+{
+	struct dm_cache_metadata *cmd;
+
+	list_for_each_entry(cmd, &table, list)
+		if (cmd->bdev == bdev) {
+			atomic_inc(&cmd->ref_count);
+			return cmd;
+		}
+
+	return NULL;
+}
+
+static struct dm_cache_metadata *lookup_or_open(struct block_device *bdev,
+						sector_t data_block_size,
+						bool may_format_device,
+						size_t policy_hint_size)
+{
+	struct dm_cache_metadata *cmd, *cmd2;
+
+	mutex_lock(&table_lock);
+	cmd = lookup(bdev);
+	mutex_unlock(&table_lock);
+
+	if (cmd)
+		return cmd;
+
+	cmd = metadata_open(bdev, data_block_size, may_format_device, policy_hint_size);
+	if (cmd) {
+		mutex_lock(&table_lock);
+		cmd2 = lookup(bdev);
+		if (cmd2) {
+			mutex_unlock(&table_lock);
+			__destroy_persistent_data_objects(cmd);
+			kfree(cmd);
+			return cmd2;
+		}
+		list_add(&cmd->list, &table);
+		mutex_unlock(&table_lock);
+	}
+
+	return cmd;
+}
+
+static bool same_params(struct dm_cache_metadata *cmd, sector_t data_block_size)
+{
+	if (cmd->data_block_size != data_block_size) {
+		DMERR("data_block_size (%llu) different from that in metadata (%llu)\n",
+		      (unsigned long long) data_block_size,
+		      (unsigned long long) cmd->data_block_size);
+		return false;
+	}
+
+	return true;
+}
+
+struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
+						 sector_t data_block_size,
+						 bool may_format_device,
+						 size_t policy_hint_size)
+{
+	struct dm_cache_metadata *cmd = lookup_or_open(bdev, data_block_size,
+						       may_format_device, policy_hint_size);
+	if (cmd && !same_params(cmd, data_block_size)) {
+		dm_cache_metadata_close(cmd);
+		return NULL;
+	}
+
+	return cmd;
+}
+
 void dm_cache_metadata_close(struct dm_cache_metadata *cmd)
 {
-	__destroy_persistent_data_objects(cmd);
-	kfree(cmd);
+	if (atomic_dec_and_test(&cmd->ref_count)) {
+		mutex_lock(&table_lock);
+		list_del(&cmd->list);
+		mutex_unlock(&table_lock);
+
+		__destroy_persistent_data_objects(cmd);
+		kfree(cmd);
+	}
 }

 /*