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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Joe Thornber · Greg Kroah-Hartman
1 parent 7536e0dce3
Showing 1 changed file with 95 additions and 6 deletions Side-by-side Diff
drivers/md/dm-cache-metadata.c
@@ -94,6 +94,9 @@
 } __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 @@
  
 /*----------------------------------------------------------------*/
  
-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 @@
 		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 @@
 	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);
+	}
 }
  
 /*
...	...	@@ -94,6 +94,9 @@
94	94	} __packed;
95	95
96	96	struct dm_cache_metadata {
	97	+ atomic_t ref_count;
	98	+ struct list_head list;
	99	+
97	100	struct block_device *bdev;
98	101	struct dm_block_manager *bm;
99	102	struct dm_space_map *metadata_sm;
...	...	@@ -669,10 +672,10 @@
669	672
670	673	/----------------------------------------------------------------/
671	674
672		-struct dm_cache_metadata dm_cache_metadata_open(struct block_device bdev,
673		- sector_t data_block_size,
674		- bool may_format_device,
675		- size_t policy_hint_size)
	675	+static struct dm_cache_metadata metadata_open(struct block_device bdev,
	676	+ sector_t data_block_size,
	677	+ bool may_format_device,
	678	+ size_t policy_hint_size)
676	679	{
677	680	int r;
678	681	struct dm_cache_metadata *cmd;
...	...	@@ -683,6 +686,7 @@
683	686	return NULL;
684	687	}
685	688
	689	+ atomic_set(&cmd->ref_count, 1);
686	690	init_rwsem(&cmd->root_lock);
687	691	cmd->bdev = bdev;
688	692	cmd->data_block_size = data_block_size;
689	693
...	...	@@ -705,10 +709,95 @@
705	709	return cmd;
706	710	}
707	711
	712	+/*
	713	+ * We keep a little list of ref counted metadata objects to prevent two
	714	+ * different target instances creating separate bufio instances. This is
	715	+ * an issue if a table is reloaded before the suspend.
	716	+ */
	717	+static DEFINE_MUTEX(table_lock);
	718	+static LIST_HEAD(table);
	719	+
	720	+static struct dm_cache_metadata lookup(struct block_device bdev)
	721	+{
	722	+ struct dm_cache_metadata *cmd;
	723	+
	724	+ list_for_each_entry(cmd, &table, list)
	725	+ if (cmd->bdev == bdev) {
	726	+ atomic_inc(&cmd->ref_count);
	727	+ return cmd;
	728	+ }
	729	+
	730	+ return NULL;
	731	+}
	732	+
	733	+static struct dm_cache_metadata lookup_or_open(struct block_device bdev,
	734	+ sector_t data_block_size,
	735	+ bool may_format_device,
	736	+ size_t policy_hint_size)
	737	+{
	738	+ struct dm_cache_metadata cmd, cmd2;
	739	+
	740	+ mutex_lock(&table_lock);
	741	+ cmd = lookup(bdev);
	742	+ mutex_unlock(&table_lock);
	743	+
	744	+ if (cmd)
	745	+ return cmd;
	746	+
	747	+ cmd = metadata_open(bdev, data_block_size, may_format_device, policy_hint_size);
	748	+ if (cmd) {
	749	+ mutex_lock(&table_lock);
	750	+ cmd2 = lookup(bdev);
	751	+ if (cmd2) {
	752	+ mutex_unlock(&table_lock);
	753	+ __destroy_persistent_data_objects(cmd);
	754	+ kfree(cmd);
	755	+ return cmd2;
	756	+ }
	757	+ list_add(&cmd->list, &table);
	758	+ mutex_unlock(&table_lock);
	759	+ }
	760	+
	761	+ return cmd;
	762	+}
	763	+
	764	+static bool same_params(struct dm_cache_metadata *cmd, sector_t data_block_size)
	765	+{
	766	+ if (cmd->data_block_size != data_block_size) {
	767	+ DMERR("data_block_size (%llu) different from that in metadata (%llu)\n",
	768	+ (unsigned long long) data_block_size,
	769	+ (unsigned long long) cmd->data_block_size);
	770	+ return false;
	771	+ }
	772	+
	773	+ return true;
	774	+}
	775	+
	776	+struct dm_cache_metadata dm_cache_metadata_open(struct block_device bdev,
	777	+ sector_t data_block_size,
	778	+ bool may_format_device,
	779	+ size_t policy_hint_size)
	780	+{
	781	+ struct dm_cache_metadata *cmd = lookup_or_open(bdev, data_block_size,
	782	+ may_format_device, policy_hint_size);
	783	+ if (cmd && !same_params(cmd, data_block_size)) {
	784	+ dm_cache_metadata_close(cmd);
	785	+ return NULL;
	786	+ }
	787	+
	788	+ return cmd;
	789	+}
	790	+
708	791	void dm_cache_metadata_close(struct dm_cache_metadata *cmd)
709	792	{
710		- __destroy_persistent_data_objects(cmd);
711		- kfree(cmd);
	793	+ if (atomic_dec_and_test(&cmd->ref_count)) {
	794	+ mutex_lock(&table_lock);
	795	+ list_del(&cmd->list);
	796	+ mutex_unlock(&table_lock);
	797	+
	798	+ __destroy_persistent_data_objects(cmd);
	799	+ kfree(cmd);
	800	+ }
712	801	}
713	802
714	803	/*