On Wed, Jan 17, 2018 at 12:00:54PM +1100, Dave Chinner wrote:
> 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?
Yes, I think so.
> 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>
Hooray! Thanks for the review!
--D
>
> 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
--
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
