On Tue, Jan 16, 2018 at 03:26:28PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Cross reference the refcount data with the rmap data to check that the
> number of rmaps for a given block match the refcount of that block, and
> that CoW blocks (which are owned entirely by the refcountbt) are tracked
> as well.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> v3: fix indenting and standardize helper naming, refactor helpers per dave'sr
> suggestion and add clarifying comments
> v2: streamline scrubber arguments, remove stack allocated objects
> ---
> fs/xfs/scrub/refcount.c | 340 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 338 insertions(+), 2 deletions(-)
One minor thing:
> + while (!list_empty(&refchk->fragments)) {
> + /* Discard any fragments ending at rbno from the worklist. */
> + nr = 0;
> + next_rbno = NULLAGBLOCK;
> + list_for_each_entry_safe(frag, n, &worklist, list) {
> + bno = frag->rm.rm_startblock + frag->rm.rm_blockcount;
> + if (bno != rbno) {
> + if (bno < next_rbno)
> + next_rbno = bno;
> + continue;
> + }
> + list_del(&frag->list);
> + kmem_free(frag);
> + nr++;
> + }
> +
> + /* Empty list? We're done. */
> + if (list_empty(&refchk->fragments))
> + break;
We haven't modified the &refchk->fragments list since the check at
the start of the loop, so this seems redundant and could be removed?
Other than that, I found this much easier to understand the second
time through. A bit of time for it to sink in and a few small
changes to logic and comments has done wonders :)
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
