Re: [PATCH 2/3] xfs: introduce vectored scrub mode

Darrick J. Wong
2024-04-11T04:31:49Z

On Wed, Apr 10, 2024 at 08:38:38PM -0700, Christoph Hellwig wrote:
> On Wed, Apr 10, 2024 at 05:59:41PM -0700, Darrick J. Wong wrote:
> > I thought about designing this interface that way, where userspace
> > passes a pointer to an empty buffer, and the kernel formats that with
> > xfs_scrub_vecs that tell userspace what it scrubbed and what the outcome
> > was. I didn't like that, because now the kernel has to have a way to
> > communicate that the buffer needed to have been at least X size, even
> > though for our cases XFS_SCRUB_TYPE_NR + 2 would always be enough.
> >
> > Better, I thought, to let userspace figure out what it wants to run, and
> > tell that explicitly to the kernel, and then the kernel can just do
> > that. The downside is that now we need the barriers.
>
> And the downside is the userspace needs to known about all the passes
> and dependencies. Which I guess it does anyway due to the older
> scrub interface, but maybe that's worth documenting?

Yes, that's correct that userspace would have needed to know all that
anyway. I'll summarize this conversation in the commit message.

> >
> > > > + BUILD_BUG_ON(sizeof(struct xfs_scrub_vec_head) ==
> > > > + sizeof(struct xfs_scrub_metadata));
> > > > + BUILD_BUG_ON(XFS_IOC_SCRUB_METADATA == XFS_IOC_SCRUBV_METADATA);
> > >
> > > What is the point of these BUILD_BUG_ONs?
> >
> > Reusing the same ioctl number instead of burning another one. It's not
> > really necessary I suppose.
>
> I find reusing the numbers really confusings even if it does work due
> to the size encoding. If you're fine with getting rid of it I'm all
> for it.

Done.

--D