[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[dm-devel] [PATCH for-dm-3.14-fixes 8/8] dm thin: allow metadata space larger than supported to go unused



It was always intended that a user could provide a thin metadata device
that is larger than the max supported by the on-disk format.  The extra
space would just go unused.

That would only work when extending the metadata device, not on initial
thin-pool creation.  If the user attempted to use a larger metadata
device on creation they would get an error like the following:

 device-mapper: space map common: space map too large
 device-mapper: transaction manager: couldn't create metadata space map
 device-mapper: thin metadata: tm_create_with_sm failed
 device-mapper: table: 252:17: thin-pool: Error creating metadata object
 device-mapper: ioctl: error adding target to table

Fix this by allowing the initial thin-pool creation to cap the size of
the metadata area to be smaller than the size of the metadata device.

Also, the calculation for THIN_METADATA_MAX_SECTORS didn't account for
the sizeof the disk_bitmap_header.  So the supported maximum metadata
size is a bit smaller (reduced from 33423360 to 33292800 sectors).

Lastly, remove the "excess space will not be used" warning message from
get_metadata_dev_size(); it resulted in printing the warning multiple
times.  Factor out warn_if_metadata_device_too_big(), call it from
pool_ctr() and maybe_resize_metadata_dev().

Signed-off-by: Mike Snitzer <snitzer redhat com>
---
 drivers/md/dm-cache-metadata.c                     |  2 +-
 drivers/md/dm-thin-metadata.c                      | 21 +++++++------
 drivers/md/dm-thin-metadata.h                      |  5 ++--
 drivers/md/dm-thin.c                               | 35 ++++++++++++++--------
 .../md/persistent-data/dm-transaction-manager.c    | 13 ++++----
 .../md/persistent-data/dm-transaction-manager.h    |  2 +-
 6 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index 9ef0752..1baf615 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -323,7 +323,7 @@ static int __format_metadata(struct dm_cache_metadata *cmd)
 {
 	int r;
 
-	r = dm_tm_create_with_sm(cmd->bm, CACHE_SUPERBLOCK_LOCATION,
+	r = dm_tm_create_with_sm(cmd->bm, CACHE_SUPERBLOCK_LOCATION, 0,
 				 &cmd->tm, &cmd->metadata_sm);
 	if (r < 0) {
 		DMERR("tm_create_with_sm failed");
diff --git a/drivers/md/dm-thin-metadata.c b/drivers/md/dm-thin-metadata.c
index 7c2bc26..3780650 100644
--- a/drivers/md/dm-thin-metadata.c
+++ b/drivers/md/dm-thin-metadata.c
@@ -503,11 +503,11 @@ bad_locked:
 	return r;
 }
 
-static int __format_metadata(struct dm_pool_metadata *pmd)
+static int __format_metadata(struct dm_pool_metadata *pmd, dm_block_t nr_blocks)
 {
 	int r;
 
-	r = dm_tm_create_with_sm(pmd->bm, THIN_SUPERBLOCK_LOCATION,
+	r = dm_tm_create_with_sm(pmd->bm, THIN_SUPERBLOCK_LOCATION, nr_blocks,
 				 &pmd->tm, &pmd->metadata_sm);
 	if (r < 0) {
 		DMERR("tm_create_with_sm failed");
@@ -642,7 +642,8 @@ bad_unlock_sblock:
 	return r;
 }
 
-static int __open_or_format_metadata(struct dm_pool_metadata *pmd, bool format_device)
+static int __open_or_format_metadata(struct dm_pool_metadata *pmd, bool format_device,
+				     dm_block_t nr_blocks)
 {
 	int r, unformatted;
 
@@ -651,12 +652,13 @@ static int __open_or_format_metadata(struct dm_pool_metadata *pmd, bool format_d
 		return r;
 
 	if (unformatted)
-		return format_device ? __format_metadata(pmd) : -EPERM;
+		return format_device ? __format_metadata(pmd, nr_blocks) : -EPERM;
 
 	return __open_metadata(pmd);
 }
 
-static int __create_persistent_data_objects(struct dm_pool_metadata *pmd, bool format_device)
+static int __create_persistent_data_objects(struct dm_pool_metadata *pmd, bool format_device,
+					    dm_block_t nr_blocks)
 {
 	int r;
 
@@ -668,7 +670,7 @@ static int __create_persistent_data_objects(struct dm_pool_metadata *pmd, bool f
 		return PTR_ERR(pmd->bm);
 	}
 
-	r = __open_or_format_metadata(pmd, format_device);
+	r = __open_or_format_metadata(pmd, format_device, nr_blocks);
 	if (r)
 		dm_block_manager_destroy(pmd->bm);
 
@@ -808,7 +810,8 @@ out_locked:
 
 struct dm_pool_metadata *dm_pool_metadata_open(struct block_device *bdev,
 					       sector_t data_block_size,
-					       bool format_device)
+					       bool format_device,
+					       dm_block_t nr_blocks)
 {
 	int r;
 	struct dm_pool_metadata *pmd;
@@ -828,7 +831,7 @@ struct dm_pool_metadata *dm_pool_metadata_open(struct block_device *bdev,
 	pmd->bdev = bdev;
 	pmd->data_block_size = data_block_size;
 
-	r = __create_persistent_data_objects(pmd, format_device);
+	r = __create_persistent_data_objects(pmd, format_device, nr_blocks);
 	if (r) {
 		kfree(pmd);
 		return ERR_PTR(r);
@@ -1578,7 +1581,7 @@ int dm_pool_abort_metadata(struct dm_pool_metadata *pmd)
 
 	__set_abort_with_changes_flags(pmd);
 	__destroy_persistent_data_objects(pmd);
-	r = __create_persistent_data_objects(pmd, false);
+	r = __create_persistent_data_objects(pmd, false, 0);
 	if (r)
 		pmd->fail_io = true;
 
diff --git a/drivers/md/dm-thin-metadata.h b/drivers/md/dm-thin-metadata.h
index 583ff5d..e24169c 100644
--- a/drivers/md/dm-thin-metadata.h
+++ b/drivers/md/dm-thin-metadata.h
@@ -18,7 +18,7 @@
  * We have one block of index, which can hold 255 index entries.  Each
  * index entry contains allocation info about 16k metadata blocks.
  */
-#define THIN_METADATA_MAX_SECTORS (255 * (1 << 14) * (THIN_METADATA_BLOCK_SIZE / (1 << SECTOR_SHIFT)))
+#define THIN_METADATA_MAX_SECTORS (255 * ((1 << 14) - 64) * (THIN_METADATA_BLOCK_SIZE / (1 << SECTOR_SHIFT)))
 
 /*
  * A metadata device larger than 16GB triggers a warning.
@@ -45,7 +45,8 @@ typedef uint64_t dm_thin_id;
  */
 struct dm_pool_metadata *dm_pool_metadata_open(struct block_device *bdev,
 					       sector_t data_block_size,
-					       bool format_device);
+					       bool format_device,
+					       dm_block_t nr_blocks);
 
 int dm_pool_metadata_close(struct dm_pool_metadata *pmd);
 
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index cd52cf2..c4cee29 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -1829,6 +1829,8 @@ static void __pool_destroy(struct pool *pool)
 
 static struct kmem_cache *_new_mapping_cache;
 
+static dm_block_t get_metadata_dev_size_in_blocks(struct block_device *);
+
 static struct pool *pool_create(struct mapped_device *pool_md,
 				struct block_device *metadata_dev,
 				unsigned long block_size,
@@ -1839,8 +1841,10 @@ static struct pool *pool_create(struct mapped_device *pool_md,
 	struct pool *pool;
 	struct dm_pool_metadata *pmd;
 	bool format_device = read_only ? false : true;
+	dm_block_t nr_blocks = get_metadata_dev_size_in_blocks(metadata_dev);
 
-	pmd = dm_pool_metadata_open(metadata_dev, block_size, format_device);
+	pmd = dm_pool_metadata_open(metadata_dev, block_size, format_device,
+				    nr_blocks);
 	if (IS_ERR(pmd)) {
 		*error = "Error creating metadata object";
 		return (struct pool *)pmd;
@@ -2078,16 +2082,27 @@ static void metadata_low_callback(void *context)
 	dm_table_event(pool->ti->table);
 }
 
-static sector_t get_metadata_dev_size(struct block_device *bdev)
+static sector_t get_dev_size(struct block_device *bdev)
 {
-	sector_t metadata_dev_size = i_size_read(bdev->bd_inode) >> SECTOR_SHIFT;
+	return i_size_read(bdev->bd_inode) >> SECTOR_SHIFT;
+}
+
+static void warn_if_metadata_device_too_big(struct block_device *bdev)
+{
+	sector_t metadata_dev_size = get_dev_size(bdev);
 	char buffer[BDEVNAME_SIZE];
 
-	if (metadata_dev_size > THIN_METADATA_MAX_SECTORS_WARNING) {
+	if (metadata_dev_size > THIN_METADATA_MAX_SECTORS_WARNING)
 		DMWARN("Metadata device %s is larger than %u sectors: excess space will not be used.",
 		       bdevname(bdev, buffer), THIN_METADATA_MAX_SECTORS);
-		metadata_dev_size = THIN_METADATA_MAX_SECTORS_WARNING;
-	}
+}
+
+static sector_t get_metadata_dev_size(struct block_device *bdev)
+{
+	sector_t metadata_dev_size = get_dev_size(bdev);
+
+	if (metadata_dev_size > THIN_METADATA_MAX_SECTORS)
+		metadata_dev_size = THIN_METADATA_MAX_SECTORS;
 
 	return metadata_dev_size;
 }
@@ -2174,12 +2189,7 @@ static int pool_ctr(struct dm_target *ti, unsigned argc, char **argv)
 		ti->error = "Error opening metadata block device";
 		goto out_unlock;
 	}
-
-	/*
-	 * Run for the side-effect of possibly issuing a warning if the
-	 * device is too big.
-	 */
-	(void) get_metadata_dev_size(metadata_dev->bdev);
+	warn_if_metadata_device_too_big(metadata_dev->bdev);
 
 	r = dm_get_device(ti, argv[1], FMODE_READ | FMODE_WRITE, &data_dev);
 	if (r) {
@@ -2378,6 +2388,7 @@ static int maybe_resize_metadata_dev(struct dm_target *ti, bool *need_commit)
 			return -EINVAL;;
 		}
 
+		warn_if_metadata_device_too_big(pool->md_dev);
 		DMINFO("%s: growing the metadata device from %llu to %llu blocks",
 		       dm_device_name(pool->pool_md),
 		       sb_metadata_dev_size, metadata_dev_size);
diff --git a/drivers/md/persistent-data/dm-transaction-manager.c b/drivers/md/persistent-data/dm-transaction-manager.c
index 81da1a2..495277e 100644
--- a/drivers/md/persistent-data/dm-transaction-manager.c
+++ b/drivers/md/persistent-data/dm-transaction-manager.c
@@ -322,7 +322,7 @@ static int dm_tm_create_internal(struct dm_block_manager *bm,
 				 dm_block_t sb_location,
 				 struct dm_transaction_manager **tm,
 				 struct dm_space_map **sm,
-				 int create,
+				 int create, dm_block_t nr_blocks,
 				 void *sm_root, size_t sm_len)
 {
 	int r;
@@ -338,8 +338,9 @@ static int dm_tm_create_internal(struct dm_block_manager *bm,
 	}
 
 	if (create) {
-		r = dm_sm_metadata_create(*sm, *tm, dm_bm_nr_blocks(bm),
-					  sb_location);
+		if (!nr_blocks)
+			nr_blocks = dm_bm_nr_blocks(bm);
+		r = dm_sm_metadata_create(*sm, *tm, nr_blocks, sb_location);
 		if (r) {
 			DMERR("couldn't create metadata space map");
 			goto bad;
@@ -362,10 +363,10 @@ bad:
 }
 
 int dm_tm_create_with_sm(struct dm_block_manager *bm, dm_block_t sb_location,
-			 struct dm_transaction_manager **tm,
+			 dm_block_t nr_blocks, struct dm_transaction_manager **tm,
 			 struct dm_space_map **sm)
 {
-	return dm_tm_create_internal(bm, sb_location, tm, sm, 1, NULL, 0);
+	return dm_tm_create_internal(bm, sb_location, tm, sm, 1, nr_blocks, NULL, 0);
 }
 EXPORT_SYMBOL_GPL(dm_tm_create_with_sm);
 
@@ -374,7 +375,7 @@ int dm_tm_open_with_sm(struct dm_block_manager *bm, dm_block_t sb_location,
 		       struct dm_transaction_manager **tm,
 		       struct dm_space_map **sm)
 {
-	return dm_tm_create_internal(bm, sb_location, tm, sm, 0, sm_root, root_len);
+	return dm_tm_create_internal(bm, sb_location, tm, sm, 0, 0, sm_root, root_len);
 }
 EXPORT_SYMBOL_GPL(dm_tm_open_with_sm);
 
diff --git a/drivers/md/persistent-data/dm-transaction-manager.h b/drivers/md/persistent-data/dm-transaction-manager.h
index b5b1390..a8403f0 100644
--- a/drivers/md/persistent-data/dm-transaction-manager.h
+++ b/drivers/md/persistent-data/dm-transaction-manager.h
@@ -120,7 +120,7 @@ struct dm_block_manager *dm_tm_get_bm(struct dm_transaction_manager *tm);
  * shouldn't be used.
  */
 int dm_tm_create_with_sm(struct dm_block_manager *bm, dm_block_t sb_location,
-			 struct dm_transaction_manager **tm,
+			 dm_block_t nr_blocks, struct dm_transaction_manager **tm,
 			 struct dm_space_map **sm);
 
 int dm_tm_open_with_sm(struct dm_block_manager *bm, dm_block_t sb_location,
-- 
1.8.3.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]