All of lore.kernel.org
 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; 5+ 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 related	[flat|nested] 5+ 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
  2022-05-13 15:05   ` Coly Li
  2021-06-28 16:09 ` [PATCH 1/2] bcache-tools: make --discard a per device option Coly Li
  1 sibling, 2 replies; 5+ 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 related	[flat|nested] 5+ 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
  2022-05-13 15:05   ` Coly Li
  1 sibling, 0 replies; 5+ 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] 5+ 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; 5+ 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] 5+ 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
@ 2022-05-13 15:05   ` Coly Li
  1 sibling, 0 replies; 5+ messages in thread
From: Coly Li @ 2022-05-13 15:05 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 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


Applied and complemented commit log. Thanks.


Coly Li


> ---
>   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);



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

end of thread, other threads:[~2022-05-13 15:05 UTC | newest]

Thread overview: 5+ 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
2022-05-13 15:05   ` Coly Li
2021-06-28 16:09 ` [PATCH 1/2] bcache-tools: make --discard a per device option Coly Li

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.