diff mbox series

btrfs-progs: scrub: unify the output numbers for "Total to scrub"

Message ID 2e1ee8fb0a05dbb2f6a4327d5b1383c3f7635dea.1685924954.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: scrub: unify the output numbers for "Total to scrub" | expand

Commit Message

Qu Wenruo June 5, 2023, 12:29 a.m. UTC
[BUG]
Command `btrfs scrub start -B` and `btrfs scrub status` are reporting
very different results for "Total to scrub":

  $ sudo btrfs scrub start -B /mnt/btrfs/
  scrub done for c107ef62-0a5d-4fd7-a119-b88f38b8e084
  Scrub started:    Mon Jun  5 07:54:07 2023
  Status:           finished
  Duration:         0:00:00
  Total to scrub:   1.52GiB
  Rate:             0.00B/s
  Error summary:    no errors found

  $ sudo btrfs scrub status /mnt/btrfs/
  UUID:             c107ef62-0a5d-4fd7-a119-b88f38b8e084
  Scrub started:    Mon Jun  5 07:54:07 2023
  Status:           finished
  Duration:         0:00:00
  Total to scrub:   12.00MiB
  Rate:             0.00B/s
  Error summary:    no errors found

This can be very confusing for end users.

[CAUSE]
It's the function print_fs_stat() handling the "Total to scrub" output.

For `btrfs scrub start` command, we use the used bytes (aka, the total
used dev exntents of a device) for output.

This is not really accurate, as the chunks may be mostly empty just like
the following:

  $ sudo btrfs fi df /mnt/btrfs/
  Data, single: total=1.01GiB, used=9.06MiB
  System, DUP: total=40.00MiB, used=64.00KiB
  Metadata, DUP: total=256.00MiB, used=1.38MiB
  GlobalReserve, single: total=22.00MiB, used=0.00B

Thus we're reporting 1.5GiB to scrub (1.01GiB + 40MiB * 2 + 256MiB * 2).
But in reality, we only scrubbed 12MiB
(9.06MiB + 64KiB * 2 + 1.38MiB * 2).

[FIX]
Instead of using the used dev-extent bytes of a device, go with proper
scrubbed bytes for each device.

This involves print_fs_stat() and print_scrub_dev() called inside
scrub_start().

Now the output should match each other.

Issue: #636
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/scrub.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

David Sterba June 14, 2023, 10:41 p.m. UTC | #1
On Mon, Jun 05, 2023 at 08:29:21AM +0800, Qu Wenruo wrote:
> [BUG]
> Command `btrfs scrub start -B` and `btrfs scrub status` are reporting
> very different results for "Total to scrub":
> 
>   $ sudo btrfs scrub start -B /mnt/btrfs/
>   scrub done for c107ef62-0a5d-4fd7-a119-b88f38b8e084
>   Scrub started:    Mon Jun  5 07:54:07 2023
>   Status:           finished
>   Duration:         0:00:00
>   Total to scrub:   1.52GiB
>   Rate:             0.00B/s
>   Error summary:    no errors found
> 
>   $ sudo btrfs scrub status /mnt/btrfs/
>   UUID:             c107ef62-0a5d-4fd7-a119-b88f38b8e084
>   Scrub started:    Mon Jun  5 07:54:07 2023
>   Status:           finished
>   Duration:         0:00:00
>   Total to scrub:   12.00MiB
>   Rate:             0.00B/s
>   Error summary:    no errors found
> 
> This can be very confusing for end users.
> 
> [CAUSE]
> It's the function print_fs_stat() handling the "Total to scrub" output.
> 
> For `btrfs scrub start` command, we use the used bytes (aka, the total
> used dev exntents of a device) for output.
> 
> This is not really accurate, as the chunks may be mostly empty just like
> the following:
> 
>   $ sudo btrfs fi df /mnt/btrfs/
>   Data, single: total=1.01GiB, used=9.06MiB
>   System, DUP: total=40.00MiB, used=64.00KiB
>   Metadata, DUP: total=256.00MiB, used=1.38MiB
>   GlobalReserve, single: total=22.00MiB, used=0.00B
> 
> Thus we're reporting 1.5GiB to scrub (1.01GiB + 40MiB * 2 + 256MiB * 2).
> But in reality, we only scrubbed 12MiB
> (9.06MiB + 64KiB * 2 + 1.38MiB * 2).
> 
> [FIX]
> Instead of using the used dev-extent bytes of a device, go with proper
> scrubbed bytes for each device.
> 
> This involves print_fs_stat() and print_scrub_dev() called inside
> scrub_start().
> 
> Now the output should match each other.
> 
> Issue: #636
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Added to devel, thanks.
diff mbox series

Patch

diff --git a/cmds/scrub.c b/cmds/scrub.c
index 96e3bb5cfd65..2af4e429d9b0 100644
--- a/cmds/scrub.c
+++ b/cmds/scrub.c
@@ -326,7 +326,8 @@  static void print_scrub_dev(struct btrfs_ioctl_dev_info_args *di,
 		if (raw)
 			print_scrub_full(p);
 		else
-			print_scrub_summary(p, ss, di->bytes_used);
+			print_scrub_summary(p, ss, p->data_bytes_scrubbed +
+						   p->tree_bytes_scrubbed);
 	}
 }
 
@@ -1540,28 +1541,32 @@  static int scrub_start(const struct cmd_struct *cmd, int argc, char **argv,
 
 	if (do_print) {
 		const char *append = "done";
-		u64 total_bytes_used = 0;
+		u64 total_bytes_scrubbed = 0;
 
 		if (!do_stats_per_dev)
 			init_fs_stat(&fs_stat);
 		for (i = 0; i < fi_args.num_devices; ++i) {
+			struct btrfs_scrub_progress *cur_progress =
+						&sp[i].scrub_args.progress;
+
 			if (do_stats_per_dev) {
 				print_scrub_dev(&di_args[i],
-						&sp[i].scrub_args.progress,
+						cur_progress,
 						print_raw,
 						sp[i].ret ? "canceled" : "done",
 						&sp[i].stats);
 			} else {
 				if (sp[i].ret)
 					append = "canceled";
-				add_to_fs_stat(&sp[i].scrub_args.progress,
-						&sp[i].stats, &fs_stat);
+				add_to_fs_stat(cur_progress, &sp[i].stats,
+						&fs_stat);
 			}
-			total_bytes_used += di_args[i].bytes_used;
+			total_bytes_scrubbed += cur_progress->data_bytes_scrubbed +
+						cur_progress->tree_bytes_scrubbed;
 		}
 		if (!do_stats_per_dev) {
 			pr_verbose(LOG_DEFAULT, "scrub %s for %s\n", append, fsid);
-			print_fs_stat(&fs_stat, print_raw, total_bytes_used);
+			print_fs_stat(&fs_stat, print_raw, total_bytes_scrubbed);
 		}
 	}