On Wed, Feb 10, 2016 at 11:07:32AM -0500, Theodore Ts'o wrote: > On Wed, Feb 10, 2016 at 05:07:16PM +1100, Dave Chinner wrote: > > > # Remounting with nodiratime option > > > -_scratch_unmount > > > -_scratch_mount "-o nodiratime" > > > +_scratch_remount "-o nodiratime" > > > > This makes me go "no, that can't work, nodiratime is not an option > > that we allow on remount." > > Hmm, yes, we're not consistent here. xfs doesn't allow nodiratime on > remounts. ext4 allows nodiratime on remount, but not diratime, so > there's no way to clear nodiratime once its set. tmpfs allows > diratime and nodiratime on remount. > > I wonder if it's worth it make things more consistent across the > various file systems. What do you think? Different problem, care factor approaching zero right now. :/ Talk to Eric - he's futzing with this stuff on XFS right now. > > So, at minimum, the name of the helper needs to get changed so that > > it doesn't imply that a "-o remount" with new options is being > > done... > > Hmm, how about having two helper functions: _scratch_remount that > doesn't take any arguments, and a _scratch_change_mount_opts which > does? No, it's not really the options that are the problem here. The problem is -o remount vs unmount/mount and what the test is actually expecting. I'd say "_scratch_remount" should do "-o remount" unconditionally (least surprise) and the current _scratch_remount should be changed to something like _scratch_cycle_mount(). That way both can take options, but it's clear they do different things. tmpfs can simply implement them the same way. > > > -_umount_mount > > > +_scratch_mkfs >/dev/null 2>&1 || _fail "mkfs failed" > > > +_scratch_mount > /dev/null 2>&1 || _fail "mount failed" > > > > Please don't add _fail to mkfs/mount like this, especially where the > > test doesn't already have them. > > Could you say more about why? We do have tests that do use _fail if > the mkfs or mount fails. Should we be changing them to remove it? Because, in general, scratch_mkfs should not ever fail, and nor should a mounting the scratch device. If they do fail, something has already gone wrong... > But if we do that, and the mount fails for some reason, then won't > things stagger on and perhaps make life harder to debug. Yes, but we don't care that they make lots of noise or that they stagger on, because that staggering on can exercise failure and other unexpected paths that wouldn't otherwise get exercised. I've lost count of the number of times that I've had a stuck process prevent an unmount and a subsequent test has tripped over and uncovered a previously unknown bug because we allowed mkfs and mount to fail.... As for debugging - you always have to go back to the first failure you see in xfstests and work from there, so the subsequent noise of other tests failing can simply be ignored. Yes, it can make it a little harder to identify the first failure, but it only takes a few seconds looking at log files to identify that these functions have failed... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
