linux-bcache.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] bcache-tools: make --discard a per device option
@ 2021-06-25  1:30 yanhuan916
  2021-06-25  1:30 ` [PATCH 2/2] bcache-tools: Correct super block version check codes yanhuan916
  2021-06-28 16:09 ` [PATCH 1/2] bcache-tools: make --discard a per device option Coly Li
  0 siblings, 2 replies; 4+ messages in thread
From: yanhuan916 @ 2021-06-25  1:30 UTC (permalink / raw)
  To: linux-bcache; +Cc: colyli, dahefanteng, Huan Yan

From: Huan Yan <yanhuan916@gmail.com>

This patch make --discard option more flexible, it can be indicated after each
backing or cache device.
---
 make.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/make.c b/make.c
index 34d8011..39b381a 100644
--- a/make.c
+++ b/make.c
@@ -402,6 +402,18 @@ static void write_sb(char *dev, struct sb_context *sbc, bool bdev, bool force)
 		       (unsigned int) sb.version,
 		       sb.block_size,
 		       data_offset);
+
+		/* Attempting to discard backing device
+		 */
+		if (discard) {
+			if (blkdiscard_all(dev, fd) < 0) {
+				fprintf(stderr,
+					"Failed to discard device %s, %s\n",
+					dev, strerror(errno));
+				exit(EXIT_FAILURE);
+			}
+		}
+
 		putchar('\n');
 	} else {
 		if (nvdimm_meta)
@@ -446,8 +458,15 @@ static void write_sb(char *dev, struct sb_context *sbc, bool bdev, bool force)
 
 		/* Attempting to discard cache device
 		 */
-		if (discard)
-			blkdiscard_all(dev, fd);
+		if (discard) {
+			if (blkdiscard_all(dev, fd) < 0) {
+				fprintf(stderr,
+					"Failed to discard device %s, %s\n",
+					dev, strerror(errno));
+				exit(EXIT_FAILURE);
+			}
+		}
+
 		putchar('\n');
 	}
 
