On Tue, Oct 03, 2017 at 01:42:49PM -0700, Darrick J. Wong wrote:
> +
> +/* Set us up to scrub a file's contents. */
> +int
> +xfs_scrub_setup_inode_contents(
> + struct xfs_scrub_context *sc,
> + struct xfs_inode *ip,
> + unsigned int resblks)
> +{
> + struct xfs_mount *mp = sc->mp;
> + int error;
> +
> + error = xfs_scrub_get_inode(sc, ip);
> + if (error)
> + return error;
> +
> + /* Got the inode, lock it and we're ready to go. */
> + sc->ilock_flags = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
> + xfs_ilock(sc->ip, sc->ilock_flags);
> + error = xfs_scrub_trans_alloc(sc->sm, mp, &sc->tp);
> + if (error)
> + goto out_unlock;
> + sc->ilock_flags |= XFS_ILOCK_EXCL;
> + xfs_ilock(sc->ip, XFS_ILOCK_EXCL);
> +
> + return 0;
> +out_unlock:
> + xfs_iunlock(sc->ip, sc->ilock_flags);
> + if (sc->ip != ip)
> + iput(VFS_I(sc->ip));
> + sc->ip = NULL;
> + return error;
> +}
I've seen this get/lock/alloc code many times now - seems like scope
for factoring?
> +/* Scrub a directory entry. */
> +
> +struct xfs_scrub_dir_ctx {
> + struct dir_context dc;
> + struct xfs_scrub_context *sc;
> +};
These two character variable names make the code really hard to
understand, especially when....
> +
> +/* Check that an inode's mode matches a given DT_ type. */
> +STATIC int
> +xfs_scrub_dir_check_ftype(
> + struct xfs_scrub_dir_ctx *sdc,
... we now have sc, dc, and sdc all intertwined. Especially as some
of these functions seem to invert the calling convention of the rest
of the scrub code in that the scrub context is the primary object
that is passed around and everything is attached to the sc....
Brain hurts trying to keep all this straight here...
/me keeps blundering around until it becomes apparent that
dir_context has nothing to do with scrub, but is a VFS filldir
callback structure triggered through readdir.
Darrick, can you add some comments explaining how the dirent
scrubbing works? It'd make it so much easier to understand if I
didn't have to reverse engineer the intent and design from the
patch. I'm going to skip this one for now...
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
