[PATCH v3 0/4] btrfs-progs: stat info for btrfs rescue chunk-recover

[PATCH v3 0/4] btrfs-progs: stat info for btrfs rescue chunk-recover

[Zhao Lei]

chunk-recover need to use many many time in scan_devices(),
and no output in screen:
 # btrfs rescue chunk-recover /dev/sda6
 (no output here, but need long time)

To notice user that "the command is not hang", this patch add
dynamic updated stat information in above period:
 # btrfs rescue chunk-recover /dev/sda6
 Scanning: DONE in dev[0], 19998441472 in dev[1], DONE in dev[2]
 (until)
 Scanning: DONE in dev0, DONE in dev1, DONE in dev2
 Check chunks successfully with no orphans
 Recover the chunk tree successfully.


Changelog v2->v3:
 Add [PATCH 2/4] to fix a potential invalid memory access of pthread.

Changelog v1->v2:
 [PATCH 3/3] in v1 lost a little cleanup, added in v2.

Zhao Lei (4):
  btrfs-progs: use for loop for scan_devices
  btrfs-progs: Use long type to get thread's return value
  btrfs-progs: stat info for btrfs rescue chunk-recover
  btrfs-progs: Fix some spelling typo in chunk-recover.c

 chunk-recover.c | 89 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 28 deletions(-)

-- 
1.8.5.1

[PATCH v3 1/4] btrfs-progs: use for loop for scan_devices

[Zhao Lei]

for() is more suitable than while() in this code block.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 chunk-recover.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 832b3b1..66a4ce6 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -871,8 +871,7 @@ static int scan_devices(struct recover_control *rc)
 		devidx++;
 	}
 