@@ -660,21 +679,27 @@ static unsigned int get_blocksize(const char *path)
 int make_bcache(int argc, char **argv)
 {
 	int c;
-	unsigned int i;
+	unsigned int i, n;
 	int cdev = -1, bdev = -1, mdev = -1;
 	unsigned int ncache_devices = 0, ncache_nvm_devices = 0;
 	unsigned int nbacking_devices = 0;
 	char *cache_devices[argc];
 	char *cache_nvm_devices[argc];
 	char *backing_devices[argc];
+	bool cache_devices_discard[argc];
+	bool backing_devices_discard[argc];
+	bool *device_discard = NULL;
 	char label[SB_LABEL_SIZE] = { 0 };
 	unsigned int block_size = 0, bucket_size = 1024;
-	int writeback = 0, discard = 0, wipe_bcache = 0, force = 0;
+	int writeback = 0, wipe_bcache = 0, force = 0;
 	unsigned int cache_replacement_policy = 0;
 	uint64_t data_offset = BDEV_DATA_START_DEFAULT;
 	uuid_t set_uuid;
 	struct sb_context sbc;
 
+	memset(cache_devices_discard, 0, sizeof(cache_devices_discard));
+	memset(backing_devices_discard, 0, sizeof(backing_devices_discard));
+
 	uuid_generate(set_uuid);
 
 	struct option opts[] = {
@@ -685,10 +710,8 @@ int make_bcache(int argc, char **argv)
 		{ "block",		1, NULL,	'w' },
 		{ "writeback",		0, &writeback,	1 },
 		{ "wipe-bcache",	0, &wipe_bcache,	1 },
-		{ "discard",		0, &discard,	1 },
-		{ "cache_replacement_policy", 1, NULL, 'p' },
+		{ "discard",		0, NULL,	'd' },
 		{ "cache-replacement-policy", 1, NULL, 'p' },
-		{ "data_offset",	1, NULL,	'o' },
 		{ "data-offset",	1, NULL,	'o' },
 		{ "cset-uuid",		1, NULL,	'u' },
 		{ "help",		0, NULL,	'h' },
@@ -710,6 +733,10 @@ int make_bcache(int argc, char **argv)
 		case 'M':
 			mdev = 1;
 			break;
+		case 'd':
+			if (device_discard)
+				*device_discard = true;
+			break;
 		case 'b':
 			bucket_size =
 				hatoi_validate(optarg, "bucket size", UINT_MAX);
@@ -762,15 +789,20 @@ int make_bcache(int argc, char **argv)
 			}
 
 			if (bdev > 0) {
-				backing_devices[nbacking_devices++] = optarg;
-				printf("backing_devices[%d]: %s\n", nbacking_devices - 1, optarg);
+				n = nbacking_devices++;
+				backing_devices[n] = optarg;
+				printf("backing_devices[%d]: %s\n", n, optarg);
+				device_discard = &backing_devices_discard[n];
 				bdev = -1;
 			} else if (cdev > 0) {
-				cache_devices[ncache_devices++] = optarg;
-				printf("cache_devices[%d]: %s\n", ncache_devices - 1, optarg);
+				n = ncache_devices++;
+				cache_devices[n] = optarg;
+				printf("cache_devices[%d]: %s\n", n, optarg);
+				device_discard = &cache_devices_discard[n];
 				cdev = -1;
 			} else if (mdev > 0) {
-				cache_nvm_devices[ncache_nvm_devices++] = optarg;
+				n = ncache_nvm_devices++;
+				cache_nvm_devices[n] = optarg;
 				mdev = -1;
 			}
 			break;
@@ -806,7 +838,6 @@ int make_bcache(int argc, char **argv)
 	sbc.block_size = block_size;
 	sbc.bucket_size = bucket_size;
 	sbc.writeback = writeback;
-	sbc.discard = discard;
 	sbc.wipe_bcache = wipe_bcache;
 	sbc.cache_replacement_policy = cache_replacement_policy;
 	sbc.data_offset = data_offset;
@@ -814,13 +845,16 @@ int make_bcache(int argc, char **argv)
 	sbc.label = label;
 	sbc.nvdimm_meta = (ncache_nvm_devices > 0) ? true : false;
 
-	for (i = 0; i < ncache_devices; i++)
+	for (i = 0; i < ncache_devices; i++) {
+		sbc.discard = cache_devices_discard[i];
 		write_sb(cache_devices[i], &sbc, false, force);
+	}
 
 	for (i = 0; i < nbacking_devices; i++) {
 		check_data_offset_for_zoned_device(backing_devices[i],
 						   &sbc.data_offset);
 
+		sbc.discard = backing_devices_discard[i];
 		write_sb(backing_devices[i], &sbc, true, force);
 	}
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] bcache-tools: Correct super block version check codes
  2021-06-25  1:30 [PATCH 1/2] bcache-tools: make --discard a per device option yanhuan916
