On Tue, Jan 16, 2018 at 09:09:14AM -0500, Brian Foster wrote: > On Tue, Jan 16, 2018 at 07:50:23PM +1100, Dave Chinner wrote: > > On Tue, Jan 16, 2018 at 12:02:54PM +0800, Eryu Guan wrote: > > > On Tue, Jan 16, 2018 at 08:03:52AM +1100, Dave Chinner wrote: > > > > On Mon, Jan 15, 2018 at 07:45:23AM -0500, Brian Foster wrote: > > > > > On Sat, Jan 13, 2018 at 01:23:53PM +1100, Dave Chinner wrote: > > > > > > That's the whole point of adding debug asserts in cases like this - > > > > > > they are supposed to stop test execution in it's tracks and leave a > > > > > > corpse to analyse. The auto group regression tests are not supposed > > > > > > to take the machine down on normal test configs (i.e. > > > > > > CONFIG_XFS_DEBUG=y). > > > > > > > > > > > > > > > > FWIW, my understanding of the dangerous group has always been that it's > > > > > for tests that when they trigger a regression, forcibly affect the > > > > > entire system as such (lockup, hang, crash, etc.). IMO, a test that > > > > > > I had the same understanding of dangerous group. And I recommended the > > > usage of "-g auto -x dangerous" before[1], and Dave acknowledged this > > > dangerous group usage[2] :) > > > > > > [1] https://www.spinics.net/lists/linux-btrfs/msg57312.html > > > [2] https://www.spinics.net/lists/linux-btrfs/msg57330.html > > > > Context is important. The context that the above links were talking > > about filtering the dangerous group was for older kernels that don't > > have the fixes for the bugs that crash the kernel. > > > > This particular test fails this auto group criteria in the case of > > people using CONFIG_XFS_DEBUG=y: > > > > " > > - it passes on current upstream kernels, if it fails, it's > > likely to be resolved in forseeable future [2] > > " > > > > It fails, and isn't likely to ever work, because the assert needs to > > remain there to catch userspace tool screwups.... > > > > > > If we really want to test these "should not ever happen" conditions > > > > that trigger asserts on debug kernels as "everyone always runs" > > > > regression testing, then these need to _notrun on CONFIG_XFS_DEBUG=y > > > > kernels. fstests already knows that it's running on a debug kernel, > > > > so adding a _requires_production_kernel check may be the way around > > > > this being considered a dangerous test. > > > > > > This reminds that we already have a _require_no_xfs_debug rule, as used > > > in xfs/115, which is known to trigger ASSERT failure on debug build. So > > > we can do the same in this new test. > > > > Ok, good! And it appears to be addressing the same "intentional > > corruption triggers failures" case as we are discussing here: > > > > My only suggestion (re: my previous comment) is to consider using a new > check that looks at bug_on_assert when the knob is available for tests > that are expected to generate asserts as such. The test stil has to Looks like we need a new _require_no_xfs_bug_on_assert, which looks at /sys/fs/xfs/debug/bug_on_assert value if it's available, and just calls _require_no_xfs_debug if it's not available. > filter the assert itself to cover the WARN=1 case, right? I think so (and to cover the bug_on_assert=0 case). > > > # we corrupt XFS on purpose, and debug built XFS would crash due to assert > > # failure, so skip if we're testing on a debug built XFS > > _require_no_xfs_debug > > > > Only with CONFIG_XFS_ASSERT_FATAL=y! :) XFS_ASSERT_FATAL and _require_no_xfs_debug were introduced closely in time. XFS_ASSERT_FATAL was there first, and it was too new to be noticed when we then introduced _require_no_xfs_debug.. Thanks, Eryu -- 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
