On Sat, Nov 10, 2018 at 12:15:49AM -0700, Allison Henderson wrote:
>
>
> On 11/9/18 5:45 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > When there are fewer than 1024 inodes in the filesystem, scrub reports
> > fractional inodes in its final report:
> >
> > 35.2MiB data used; 5.0 inodes used.
> > 34.2MiB data found; 5.0 inodes found.
> > 5.0 inodes counted; 5.0 inodes checked.
> >
> > Inodes are indivisible, so only report the fractional part when we have
> > a large enough number of inodes to perform a unit conversion:
> >
> > 35.2MiB data used; 5 inodes used.
> > 34.2MiB data found; 5 inodes found.
> > 5 inodes counted; 5 inodes checked.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > scrub/common.c | 5 ++++-
> > scrub/common.h | 2 +-
> > scrub/phase7.c | 34 ++++++++++++++++++----------------
> > 3 files changed, 23 insertions(+), 18 deletions(-)
> >
> >
> > diff --git a/scrub/common.c b/scrub/common.c
> > index 96b86a26..78afc4bf 100644
> > --- a/scrub/common.c
> > +++ b/scrub/common.c
> > @@ -200,10 +200,12 @@ auto_space_units(
> > double
> > auto_units(
> > unsigned long long number,
> > - char **units)
> > + char **units,
> > + int *precision)
> > {
> > if (debug > 1)
> > goto no_prefix;
> > + *precision = 1;
> > if (number > 1000000000000ULL) {
> > *units = "T";
> > return number / 1000000000000.0;
> > @@ -220,6 +222,7 @@ auto_units(
> > no_prefix:
> > *units = "";
> > + *precision = 0;
> > return number;
> > }
> > diff --git a/scrub/common.h b/scrub/common.h
> > index 5a43ac0c..e85a0333 100644
> > --- a/scrub/common.h
> > +++ b/scrub/common.h
> > @@ -62,7 +62,7 @@ debug_tweak_on(
> > double timeval_subtract(struct timeval *tv1, struct timeval *tv2);
> > double auto_space_units(unsigned long long kilobytes, char **units);
> > -double auto_units(unsigned long long number, char **units);
> > +double auto_units(unsigned long long number, char **units, int *precision);
> > unsigned int scrub_nproc(struct scrub_ctx *ctx);
> > unsigned int scrub_nproc_workqueue(struct scrub_ctx *ctx);
> > diff --git a/scrub/phase7.c b/scrub/phase7.c
> > index cac0c75a..504a6927 100644
> > --- a/scrub/phase7.c
> > +++ b/scrub/phase7.c
> > @@ -107,6 +107,7 @@ xfs_scan_summary(
> > unsigned long long f_free;
> > bool moveon;
> > bool complain;
> > + int ip;
> > int error;
> > /* Flush everything out to disk before we start counting. */
> > @@ -176,27 +177,27 @@ xfs_scan_summary(
> > if (used_rt || stat_rt) {
> > d = auto_space_units(used_data, &du);
> > r = auto_space_units(used_rt, &ru);
> > - i = auto_units(used_files, &iu);
> > + i = auto_units(used_files, &iu, &ip);
> > fprintf(stdout,
> > -_("%.1f%s data used; %.1f%s realtime data used; %.2f%s inodes used.\n"),
> > - d, du, r, ru, i, iu);
> > +_("%.1f%s data used; %.1f%s realtime data used; %.*f%s inodes used.\n"),
> > + d, du, r, ru, ip, i, iu);
> > d = auto_space_units(stat_data, &du);
> > r = auto_space_units(stat_rt, &ru);
> > - i = auto_units(counted_inodes, &iu);
> > + i = auto_units(counted_inodes, &iu, &ip);
> > fprintf(stdout,
> > -_("%.1f%s data found; %.1f%s realtime data found; %.2f%s inodes found.\n"),
> > - d, du, r, ru, i, iu);
> > +_("%.1f%s data found; %.1f%s realtime data found; %.*f%s inodes found.\n"),
> > + d, du, r, ru, ip, i, iu);
>
> Just a suggestion:
> Is plumbing in the extra precision parameter really preferable to just doing
> something like (i == (int)i ? 0 : 1) Or maybe a precision macro or
> something.
auto_units() is supposed to turn a big number into something more
human-friendly for display, so it's in charge of figuring out all the
formatting stuff (label, precision, unit converted number). Better to
keep all that in a single function than open-code it everywhere.
--D
>
> Otherwise looks fine.
>
> Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> > } else {
> > d = auto_space_units(used_data, &du);
> > - i = auto_units(used_files, &iu);
> > + i = auto_units(used_files, &iu, &ip);
> > fprintf(stdout,
> > -_("%.1f%s data used; %.1f%s inodes used.\n"),
> > - d, du, i, iu);
> > +_("%.1f%s data used; %.*f%s inodes used.\n"),
> > + d, du, ip, i, iu);
> > d = auto_space_units(stat_data, &du);
> > - i = auto_units(counted_inodes, &iu);
> > + i = auto_units(counted_inodes, &iu, &ip);
> > fprintf(stdout,
> > -_("%.1f%s data found; %.1f%s inodes found.\n"),
> > - d, du, i, iu);
> > +_("%.1f%s data found; %.*f%s inodes found.\n"),
> > + d, du, ip, i, iu);
> > }
> > fflush(stdout);
> > }
> > @@ -210,12 +211,13 @@ _("%.1f%s data found; %.1f%s inodes found.\n"),
> > _("checked inodes"))) {
> > double i1, i2;
> > char *i1u, *i2u;
> > + int i1p, i2p;
> > - i1 = auto_units(counted_inodes, &i1u);
> > - i2 = auto_units(ctx->inodes_checked, &i2u);
> > + i1 = auto_units(counted_inodes, &i1u, &i1p);
> > + i2 = auto_units(ctx->inodes_checked, &i2u, &i2p);
> > fprintf(stdout,
> > -_("%.1f%s inodes counted; %.1f%s inodes checked.\n"),
> > - i1, i1u, i2, i2u);
> > +_("%.*f%s inodes counted; %.*f%s inodes checked.\n"),
> > + i1p, i1, i1u, i2p, i2, i2u);
> > fflush(stdout);
> > }
> >