-	i = 0;
-	while (i < devidx) {
+	for (i = 0; i < devidx; i++) {
 		ret = pthread_join(t_scans[i], (void **)&t_rets[i]);
 		if (ret || t_rets[i]) {
 			ret = 1;
@@ -880,7 +879,6 @@ static int scan_devices(struct recover_control *rc)
 			cancel_to = devnr - 1;
 			goto out1;
 		}
-		i++;
 	}
 out1:
 	while (ret && (cancel_from <= cancel_to)) {
-- 
1.8.5.1

[PATCH v3 2/4] btrfs-progs: Use long type to get thread's return value

[Zhao Lei]

pthread use void * to save return status, we can use this pointer to
save our return value, but we need keep the same length.

This patch move to use long type variable to save return value
of our thread, to avoid potentia invalid memory access.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 chunk-recover.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 66a4ce6..3cb22ae 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -829,7 +829,7 @@ static int scan_devices(struct recover_control *rc)
 	struct btrfs_device *dev;
 	struct device_scan *dev_scans;
 	pthread_t *t_scans;
-	int *t_rets;
+	long *t_rets;
 	int devnr = 0;
 	int devidx = 0;
 	int cancel_from = 0;
@@ -845,7 +845,7 @@ static int scan_devices(struct recover_control *rc)
 	t_scans = (pthread_t *)malloc(sizeof(pthread_t) * devnr);
 	if (!t_scans)
 		return -ENOMEM;
-	t_rets = (int *)malloc(sizeof(int) * devnr);
+	t_rets = (long *)malloc(sizeof(long) * devnr);
 	if (!t_rets)
 		return -ENOMEM;
 
-- 
1.8.5.1

[PATCH v3 3/4] btrfs-progs: stat info for btrfs rescue chunk-recover

[Zhao Lei]

chunk-recover need to use many many time in scan_devices(),
and no output in screen:
 # btrfs rescue chunk-recover /dev/sda6
 (no output here, but need long time)

To notice user that "the command is not hang", this patch add
dynamic updated stat information in above period:
 # btrfs rescue chunk-recover /dev/sda6
 Scanning: DONE in dev[0], 19998441472 in dev[1], DONE in dev[2]
 (until)
 Scanning: DONE in dev0, DONE in dev1, DONE in dev2
 Check chunks successfully with no orphans
 Recover the chunk tree successfully.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 chunk-recover.c | 72 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 54 insertions(+), 18 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 3cb22ae..562817b 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -76,6 +76,7 @@ struct device_scan {
 	struct recover_control *rc;
 	struct btrfs_device *dev;
 	int fd;
+	u64 bytenr;
 };
 
 static struct extent_record *btrfs_new_extent_record(struct extent_buffer *eb)
@@ -766,6 +767,8 @@ static int scan_one_device(void *dev_scan_struct)
 
 	bytenr = 0;
 	while (1) {
+		dev_scan->bytenr = bytenr;
+
 		if (is_super_block_address(bytenr))
 			bytenr += rc->sectorsize;
 
@@ -832,9 +835,8 @@ static int scan_devices(struct recover_control *rc)
 	long *t_rets;
 	int devnr = 0;
 	int devidx = 0;
-	int cancel_from = 0;
-	int cancel_to = 0;
 	int i;
+	int all_done;
 
 	list_for_each_entry(dev, &rc->fs_devices->devices, dev_list)
 		devnr++;
@@ -860,30 +862,64 @@ static int scan_devices(struct recover_control *rc)
 		dev_scans[devidx].rc = rc;
 		dev_scans[devidx].dev = dev;
 		dev_scans[devidx].fd = fd;
-		ret = pthread_create(&t_scans[devidx], NULL,
-				     (void *)scan_one_device,
-				     (void *)&dev_scans[devidx]);
-		if (ret) {
-			cancel_from = 0;
-			cancel_to = devidx - 1;
-			goto out1;
-		}
+		dev_scans[devidx].bytenr = -1;
 		devidx++;
 	}
 
 	for (i = 0; i < devidx; i++) {
-		ret = pthread_join(t_scans[i], (void **)&t_rets[i]);
-		if (ret || t_rets[i]) {
-			ret = 1;
-			cancel_from = i + 1;
-			cancel_to = devnr - 1;
+		ret = pthread_create(&t_scans[i], NULL,
+				     (void *)scan_one_device,
+				     (void *)&dev_scans[i]);
+		if (ret)
 			goto out1;
+
+		dev_scans[i].bytenr = 0;
+	}
+
+	while (1) {
+		all_done = 1;
+		for (i = 0; i < devidx; i++) {
+			if (dev_scans[i].bytenr == -1)
+				continue;
+			ret = pthread_tryjoin_np(t_scans[i],
+						 (void **)&t_rets[i]);
+			if (ret == EBUSY) {
+				all_done = 0;
+				continue;
+			}
+			if (ret || t_rets[i]) {
+				ret = 1;
+				goto out1;
+			}
+			dev_scans[i].bytenr = -1;
+		}
+
+		printf("\rScanning: ");
+		for (i = 0; i < devidx; i++) {
+			if (dev_scans[i].bytenr == -1)
+				printf("%sDONE in dev%d",
+				       i ? ", " : "", i);
+			else
+				printf("%s%llu in dev%d",
+				       i ? ", " : "", dev_scans[i].bytenr, i);
 		}
+		/* clear chars if exist in tail */
+		printf("                ");
+		printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
+		fflush(stdout);
+
+		if (all_done) {
+			printf("\n");
+			break;
+		}
+
+		sleep(1);
 	}
 out1:
-	while (ret && (cancel_from <= cancel_to)) {
-		pthread_cancel(t_scans[cancel_from]);
-		cancel_from++;
+	for (i = 0; i < devidx; i++) {
+		if (dev_scans[i].bytenr == -1)
+			continue;
+		pthread_cancel(t_scans[i]);
 	}
 out2:
 	free(dev_scans);
-- 
1.8.5.1

[PATCH v3 4/4] btrfs-progs: Fix some spelling typo in chunk-recover.c

[Zhao Lei]

Only comment, not big issue.

And remove no-use aggument in block_group_free_all_extent().

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 chunk-recover.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/chunk-recover.c b/chunk-recover.c
index 562817b..962a72b 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -259,7 +259,7 @@ again:
 		list_del_init(&exist->list);
 		free(exist);
 		/*
-		 * We must do seach again to avoid the following cache.
+		 * We must do search again to avoid the following cache.
 		 * /--old bg 1--//--old bg 2--/
 		 *        /--new bg--/
 		 */
@@ -1092,8 +1092,7 @@ err:
 	return ret;
 }
 
-static int block_group_free_all_extent(struct btrfs_trans_handle *trans,
-				       struct btrfs_root *root,
+static int block_group_free_all_extent(struct btrfs_root *root,
 				       struct block_group_record *bg)
 {
 	struct btrfs_block_group_cache *cache;
@@ -1133,7 +1132,7 @@ static int remove_chunk_extent_item(struct btrfs_trans_handle *trans,
 		if (ret)
 			return ret;
 
-		ret = block_group_free_all_extent(trans, root, chunk->bg_rec);
+		ret = block_group_free_all_extent(root, chunk->bg_rec);
 		if (ret)
 			return ret;
 	}
@@ -2310,7 +2309,7 @@ static void validate_rebuild_chunks(struct recover_control *rc)
 }
 
 /*
- * Return 0 when succesful, < 0 on error and > 0 if aborted by user
+ * Return 0 when successful, < 0 on error and > 0 if aborted by user
  */
 int btrfs_recover_chunk_tree(char *path, int verbose, int yes)
 {
-- 
1.8.5.1

Re: [PATCH v3 0/4] btrfs-progs: stat info for btrfs rescue chunk-recover

[David Sterba]

On Wed, Sep 02, 2015 at 08:22:28PM +0800, Zhao Lei wrote:
> chunk-recover need to use many many time in scan_devices(),
> and no output in screen:
>  # btrfs rescue chunk-recover /dev/sda6
>  (no output here, but need long time)
> 
> To notice user that "the command is not hang", this patch add
> dynamic updated stat information in above period:
>  # btrfs rescue chunk-recover /dev/sda6
>  Scanning: DONE in dev[0], 19998441472 in dev[1], DONE in dev[2]
>  (until)
>  Scanning: DONE in dev0, DONE in dev1, DONE in dev2
>  Check chunks successfully with no orphans
>  Recover the chunk tree successfully.
> 
> 
> Changelog v2->v3:
>  Add [PATCH 2/4] to fix a potential invalid memory access of pthread.
> 
> Changelog v1->v2:
>  [PATCH 3/3] in v1 lost a little cleanup, added in v2.
> 
> Zhao Lei (4):
>   btrfs-progs: use for loop for scan_devices
>   btrfs-progs: Use long type to get thread's return value
>   btrfs-progs: stat info for btrfs rescue chunk-recover
>   btrfs-progs: Fix some spelling typo in chunk-recover.c

All applied, thanks. I've split patch 4 into two, typo fixes and unused
argument removal, please keep such changes separate.