@ 2021-06-25  1:30 ` yanhuan916
  2021-06-28  8:15   ` Wu Bo
  2021-06-28 16:09 ` [PATCH 1/2] bcache-tools: make --discard a per device option Coly Li
  1 sibling, 1 reply; 4+ messages in thread
From: yanhuan916 @ 2021-06-25  1:30 UTC (permalink / raw)
  To: linux-bcache; +Cc: colyli, dahefanteng, Huan Yan

From: Huan Yan <yanhuan916@gmail.com>

This patch add missing super block version below:
BCACHE_SB_VERSION_CDEV_WITH_UUID
BCACHE_SB_VERSION_BDEV_WITH_OFFSET
BCACHE_SB_VERSION_CDEV_WITH_FEATURES
BCACHE_SB_VERSION_BDEV_WITH_FEATURES
---
 bcache.c | 22 +++++++++++++++-------
 bcache.h |  3 ++-
 lib.c    | 15 ++++++++++-----
 make.c   |  8 ++++++--
 show.c   |  6 ++++--
 5 files changed, 37 insertions(+), 17 deletions(-)

diff --git a/bcache.c b/bcache.c
index 1c4cef9..62ed08d 100644
--- a/bcache.c
+++ b/bcache.c
@@ -199,7 +199,8 @@ int tree(void)
 	sprintf(out, "%s", begin);
 	list_for_each_entry_safe(devs, n, &head, dev_list) {
 		if ((devs->version == BCACHE_SB_VERSION_CDEV
-		     || devs->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
+		     || devs->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
+		     || devs->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
 		    && strcmp(devs->state, BCACHE_BASIC_STATE_ACTIVE) == 0) {
 			sprintf(out + strlen(out), "%s\n", devs->name);
 			list_for_each_entry_safe(tmp, m, &head, dev_list) {
@@ -231,7 +232,8 @@ int attach_both(char *cdev, char *backdev)
 	if (ret != 0)
 		return ret;
 	if (type != BCACHE_SB_VERSION_BDEV
-	    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET) {
+	    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+	    && type != BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
 		fprintf(stderr, "%s is not an backend device\n", backdev);
 		return 1;
 	}
@@ -244,7 +246,8 @@ int attach_both(char *cdev, char *backdev)
 	if (strlen(cdev) != 36) {
 		ret = detail_dev(cdev, &bd, &cd, NULL, &type);
 		if (type != BCACHE_SB_VERSION_CDEV
-		    && type != BCACHE_SB_VERSION_CDEV_WITH_UUID) {
+		    && type != BCACHE_SB_VERSION_CDEV_WITH_UUID
+		    && type != BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
 			fprintf(stderr, "%s is not an cache device\n", cdev);
 			return 1;
 		}
@@ -359,10 +362,13 @@ int main(int argc, char **argv)
 		ret = detail_dev(devname, &bd, &cd, NULL, &type);
 		if (ret != 0)
 			return ret;
-		if (type == BCACHE_SB_VERSION_BDEV) {
+		if (type == BCACHE_SB_VERSION_BDEV
+		    || type == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+		    || type == BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
 			return stop_backdev(devname);
 		} else if (type == BCACHE_SB_VERSION_CDEV
-			   || type == BCACHE_SB_VERSION_CDEV_WITH_UUID) {
+			   || type == BCACHE_SB_VERSION_CDEV_WITH_UUID
+			   || type == BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
 			return unregister_cset(cd.base.cset);
 		}
 		return 1;
@@ -408,7 +414,8 @@ int main(int argc, char **argv)
 			return ret;
 		}
 		if (type != BCACHE_SB_VERSION_BDEV
-		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET) {
+		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+		    && type != BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
 			fprintf(stderr,
 				"Only backend device is suppported\n");
 			return 1;
@@ -434,7 +441,8 @@ int main(int argc, char **argv)
 			return ret;
 		}
 		if (type != BCACHE_SB_VERSION_BDEV
-		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET) {
+		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+		    && type != BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
 			fprintf(stderr,
 				"Only backend device is suppported\n");
 			return 1;
diff --git a/bcache.h b/bcache.h
index 2ae25ee..b10d4c0 100644
--- a/bcache.h
+++ b/bcache.h
@@ -164,7 +164,8 @@ struct cache_sb {
 static inline bool SB_IS_BDEV(const struct cache_sb *sb)
 {
 	return sb->version == BCACHE_SB_VERSION_BDEV
-		|| sb->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET;
+		|| sb->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+		|| sb->version == BCACHE_SB_VERSION_BDEV_WITH_FEATURES;
 }
 
 BITMASK(CACHE_SYNC,		struct cache_sb, flags, 0, 1);
diff --git a/lib.c b/lib.c
index 745dab6..ea1f18d 100644
--- a/lib.c
+++ b/lib.c
@@ -281,10 +281,12 @@ int get_dev_bname(char *devname, char *bname)
 int get_bname(struct dev *dev, char *bname)
 {
 	if (dev->version == BCACHE_SB_VERSION_CDEV
-	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
+	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
+	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
 		strcpy(bname, BCACHE_NO_SUPPORT);
 	else if (dev->version == BCACHE_SB_VERSION_BDEV
-		   || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET)
+		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_FEATURES)
 		return get_dev_bname(dev->name, bname);
 	return 0;
 }
@@ -317,10 +319,12 @@ int get_backdev_attachpoint(char *devname, char *point)
 int get_point(struct dev *dev, char *point)
 {
 	if (dev->version == BCACHE_SB_VERSION_CDEV
-	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
+	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
+	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
 		strcpy(point, BCACHE_NO_SUPPORT);
 	else if (dev->version == BCACHE_SB_VERSION_BDEV
-		   || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET)
+		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_FEATURES)
 		return get_backdev_attachpoint(dev->name, point);
 	return 0;
 }
@@ -331,7 +335,8 @@ int cset_to_devname(struct list_head *head, char *cset, char *devname)
 
 	list_for_each_entry(dev, head, dev_list) {
 		if ((dev->version == BCACHE_SB_VERSION_CDEV
-		     || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
+		     || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
+		     || dev->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
 		    && strcmp(dev->cset, cset) == 0)
 			strcpy(devname, dev->name);
 	}
diff --git a/make.c b/make.c
index 39b381a..d3b4baa 100644
--- a/make.c
+++ b/make.c
@@ -272,10 +272,14 @@ static void write_sb(char *dev, struct sb_context *sbc, bool bdev, bool force)
 			ret = detail_dev(dev, &bd, &cd, NULL, &type);
 			if (ret != 0)
 				exit(EXIT_FAILURE);
-			if (type == BCACHE_SB_VERSION_BDEV) {
+			if (type == BCACHE_SB_VERSION_BDEV
+			    || type == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
+			    || type == BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
 				ret = stop_backdev(dev);
 			} else if (type == BCACHE_SB_VERSION_CDEV
-				|| type == BCACHE_SB_VERSION_CDEV_WITH_UUID) {
+				   || type == BCACHE_SB_VERSION_CDEV_WITH_UUID
+				   || type ==
+				   BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
 				ret = unregister_cset(cd.base.cset);
 			} else {
 				fprintf(stderr,
diff --git a/show.c b/show.c
index 6175f3f..15cdb95 100644
--- a/show.c
+++ b/show.c
@@ -75,8 +75,9 @@ int show_bdevs_detail(void)
 		if (strlen(devs->attachuuid) == 36) {
 			cset_to_devname(&head, devs->cset, attachdev);
 		} else if (devs->version == BCACHE_SB_VERSION_CDEV
+			   || devs->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
 			   || devs->version ==
-			   BCACHE_SB_VERSION_CDEV_WITH_UUID) {
+			   BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
 			strcpy(attachdev, BCACHE_NO_SUPPORT);
 		} else {
 			strcpy(attachdev, BCACHE_ATTACH_ALONE);
@@ -135,8 +136,9 @@ int show_bdevs(void)
 		if (strlen(devs->attachuuid) == 36) {
 			cset_to_devname(&head, devs->cset, attachdev);
 		} else if (devs->version == BCACHE_SB_VERSION_CDEV
+			   || devs->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
 			   || devs->version ==
-			   BCACHE_SB_VERSION_CDEV_WITH_UUID) {
+			   BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
 			strcpy(attachdev, BCACHE_NO_SUPPORT);
 		} else {
 			strcpy(attachdev, BCACHE_ATTACH_ALONE);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] bcache-tools: Correct super block version check codes
  2021-06-25  1:30 ` [PATCH 2/2] bcache-tools: Correct super block version check codes yanhuan916
