Re: [PATCH] xfs_repair: multithread phase 2

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

Looks good except for some trivial nitpicks below,


Reviewed-by: Christoph Hellwig <hch@xxxxxx>

Btw, your previous patch used just

"repair:" as the Subject prefix, while this one uses xfs_repair.  I
don't really care about, but we should standardize on one.  The more
recent usage seems to include the xfs_ prefix.

> +                                  scanfunc_bno : scanfunc_cnt, 0,
> +                                  (void *)agcnts);

no need for the void cast.

> +#define SCAN_THREADS 32

this is unused now.

> +     agcnts = malloc(mp->m_sb.sb_agcount * sizeof(*agcnts));
> +     if (!agcnts) {
> +             do_abort(_("no memory for ag header counts\n"));
> +             return;
> +     }
> +     memset(agcnts, 0, mp->m_sb.sb_agcount * sizeof(*agcnts));

this could use a calloc.

>                                       break;
> +                             case PHASE2_THREADS:
> +                                     phase2_threads = (int)strtol(val, NULL, 
> 0);
> +                                     break;

This option also needs to be documented in the man page.  Also shouldn't
we try to handle errors from strtol?  Also maybe strtoul would be a
better choice as we certainly don't want a negative number of threads.