On Wed, Oct 04, 2017 at 12:56:06PM +1100, Dave Chinner wrote:
> On Tue, Oct 03, 2017 at 05:02:09PM -0700, Darrick J. Wong wrote:
> > On Wed, Oct 04, 2017 at 10:32:47AM +1100, Dave Chinner wrote:
> > > On Tue, Oct 03, 2017 at 01:41:08PM -0700, Darrick J. Wong wrote:
> > > > +/*
> > > > + * Scrub probe -- userspace uses this to probe if we're willing to
> > > > + * scrub or repair a given mountpoint.
> > > > + */
> > > > +int
> > > > +xfs_scrub_probe(
> > > > + struct xfs_scrub_context *sc)
> > > > +{
> > > > + if (sc->sm->sm_ino || sc->sm->sm_agno)
> > > > + return -EINVAL;
> > > > + if (sc->sm->sm_gen & ~XFS_SCRUB_FLAGS_OUT)
> > >
> > > sm_flags?
> >
> > sm_gen is correct because in xfs_scrub_probe we reflect back to
> > userspace any valid scrub outflags that were passed in via sm_gen.
> > Therefore we have to check sm_gen for unknown output flags so that we
> > can error out on any invalid inputs from userland.
>
> Hmmm - seeing sm_ino and sm_gen in the same structure immediately
> makes me think {inode number, generation} tuple for uniquely
> identifying inodes from userspace. What on earth would scrub output
> flags be doing in a generation number? Indeed, from the first patch:
For actual inode-related scrubbing, yes sm_ino/sm_gen represent a tuple
for uniquely identifying inodes.
> +struct xfs_scrub_metadata {
> + __u32 sm_type; /* What to check? */
> + __u32 sm_flags; /* flags; see below. */
> + __u64 sm_ino; /* inode number. */
> + __u32 sm_gen; /* inode generation. */
> + __u32 sm_agno; /* ag number. */
> + __u64 sm_reserved[5]; /* pad to 64 bytes */
> ....
>
> It is an inode generation number?
>
> So I'm somewhat confused by flags appearing in the generation
> number...
>
> > xfs_scrub_metadata() unconditionally clears all the out flags from
> > sm_flags on its way in, prior to any of the scrub handlers being called.
> >
> > (It's possible I'm misunderstanding your question.)
>
> What has tripped me up is that the probe function sm_gen abuse isn't
> documented in this patch, so I had no idea why oflags were in
> sm_gen. Maybe you explained it previously, but I don't remember
> that detail and there's nothing in the patch to remind me....
Yeah... we /are/ abusing the sm_gen field for the dorky probe function.
xfs_io doesn't touch it at all and xfs_scrub doesn't do much with it.
Easier just to drop it.
--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
