Page MenuHomeFreeBSD

Use humanize_number to format available and bad space sizes.
ClosedPublic

Authored by delphij on Jan 6 2020, 9:18 AM.

Details

Summary

Use humanize_number to format available and bad space sizes.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

delphij created this revision.Jan 6 2020, 9:18 AM
mckusick requested changes to this revision.Jan 6 2020, 10:23 PM

The current output is number of free blocks and number of bad blocks. You are changing it to number of free bytes and number of bad bytes.
Since fsck for all other filesystems reports blocks this is a non-standard semantic change, so I recommend that you restore the / 1024 to output block counts.
If you think that it is useful to have byte counts then should make it clear you are reporting bytes and not blocks (e.g., %sB free bytes).

This revision now requires changes to proceed.Jan 6 2020, 10:23 PM
delphij planned changes to this revision.Jan 7 2020, 4:49 AM

The current output is number of free blocks and number of bad blocks.

Well, no, the current output contains both free space in KB (the output does not have a unit there) and the number of free blocks (clusters). The old output would be something like:

** Phase 1 - Read FAT and checking connectivity
** Phase 2 - Checking Directories
** Phase 3 - Checking for Lost Files
331 files, 4182272 free (130696 clusters), 64 bad (2 clusters)

And after the change the output would become:

** Phase 1 - Read FAT and checking connectivity
** Phase 2 - Checking Directories
** Phase 3 - Checking for Lost Files
331 files, 4.0 GiB free (130696 clusters), 64 KiB bad (2 clusters)

You are changing it to number of free bytes and number of bad bytes.

Arguably, I think the old output was quite confusing due to the lack of unit in space. This is unlike fsck_ffs, where it's consistently using blocks instead:

** Phase 1 - Check Blocks and Sizes
** Phase 2 - Check Pathnames
** Phase 3 - Check Connectivity
** Phase 4 - Check Reference Counts
** Phase 5 - Check Cyl groups
2 files, 2 used, 507781 free (21 frags, 63470 blocks, 0.0% fragmentation)

Since fsck for all other filesystems reports blocks this is a non-standard semantic change, so I recommend that you restore the / 1024 to output block counts.
If you think that it is useful to have byte counts then should make it clear you are reporting bytes and not blocks (e.g., %sB free bytes).

It seems that this is something that needs additional discussion (I have another relatively more important fsck_msdosfs changeset that I'd like to get review first, will post it soon), so I'm marking this one as "planned changes" for now.

My confusion is what "clusters" represent. Apparently a cluster is a 32KB block. Based on your clarification, I now agree with your calculation and printing of free space, but suggest that you clarify that clusters are 32KB blocks (either by replacing the word "clusters" with "32KB blocks", or if clusters is meaningful to administrators change it to "32KB clusters").

My confusion is what "clusters" represent. Apparently a cluster is a 32KB block. Based on your clarification, I now agree with your calculation and printing of free space, but suggest that you clarify that clusters are 32KB blocks (either by replacing the word "clusters" with "32KB blocks", or if clusters is meaningful to administrators change it to "32KB clusters").

The term "cluster" is actually a concept specific to FAT (it's comparable to UFS blocks; unlike UFS there is no "fragments" in FAT and cluster is the minimal allocation unit). Like UFS, cluster size is variable (1, 2, 4, 8, 16, 32, 64, 128 sectors, but the specification only require cluster size of <= 32KB to work properly) and determined at the time of initialization of file system (newfs_msdosfs).

Although the size of cluster is variable, it's not clear to me if the size of cluster or block size is very useful for users for fsck's usage. For example, fsck_ffs will not give that information (and e2fsck doesn't do that either). The user can actually know the available space after mounting the file system, and maybe we should make fsck_msdosfs just behave like the others instead?

mckusick accepted this revision.Jan 10 2020, 12:37 AM

OK, looks good to go.

delphij updated this revision to Diff 66644.Jan 12 2020, 5:34 AM

Keep existing code for platforms where humanize_number(3) is not available.

This revision was not accepted when it landed; it landed in state Needs Review.Mon, Feb 10, 4:17 AM
This revision was automatically updated to reflect the committed changes.