@ 2021-06-28  8:15   ` Wu Bo
  0 siblings, 0 replies; 4+ messages in thread
From: Wu Bo @ 2021-06-28  8:15 UTC (permalink / raw)
  To: yanhuan916, linux-bcache; +Cc: colyli, dahefanteng

On 2021/6/25 9:30, yanhuan916@gmail.com wrote:
> From: Huan Yan <yanhuan916@gmail.com>
> 
> This patch add missing super block version below:
> BCACHE_SB_VERSION_CDEV_WITH_UUID
> BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> BCACHE_SB_VERSION_CDEV_WITH_FEATURES
> BCACHE_SB_VERSION_BDEV_WITH_FEATURES
> ---
>   bcache.c | 22 +++++++++++++++-------
>   bcache.h |  3 ++-
>   lib.c    | 15 ++++++++++-----
>   make.c   |  8 ++++++--
>   show.c   |  6 ++++--
>   5 files changed, 37 insertions(+), 17 deletions(-)
> 
> diff --git a/bcache.c b/bcache.c
> index 1c4cef9..62ed08d 100644
> --- a/bcache.c
> +++ b/bcache.c
> @@ -199,7 +199,8 @@ int tree(void)
>   	sprintf(out, "%s", begin);
>   	list_for_each_entry_safe(devs, n, &head, dev_list) {
>   		if ((devs->version == BCACHE_SB_VERSION_CDEV
> -		     || devs->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
> +		     || devs->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
> +		     || devs->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
>   		    && strcmp(devs->state, BCACHE_BASIC_STATE_ACTIVE) == 0) {
>   			sprintf(out + strlen(out), "%s\n", devs->name);
>   			list_for_each_entry_safe(tmp, m, &head, dev_list) {
> @@ -231,7 +232,8 @@ int attach_both(char *cdev, char *backdev)
>   	if (ret != 0)
>   		return ret;
>   	if (type != BCACHE_SB_VERSION_BDEV
> -	    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET) {
> +	    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +	    && type != BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
>   		fprintf(stderr, "%s is not an backend device\n", backdev);
>   		return 1;
>   	}
> @@ -244,7 +246,8 @@ int attach_both(char *cdev, char *backdev)
>   	if (strlen(cdev) != 36) {
>   		ret = detail_dev(cdev, &bd, &cd, NULL, &type);
>   		if (type != BCACHE_SB_VERSION_CDEV
> -		    && type != BCACHE_SB_VERSION_CDEV_WITH_UUID) {
> +		    && type != BCACHE_SB_VERSION_CDEV_WITH_UUID
> +		    && type != BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
>   			fprintf(stderr, "%s is not an cache device\n", cdev);
>   			return 1;
>   		}
> @@ -359,10 +362,13 @@ int main(int argc, char **argv)
>   		ret = detail_dev(devname, &bd, &cd, NULL, &type);
>   		if (ret != 0)
>   			return ret;
> -		if (type == BCACHE_SB_VERSION_BDEV) {
> +		if (type == BCACHE_SB_VERSION_BDEV
> +		    || type == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +		    || type == BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
>   			return stop_backdev(devname);
>   		} else if (type == BCACHE_SB_VERSION_CDEV
> -			   || type == BCACHE_SB_VERSION_CDEV_WITH_UUID) {
> +			   || type == BCACHE_SB_VERSION_CDEV_WITH_UUID
> +			   || type == BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {
>   			return unregister_cset(cd.base.cset);
>   		}
>   		return 1;
> @@ -408,7 +414,8 @@ int main(int argc, char **argv)
>   			return ret;
>   		}
>   		if (type != BCACHE_SB_VERSION_BDEV
> -		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET) {
> +		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +		    && type != BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
>   			fprintf(stderr,
>   				"Only backend device is suppported\n");
>   			return 1;
> @@ -434,7 +441,8 @@ int main(int argc, char **argv)
>   			return ret;
>   		}
>   		if (type != BCACHE_SB_VERSION_BDEV
> -		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET) {
> +		    && type != BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +		    && type != BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
>   			fprintf(stderr,
>   				"Only backend device is suppported\n");
>   			return 1;
> diff --git a/bcache.h b/bcache.h
> index 2ae25ee..b10d4c0 100644
> --- a/bcache.h
> +++ b/bcache.h
> @@ -164,7 +164,8 @@ struct cache_sb {
>   static inline bool SB_IS_BDEV(const struct cache_sb *sb)
>   {
>   	return sb->version == BCACHE_SB_VERSION_BDEV
> -		|| sb->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET;
> +		|| sb->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +		|| sb->version == BCACHE_SB_VERSION_BDEV_WITH_FEATURES;
>   }
>   
>   BITMASK(CACHE_SYNC,		struct cache_sb, flags, 0, 1);
> diff --git a/lib.c b/lib.c
> index 745dab6..ea1f18d 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -281,10 +281,12 @@ int get_dev_bname(char *devname, char *bname)
>   int get_bname(struct dev *dev, char *bname)
>   {
>   	if (dev->version == BCACHE_SB_VERSION_CDEV
> -	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
> +	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
> +	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
>   		strcpy(bname, BCACHE_NO_SUPPORT);
>   	else if (dev->version == BCACHE_SB_VERSION_BDEV
> -		   || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET)
> +		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_FEATURES)
>   		return get_dev_bname(dev->name, bname);
>   	return 0;
>   }
> @@ -317,10 +319,12 @@ int get_backdev_attachpoint(char *devname, char *point)
>   int get_point(struct dev *dev, char *point)
>   {
>   	if (dev->version == BCACHE_SB_VERSION_CDEV
> -	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
> +	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
> +	    || dev->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
>   		strcpy(point, BCACHE_NO_SUPPORT);
>   	else if (dev->version == BCACHE_SB_VERSION_BDEV
> -		   || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET)
> +		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +		 || dev->version == BCACHE_SB_VERSION_BDEV_WITH_FEATURES)
>   		return get_backdev_attachpoint(dev->name, point);
>   	return 0;
>   }
> @@ -331,7 +335,8 @@ int cset_to_devname(struct list_head *head, char *cset, char *devname)
>   
>   	list_for_each_entry(dev, head, dev_list) {
>   		if ((dev->version == BCACHE_SB_VERSION_CDEV
> -		     || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID)
> +		     || dev->version == BCACHE_SB_VERSION_CDEV_WITH_UUID
> +		     || dev->version == BCACHE_SB_VERSION_CDEV_WITH_FEATURES)
>   		    && strcmp(dev->cset, cset) == 0)
>   			strcpy(devname, dev->name);
>   	}
> diff --git a/make.c b/make.c
> index 39b381a..d3b4baa 100644
> --- a/make.c
> +++ b/make.c
> @@ -272,10 +272,14 @@ static void write_sb(char *dev, struct sb_context *sbc, bool bdev, bool force)
>   			ret = detail_dev(dev, &bd, &cd, NULL, &type);
>   			if (ret != 0)
>   				exit(EXIT_FAILURE);
> -			if (type == BCACHE_SB_VERSION_BDEV) {
> +			if (type == BCACHE_SB_VERSION_BDEV
> +			    || type == BCACHE_SB_VERSION_BDEV_WITH_OFFSET
> +			    || type == BCACHE_SB_VERSION_BDEV_WITH_FEATURES) {
>   				ret = stop_backdev(dev);
>   			} else if (type == BCACHE_SB_VERSION_CDEV
> -				|| type == BCACHE_SB_VERSION_CDEV_WITH_UUID) {
> +				   || type == BCACHE_SB_VERSION_CDEV_WITH_UUID
> +				   || type ==
> +				   BCACHE_SB_VERSION_CDEV_WITH_FEATURES) {

Should be encapsulated into a function to make the code simpler and 
easier to maintain.


Thanks.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] bcache-tools: make --discard a per device option
  2021-06-25  1:30 [PATCH 1/2] bcache-tools: make --discard a per device option yanhuan916
  2021-06-25  1:30 ` [PATCH 2/2] bcache-tools: Correct super block version check codes yanhuan916
