On Thu, Feb 15, 2018 at 08:22:28AM +1100, Dave Chinner wrote: > On Wed, Feb 14, 2018 at 09:02:54AM -0800, Darrick J. Wong wrote: > > On Wed, Feb 14, 2018 at 10:34:09AM -0600, Eric Sandeen wrote: > > > On 2/8/18 6:51 AM, Eryu Guan wrote: > > > > On Wed, Feb 07, 2018 at 01:19:31PM -0800, Darrick J. Wong wrote: > > > >> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > >> > > > >> xfs_check has been long obsolete, so stop running it automatically > > > >> after every test. Tests that explicitly want xfs_check can call it > > > >> via _scratch_xfs_check or _xfs_check; that part doesn't go away. > > > >> > > > >> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > > > > > I'd like to see an ACK on this from XFS community. > > > > > > So, I thought we still had a local implementation of xfs_check-via-xfs_db > > > in xfstests as an intentional validation against xfs_repair: > > > > > > +# xfs_check script is planned to be deprecated. But, we want to > > > +# be able to invoke "xfs_check" behavior in xfstests in order to > > > +# maintain the current verification levels. > > > +_xfs_check() > > > > > > IOWS, "xfs_check is obsolete" is true for users/admins, but I thought > > > we were keeping it alive as a developer tool for now, and that it had > > > a place in xfstests. No? > > > > > > (But if it /is/ removed, wouldn't you kill off the _xfs_check() function as > > > well, to truly nuke it from orbit?) > > > > > > Anyway, I don't think I understand the justification for this change; > > > it was explicitly kept alive in xfstests since commit > > > > > > 187bccd xfstests: Remove dependence of xfs_check script > > > > > > and "xfs_check is obsolete for admins" doesn't seem to be sufficient rationale. > > > > > > What do we gain or lose by removing this from _check_xfs_filesystem? > > > > xfs_check likes to consume memory, which means that it consistently runs > > out and segfaults when I run xfstests on a 1k block configuration. It's > > also slow if the fs is really fragmented (which some of the reflink > > tests set up), so I figured I'd kill both birds with one patch by > > removing auto-xfscheck. > > I don't see it run out of memory on my 1p, 1GB RAM, 1k block size > test VM, running on 10GB test, 20GB scratch devices.... Maybe you're not crazy like me, who doesn't allow memory overcommit on his test VMs. :P > > We /could/ leave it as a check against xfs_repair, but at this point we > > have three different fsck(ish) tools and maybe it's just time to get rid > > of check. Consider that blockget didn't even support v5 filesystems > > until October 2015[1], which was ~2 years after v5 support landed in > > repair -- that convinced me (at the time) that nobody really cared about > > xfs_check anymore. > > Actually, that was more a result of the developer who was doing all > the v5 work being made the as maintainer half way thru it's > experimental stage and that meant time to work on the non-critical > pieces of v5 support were pretty severely curtailed. i.e. it wasn't > because we never intended to support this, just resources and > priorities changed drastically around that time. Aha :) > Once we get scrub doing everything check does and have some > confidence that it's working correctly, then we can remove the db > based check code. Right now we still rely on it to cross check > repair and scrub is about to drive a bunch of new changes to th > erepair code to fix up stuff it doesn't detect. Once we've got to > the point that repair and scrub to the same level and have a bit > more confidence in the scrub code, then we can think about retiring > the db-based check code... TBH I've wondered off and on whether it would be worth it to duplicate the existing fuzz tests so that we could assess whether or not xfs_repair actually catches everything that xfs_check can... but first I want to get scrub upstreamed so that I can fix all the things that either of /those/ tools should catch but does not. I withdraw this patch for now. --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
