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