@ 2021-06-28 16:09 ` Coly Li
  1 sibling, 0 replies; 4+ messages in thread
From: Coly Li @ 2021-06-28 16:09 UTC (permalink / raw)
  To: yanhuan916; +Cc: dahefanteng, linux-bcache

On 6/25/21 9:30 AM, yanhuan916@gmail.com wrote:
> From: Huan Yan <yanhuan916@gmail.com>
>
> This patch make --discard option more flexible, it can be indicated after each
> backing or cache device.

NACK.

Reason 1,  Only cache device requires discard in bcache-tools. Backing
device will be handled by upper layer like mkfs.
Reason 2, the patch format does not follow current bcache-tools patch
format style.

Coly Li

> ---
>  make.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 48 insertions(+), 14 deletions(-)
>
> diff --git a/make.c b/make.c
> index 34d8011..39b381a 100644
> --- a/make.c
> +++ b/make.c
> @@ -402,6 +402,18 @@ static void write_sb(char *dev, struct sb_context *sbc, bool bdev, bool force)
>  		       (unsigned int) sb.version,
>  		       sb.block_size,
>  		       data_offset);
> +
> +		/* Attempting to discard backing device
> +		 */
> +		if (discard) {
> +			if (blkdiscard_all(dev, fd) < 0) {
> +				fprintf(stderr,
> +					"Failed to discard device %s, %s\n",
> +					dev, strerror(errno));
> +				exit(EXIT_FAILURE);
> +			}
> +		}
> +
>  		putchar('\n');
>  	} else {
>  		if (nvdimm_meta)
> @@ -446,8 +458,15 @@ static void write_sb(char *dev, struct sb_context *sbc, bool bdev, bool force)
>  
>  		/* Attempting to discard cache device
>  		 */
> -		if (discard)
> -			blkdiscard_all(dev, fd);
> +		if (discard) {
> +			if (blkdiscard_all(dev, fd) < 0) {
> +				fprintf(stderr,
> +					"Failed to discard device %s, %s\n",
> +					dev, strerror(errno));
> +				exit(EXIT_FAILURE);
> +			}
> +		}
> +
>  		putchar('\n');
>  	}
>  
> @@ -660,21 +679,27 @@ static unsigned int get_blocksize(const char *path)
>  int make_bcache(int argc, char **argv)
>  {
>  	int c;
> -	unsigned int i;
> +	unsigned int i, n;
>  	int cdev = -1, bdev = -1, mdev = -1;
>  	unsigned int ncache_devices = 0, ncache_nvm_devices = 0;
>  	unsigned int nbacking_devices = 0;
>  	char *cache_devices[argc];
>  	char *cache_nvm_devices[argc];
>  	char *backing_devices[argc];
> +	bool cache_devices_discard[argc];
> +	bool backing_devices_discard[argc];
> +	bool *device_discard = NULL;
>  	char label[SB_LABEL_SIZE] = { 0 };
>  	unsigned int block_size = 0, bucket_size = 1024;
> -	int writeback = 0, discard = 0, wipe_bcache = 0, force = 0;
> +	int writeback = 0, wipe_bcache = 0, force = 0;
>  	unsigned int cache_replacement_policy = 0;
>  	uint64_t data_offset = BDEV_DATA_START_DEFAULT;
>  	uuid_t set_uuid;
>  	struct sb_context sbc;
>  
> +	memset(cache_devices_discard, 0, sizeof(cache_devices_discard));
> +	memset(backing_devices_discard, 0, sizeof(backing_devices_discard));
> +
>  	uuid_generate(set_uuid);
>  
>  	struct option opts[] = {
> @@ -685,10 +710,8 @@ int make_bcache(int argc, char **argv)
>  		{ "block",		1, NULL,	'w' },
>  		{ "writeback",		0, &writeback,	1 },
>  		{ "wipe-bcache",	0, &wipe_bcache,	1 },
> -		{ "discard",		0, &discard,	1 },
> -		{ "cache_replacement_policy", 1, NULL, 'p' },
> +		{ "discard",		0, NULL,	'd' },
>  		{ "cache-replacement-policy", 1, NULL, 'p' },
> -		{ "data_offset",	1, NULL,	'o' },
>  		{ "data-offset",	1, NULL,	'o' },
>  		{ "cset-uuid",		1, NULL,	'u' },
>  		{ "help",		0, NULL,	'h' },
> @@ -710,6 +733,10 @@ int make_bcache(int argc, char **argv)
>  		case 'M':
>  			mdev = 1;
>  			break;
> +		case 'd':
> +			if (device_discard)
> +				*device_discard = true;
> +			break;
>  		case 'b':
>  			bucket_size =
>  				hatoi_validate(optarg, "bucket size", UINT_MAX);
> @@ -762,15 +789,20 @@ int make_bcache(int argc, char **argv)
>  			}
>  
>  			if (bdev > 0) {
> -				backing_devices[nbacking_devices++] = optarg;
> -				printf("backing_devices[%d]: %s\n", nbacking_devices - 1, optarg);
> +				n = nbacking_devices++;
> +				backing_devices[n] = optarg;
> +				printf("backing_devices[%d]: %s\n", n, optarg);
> +				device_discard = &backing_devices_discard[n];
>  				bdev = -1;
>  			} else if (cdev > 0) {
> -				cache_devices[ncache_devices++] = optarg;
> -				printf("cache_devices[%d]: %s\n", ncache_devices - 1, optarg);
> +				n = ncache_devices++;
> +				cache_devices[n] = optarg;
> +				printf("cache_devices[%d]: %s\n", n, optarg);
> +				device_discard = &cache_devices_discard[n];
>  				cdev = -1;
>  			} else if (mdev > 0) {
> -				cache_nvm_devices[ncache_nvm_devices++] = optarg;
> +				n = ncache_nvm_devices++;
> +				cache_nvm_devices[n] = optarg;
>  				mdev = -1;
>  			}
>  			break;
> @@ -806,7 +838,6 @@ int make_bcache(int argc, char **argv)
>  	sbc.block_size = block_size;
>  	sbc.bucket_size = bucket_size;
>  	sbc.writeback = writeback;
> -	sbc.discard = discard;
>  	sbc.wipe_bcache = wipe_bcache;
>  	sbc.cache_replacement_policy = cache_replacement_policy;
>  	sbc.data_offset = data_offset;
> @@ -814,13 +845,16 @@ int make_bcache(int argc, char **argv)
>  	sbc.label = label;
>  	sbc.nvdimm_meta = (ncache_nvm_devices > 0) ? true : false;
>  
> -	for (i = 0; i < ncache_devices; i++)
> +	for (i = 0; i < ncache_devices; i++) {
> +		sbc.discard = cache_devices_discard[i];
>  		write_sb(cache_devices[i], &sbc, false, force);
> +	}
>  
>  	for (i = 0; i < nbacking_devices; i++) {
>  		check_data_offset_for_zoned_device(backing_devices[i],
>  						   &sbc.data_offset);
>  
> +		sbc.discard = backing_devices_discard[i];
>  		write_sb(backing_devices[i], &sbc, true, force);
>  	}
>  


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-28 16:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25  1:30 [PATCH 1/2] bcache-tools: make --discard a per device option yanhuan916
2021-06-25  1:30 ` [PATCH 2/2] bcache-tools: Correct super block version check codes yanhuan916
2021-06-28  8:15   ` Wu Bo
2021-06-28 16:09 ` [PATCH 1/2] bcache-tools: make --discard a per device option Coly Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).