Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

Hi Ted,

Thanks for encouraging and helping us in the course of building
CrashMonkey for file-system crash consistency testing. Our work is
published in OSDI '18
(https://www.usenix.org/conference/osdi18/presentation/mohan). While
our goal is to enable the integration of a tool like CrashMonkey in
the file-system development cycle, we think it would be beneficial to
add the test suite of about 300 workloads (consisting of 1 core
file-system operation) that we systematically generated, to xfstest.
The complete list of these workloads is in the CrashMonkey repository
(https://github.com/utsaslab/crashmonkey/tree/master/code/tests/seq1).
Given that developers currently use the xfstest, this addition would
ensure that simple crash-consistency bugs are eliminated from any
future kernel versions.

If this idea sounds good to you, we can write a patch for including
our workloads into xfstest, using dm_flakey. If there's some specific
format you want us to follow while writing patches for these
crash-consistency tests, we are happy to adopt it.

Thanks,
Jayashree Mohan

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Eryu Guan]

On Mon, Oct 15, 2018 at 03:58:06PM -0500, Jayashree Mohan wrote:
> Hi Ted,
> 
> Thanks for encouraging and helping us in the course of building
> CrashMonkey for file-system crash consistency testing. Our work is
> published in OSDI '18
> (https://www.usenix.org/conference/osdi18/presentation/mohan). While
> our goal is to enable the integration of a tool like CrashMonkey in
> the file-system development cycle, we think it would be beneficial to
> add the test suite of about 300 workloads (consisting of 1 core
> file-system operation) that we systematically generated, to xfstest.
> The complete list of these workloads is in the CrashMonkey repository
> (https://github.com/utsaslab/crashmonkey/tree/master/code/tests/seq1).
> Given that developers currently use the xfstest, this addition would
> ensure that simple crash-consistency bugs are eliminated from any
> future kernel versions.

It's great to have more crash-consistency tests in fstests, thanks!

> 
> If this idea sounds good to you, we can write a patch for including
> our workloads into xfstest, using dm_flakey. If there's some specific
> format you want us to follow while writing patches for these
> crash-consistency tests, we are happy to adopt it.

For starters, please follow the new test template generated by the 'new'
script, e.g.

./new generic

which will find the next free test sequence ID in 'generic' dir and
generate a tests/generic/<seq> test template file, you could modify that
file for your new tests.

Filipe Manana has contributed many tests that take use of dm_flakey, and
I find them all well-formated and easy to read (e.g. with clear comments
explaining the test). I'd recommend you take Filipe Manana's tests as
example.

Thanks,
Eryu

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

Hi Eryu,

Thanks for the response! Will follow the template you suggested for
writing  test cases.

CrashMonkey generates 300 test cases, which we’ll convert to xfstest.
Should we submit each test case as a different patch, or say 10 test
cases per patch, or all of them in a single patch ? Let us know if
there’s a preference.

On Sun, Oct 21, 2018 at 5:45 AM Eryu Guan <guaneryu@gmail.com> wrote:
>
> On Mon, Oct 15, 2018 at 03:58:06PM -0500, Jayashree Mohan wrote:
> > Hi Ted,
> >
> > Thanks for encouraging and helping us in the course of building
> > CrashMonkey for file-system crash consistency testing. Our work is
> > published in OSDI '18
> > (https://www.usenix.org/conference/osdi18/presentation/mohan). While
> > our goal is to enable the integration of a tool like CrashMonkey in
> > the file-system development cycle, we think it would be beneficial to
> > add the test suite of about 300 workloads (consisting of 1 core
> > file-system operation) that we systematically generated, to xfstest.
> > The complete list of these workloads is in the CrashMonkey repository
> > (https://github.com/utsaslab/crashmonkey/tree/master/code/tests/seq1).
> > Given that developers currently use the xfstest, this addition would
> > ensure that simple crash-consistency bugs are eliminated from any
> > future kernel versions.
>
> It's great to have more crash-consistency tests in fstests, thanks!
>
> >
> > If this idea sounds good to you, we can write a patch for including
> > our workloads into xfstest, using dm_flakey. If there's some specific
> > format you want us to follow while writing patches for these
> > crash-consistency tests, we are happy to adopt it.
>
> For starters, please follow the new test template generated by the 'new'
> script, e.g.
>
> ./new generic
>
> which will find the next free test sequence ID in 'generic' dir and
> generate a tests/generic/<seq> test template file, you could modify that
> file for your new tests.
>
> Filipe Manana has contributed many tests that take use of dm_flakey, and
> I find them all well-formated and easy to read (e.g. with clear comments
> explaining the test). I'd recommend you take Filipe Manana's tests as
> example.
>
> Thanks,
> Eryu

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Theodore Y. Ts'o]

On Sun, Oct 21, 2018 at 10:48:56AM -0500, Jayashree Mohan wrote:
> Hi Eryu,
> 
> Thanks for the response! Will follow the template you suggested for writing
>  test cases.
> 
> CrashMonkey generates 300 test cases, which we’ll convert to xfstest.
> Should we submit each test case as a different patch, or say 10 test cases
> per patch, or all of them in a single patch ? Let us know if there’s a
> preference.

How long does each test case take to run?  And note, by the way, that
by default we automatically run fsck on the test device after each
test.  So number one, if you use the test device, you don't need to
worry about running fsck explicitly; the xfstests check script will do
that, and fail the test if the file system is corrupted --- and number
two, this will influence whether which groups each test should be
assigned.

See the file xfstests-dev/tests/generic/group to see how groups get
assigned to tests.  I suppose all of the crashmonkey tests should be
assigned to a new group, say, "crashmonkey".  Whether or not they
should get assigned to the "auto" or "quick" group is a different
question.  Note that if running these tests will signicantly increase
the test run time of smoke tests and even the full "automatic"
regression tests, there may be some resistence in adding all of these
tests to the "auto" or "quick" groups.  Or even if you do, many file
system developers may choose to exclude all tests from the
"crashmonkey" group because if a 15 minute smoke test suddenly gets
extended to take 6 hours, developers are wont to get.... cranky.  :-)

						- Ted

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

> How long does each test case take to run?
All the tests would touch 4 files at most or write 32-64KB of data to
a file, starting from a empty file system image (hence very minimal
running time). We are not going to bring CrashMonkey in the loop - we
will port the generated tests to xfstest using dm-flakey (like
generic/498 [1], which was submitted in response to a bug found by
CrashMonkey in btrfs). Hence, each test should take the same time as
that of current crash-consistency tests in the xfstest suite (for
example, similar to generic/034 which takes about a second to run).

> And note, by the way, that
> by default we automatically run fsck on the test device after each
> test.  So number one, if you use the test device, you don't need to
> worry about running fsck explicitly; the xfstests check script will do
> that, and fail the test if the file system is corrupted --- and number
> two, this will influence whether which groups each test should be
> assigned.

Noted. Since we will be writing the out-file (checker) manually, will
ensure that checks only the content/metadata.

> See the file xfstests-dev/tests/generic/group to see how groups get
> assigned to tests.  I suppose all of the crashmonkey tests should be
> assigned to a new group, say, "crashmonkey".  Whether or not they
> should get assigned to the "auto" or "quick" group is a different
> question.  Note that if running these tests will signicantly increase
> the test run time of smoke tests and even the full "automatic"
> regression tests, there may be some resistence in adding all of these
> tests to the "auto" or "quick" groups.  Or even if you do, many file
> system developers may choose to exclude all tests from the
> "crashmonkey" group because if a 15 minute smoke test suddenly gets
> extended to take 6 hours, developers are wont to get.... cranky.  :-)

It makes sense to add it to a new group as you suggest, and
considering a second to run each test, it should take around 5 minutes
to run this batch of CrashMonkey tests. Once the tests cases are
ready, we can give you a better estimate of total time spent on the
newly added tests.

[1] https://patchwork.kernel.org/patch/10458749/

Thanks,
Jayashree

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Dave Chinner]

On Sun, Oct 21, 2018 at 05:52:21PM -0500, Jayashree Mohan wrote:
> > How long does each test case take to run?
> All the tests would touch 4 files at most or write 32-64KB of data to
> a file, starting from a empty file system image (hence very minimal
> running time). We are not going to bring CrashMonkey in the loop - we
> will port the generated tests to xfstest using dm-flakey (like
> generic/498 [1], which was submitted in response to a bug found by
> CrashMonkey in btrfs). Hence, each test should take the same time as
> that of current crash-consistency tests in the xfstest suite (for
> example, similar to generic/034 which takes about a second to run).
> 
> > And note, by the way, that
> > by default we automatically run fsck on the test device after each
> > test.  So number one, if you use the test device, you don't need to
> > worry about running fsck explicitly; the xfstests check script will do
> > that, and fail the test if the file system is corrupted --- and number
> > two, this will influence whether which groups each test should be
> > assigned.
> 
> Noted. Since we will be writing the out-file (checker) manually, will
> ensure that checks only the content/metadata.
> 
> > See the file xfstests-dev/tests/generic/group to see how groups get
> > assigned to tests.  I suppose all of the crashmonkey tests should be
> > assigned to a new group, say, "crashmonkey".  Whether or not they
> > should get assigned to the "auto" or "quick" group is a different
> > question.  Note that if running these tests will signicantly increase
> > the test run time of smoke tests and even the full "automatic"
> > regression tests, there may be some resistence in adding all of these
> > tests to the "auto" or "quick" groups.  Or even if you do, many file
> > system developers may choose to exclude all tests from the
> > "crashmonkey" group because if a 15 minute smoke test suddenly gets
> > extended to take 6 hours, developers are wont to get.... cranky.  :-)
> 
> It makes sense to add it to a new group as you suggest, and
> considering a second to run each test, it should take around 5 minutes
> to run this batch of CrashMonkey tests. Once the tests cases are
> ready, we can give you a better estimate of total time spent on the
> newly added tests.

Add the time between tests - fsck checks, scrub, etc, and that can
easily add another 10s per test.

Hence I'd strongly encourage you to batch similar tests into a
single xfstest test so that we're not needlessly adding 300x15s to
every test run because of the per-test external overhead.....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

Hi Dave,

Agreed that there could be other overheads per test, which we'll try
to minimize by batching.

However, even if we batch similar test cases into one,
_scratch_shutdown and _scratch_cycle_mount() will be called in each
sub-case within the test. Doesn't this mean you will run fsck after
each sub-test too?



On Sun, Oct 21, 2018 at 9:05 PM Dave Chinner <david@fromorbit.com> wrote:
>
> On Sun, Oct 21, 2018 at 05:52:21PM -0500, Jayashree Mohan wrote:
> > > How long does each test case take to run?
> > All the tests would touch 4 files at most or write 32-64KB of data to
> > a file, starting from a empty file system image (hence very minimal
> > running time). We are not going to bring CrashMonkey in the loop - we
> > will port the generated tests to xfstest using dm-flakey (like
> > generic/498 [1], which was submitted in response to a bug found by
> > CrashMonkey in btrfs). Hence, each test should take the same time as
> > that of current crash-consistency tests in the xfstest suite (for
> > example, similar to generic/034 which takes about a second to run).
> >
> > > And note, by the way, that
> > > by default we automatically run fsck on the test device after each
> > > test.  So number one, if you use the test device, you don't need to
> > > worry about running fsck explicitly; the xfstests check script will do
> > > that, and fail the test if the file system is corrupted --- and number
> > > two, this will influence whether which groups each test should be
> > > assigned.
> >
> > Noted. Since we will be writing the out-file (checker) manually, will
> > ensure that checks only the content/metadata.
> >
> > > See the file xfstests-dev/tests/generic/group to see how groups get
> > > assigned to tests.  I suppose all of the crashmonkey tests should be
> > > assigned to a new group, say, "crashmonkey".  Whether or not they
> > > should get assigned to the "auto" or "quick" group is a different
> > > question.  Note that if running these tests will signicantly increase
> > > the test run time of smoke tests and even the full "automatic"
> > > regression tests, there may be some resistence in adding all of these
> > > tests to the "auto" or "quick" groups.  Or even if you do, many file
> > > system developers may choose to exclude all tests from the
> > > "crashmonkey" group because if a 15 minute smoke test suddenly gets
> > > extended to take 6 hours, developers are wont to get.... cranky.  :-)
> >
> > It makes sense to add it to a new group as you suggest, and
> > considering a second to run each test, it should take around 5 minutes
> > to run this batch of CrashMonkey tests. Once the tests cases are
> > ready, we can give you a better estimate of total time spent on the
> > newly added tests.
>
> Add the time between tests - fsck checks, scrub, etc, and that can
> easily add another 10s per test.
>
> Hence I'd strongly encourage you to batch similar tests into a
> single xfstest test so that we're not needlessly adding 300x15s to
> every test run because of the per-test external overhead.....
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Amir Goldstein]

> >
> > > See the file xfstests-dev/tests/generic/group to see how groups get
> > > assigned to tests.  I suppose all of the crashmonkey tests should be
> > > assigned to a new group, say, "crashmonkey".

Wait, let me get it straight. Did crashmonkey produce 300 test cases or
did it find 300 bugs? Are all those test cases passing on all filesystems?
Some test cases failing on some filesystems?

In any case, I would add tests to group "crash" or "flakey", as there are
other crash consistent tests that could fit in that group in current tests.
If there is something differentiating the new tests from existing crash
tests they could be added to yet another group.

If it is not 300 bugs, but 300 generated sequences and crashmonkey
OSDI paper 2019 is going to result in 26000 generated sequences,
then the test group would be better characterized as -g generic/fuzzers.

> > > Note that if running these tests will signicantly increase
> > > the test run time of smoke tests and even the full "automatic"
> > > regression tests, there may be some resistence in adding all of these
> > > tests to the "auto" or "quick" groups.  Or even if you do, many file
> > > system developers may choose to exclude all tests from the
> > > "crashmonkey" group because if a 15 minute smoke test suddenly gets
> > > extended to take 6 hours, developers are wont to get.... cranky.  :-)
> >
> > It makes sense to add it to a new group as you suggest, and
> > considering a second to run each test, it should take around 5 minutes
> > to run this batch of CrashMonkey tests. Once the tests cases are
> > ready, we can give you a better estimate of total time spent on the
> > newly added tests.
>
> Add the time between tests - fsck checks, scrub, etc, and that can
> easily add another 10s per test.
>

Jayashree,

Please note that the time between test highly depends on the scratch
partition size and media, so you may not be seeing the full effects of
running 300 "quick" tests on your system.

> Hence I'd strongly encourage you to batch similar tests into a
> single xfstest test so that we're not needlessly adding 300x15s to
> every test run because of the per-test external overhead.....
>

And that should be easy to do as:

test case #1: do
_flakey_drop_and_remount
test case #1: check
test case #1: clean

test case #2: do
_flakey_drop_and_remount
test case #2: check
test case #2: clean

I suppose you don't HAVE to run all test cases on a fresh created fs??
so clean would be just rm -rf or even each test case could use its own
subdir?

Thanks,
Amir.

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Amir Goldstein]

On Mon, Oct 22, 2018 at 5:45 AM Jayashree Mohan <jayashree2912@gmail.com> wrote:
>
> Hi Dave,
>
> Agreed that there could be other overheads per test, which we'll try
> to minimize by batching.
>
> However, even if we batch similar test cases into one,
> _scratch_shutdown and _scratch_cycle_mount() will be called in each

Wait, why _scratch_shutdown and not using dm_flakey?
Fewer filesystems support _scratch_shutdown and _scratch_shutdown
could be buggy and not simulate crash accurately.

> sub-case within the test. Doesn't this mean you will run fsck after
> each sub-test too?
>

_scratch_cycle_mount does not fsck.

Thanks,
Amir.

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

On Sun, Oct 21, 2018 at 9:44 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> > >
> > > > See the file xfstests-dev/tests/generic/group to see how groups get
> > > > assigned to tests.  I suppose all of the crashmonkey tests should be
> > > > assigned to a new group, say, "crashmonkey".
>
> Wait, let me get it straight. Did crashmonkey produce 300 test cases or
> did it find 300 bugs? Are all those test cases passing on all filesystems?
> Some test cases failing on some filesystems?

CrashMonkey generates 300 workloads, out of which 3 tests result in
bugs in two file systems (btrfs and F2FS). Others passed clean for
ext4, xfs, btrfs and F2FS. Given that xfstest is a regression test
suite, we thought it would be beneficial to add all 300 workloads to
the generic test - it is possible that a future kernel version has a
bug that could be triggered by one of these 300 workloads. We say so
because, these workloads test the file-system in a systematic manner
using common file-system operations. Given that crash-consistency
tests are sparse, checking a file system against these 300 workloads
will at least ensure that the file system is free from simple
crash-consistency bugs (in addition to testing for bugs that were
reported in earlier kernel versions).

> In any case, I would add tests to group "crash" or "flakey", as there are
> other crash consistent tests that could fit in that group in current tests.
> If there is something differentiating the new tests from existing crash
> tests they could be added to yet another group.
>
> If it is not 300 bugs, but 300 generated sequences and crashmonkey
> OSDI paper 2019 is going to result in 26000 generated sequences,
> then the test group would be better characterized as -g generic/fuzzers.
>
> > > > Note that if running these tests will signicantly increase
> > > > the test run time of smoke tests and even the full "automatic"
> > > > regression tests, there may be some resistence in adding all of these
> > > > tests to the "auto" or "quick" groups.  Or even if you do, many file
> > > > system developers may choose to exclude all tests from the
> > > > "crashmonkey" group because if a 15 minute smoke test suddenly gets
> > > > extended to take 6 hours, developers are wont to get.... cranky.  :-)
> > >
> > > It makes sense to add it to a new group as you suggest, and
> > > considering a second to run each test, it should take around 5 minutes
> > > to run this batch of CrashMonkey tests. Once the tests cases are
> > > ready, we can give you a better estimate of total time spent on the
> > > newly added tests.
> >
> > Add the time between tests - fsck checks, scrub, etc, and that can
> > easily add another 10s per test.
> >
>
> Jayashree,
>
> Please note that the time between test highly depends on the scratch
> partition size and media, so you may not be seeing the full effects of
> running 300 "quick" tests on your system.

Got it.


> > Hence I'd strongly encourage you to batch similar tests into a
> > single xfstest test so that we're not needlessly adding 300x15s to
> > every test run because of the per-test external overhead.....
> >
>
> And that should be easy to do as:
>
> test case #1: do
> _flakey_drop_and_remount
> test case #1: check
> test case #1: clean
>
> test case #2: do
> _flakey_drop_and_remount
> test case #2: check
> test case #2: clean
>
> I suppose you don't HAVE to run all test cases on a fresh created fs??
> so clean would be just rm -rf or even each test case could use its own
> subdir?

You are right. A simple cleanup will work. We can club multiple of our
workloads into one test and create something like 1 test case per
file-system operation.

Thanks,
Jayashree.

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

On Sun, Oct 21, 2018 at 9:49 PM Amir Goldstein <amir73il@gmail.com> wrote:

> Wait, why _scratch_shutdown and not using dm_flakey?
> Fewer filesystems support _scratch_shutdown and _scratch_shutdown
> could be buggy and not simulate crash accurately.

Thanks for this note. I was looking at previous xfstest tests
(something not very old - generic/468 for eg), and it was using
scratch_shutdown. Hence I used that as an example. I'll use
flakey_drop_and_remount while writing up future patches.

Doesn't this mean the current tests in xfstest suite might miss bugs
in current/future kernel versions, because some file systems don't
support it? There are many crash-consistency tests in xfstest suite
that still use _scratch_shutdown. In fact generic/468 is one of the
test cases that could not run on btrfs because it does not support
_shutdown. But then this is the exact test case required to reveal a
bug in btrfs - where you lose allocated blocks beyond the eof on
fallocate. Just wanted to bring this up, in case you did not notice it
:)

Thanks,
Jayashree

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Amir Goldstein]

On Mon, Oct 22, 2018 at 6:09 AM Jayashree Mohan <jayashree2912@gmail.com> wrote:
>
> On Sun, Oct 21, 2018 at 9:44 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > > >
> > > > > See the file xfstests-dev/tests/generic/group to see how groups get
> > > > > assigned to tests.  I suppose all of the crashmonkey tests should be
> > > > > assigned to a new group, say, "crashmonkey".
> >
> > Wait, let me get it straight. Did crashmonkey produce 300 test cases or
> > did it find 300 bugs? Are all those test cases passing on all filesystems?
> > Some test cases failing on some filesystems?
>
> CrashMonkey generates 300 workloads, out of which 3 tests result in
> bugs in two file systems (btrfs and F2FS).

So I suppose those 3 tests should be separate and mention the fix
commit (if there is one?).

> Others passed clean for
> ext4, xfs, btrfs and F2FS. Given that xfstest is a regression test
> suite, we thought it would be beneficial to add all 300 workloads to
> the generic test - it is possible that a future kernel version has a
> bug that could be triggered by one of these 300 workloads. We say so
> because, these workloads test the file-system in a systematic manner
> using common file-system operations. Given that crash-consistency
> tests are sparse, checking a file system against these 300 workloads
> will at least ensure that the file system is free from simple
> crash-consistency bugs (in addition to testing for bugs that were
> reported in earlier kernel versions).
>

Well, assuming your tests are quick, IMO
auto quick fuzzer crash

Is descriptive enough. Let's see what other people think.

> > I suppose you don't HAVE to run all test cases on a fresh created fs??
> > so clean would be just rm -rf or even each test case could use its own
> > subdir?
>
> You are right. A simple cleanup will work. We can club multiple of our
> workloads into one test and create something like 1 test case per
> file-system operation.
>

If that results in a "few" more "quick" tests, then I think its a good
way to go.

Thanks,
Amir.

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Dave Chinner]

On Sun, Oct 21, 2018 at 10:23:06PM -0500, Jayashree Mohan wrote:
> On Sun, Oct 21, 2018 at 9:49 PM Amir Goldstein <amir73il@gmail.com> wrote:
> 
> > Wait, why _scratch_shutdown and not using dm_flakey?
> > Fewer filesystems support _scratch_shutdown and _scratch_shutdown
> > could be buggy and not simulate crash accurately.
> 
> Thanks for this note. I was looking at previous xfstest tests
> (something not very old - generic/468 for eg), and it was using
> scratch_shutdown. Hence I used that as an example. I'll use
> flakey_drop_and_remount while writing up future patches.
> 
> Doesn't this mean the current tests in xfstest suite might miss bugs
> in current/future kernel versions, because some file systems don't
> support it?

scratch_shutdown is testing shutdown behaviour, whatever the cause.
Not all filesystems have a shutdown mechanism - it's a specific set
of operations that the filesystem performs in response to failure
triggers. Some filesystems just go read-only or have other response
mechanisms, which aren't compatible with expected shutdown
behaviour.

And because shutdowns can be caused by things other than IO errors
(which dm-flakey triggers) and so need to be tested /in addition/ to
other types of failures.

> There are many crash-consistency tests in xfstest suite
> that still use _scratch_shutdown.

Yes, because shutdowns can /simulate/ such failures.

> In fact generic/468 is one of the
> test cases that could not run on btrfs because it does not support
> _shutdown.

Yes, but that doesn't stop us from having the test. If btrfs were to
implement a shutdown mechanism, then they'd gain coverage from these
tests too.

> But then this is the exact test case required to reveal a
> bug in btrfs - where you lose allocated blocks beyond the eof on
> fallocate. Just wanted to bring this up, in case you did not notice it

Then add support for shutdowns to btrfs, and it will get test
coverage from these tests. That's not a problem with the test -
that's a filesystem functionality deficiency. i.e. fix the
filesystem, don't change the test.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Dave Chinner]

On Sun, Oct 21, 2018 at 10:09:01PM -0500, Jayashree Mohan wrote:
> On Sun, Oct 21, 2018 at 9:44 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > > >
> > > > > See the file xfstests-dev/tests/generic/group to see how groups get
> > > > > assigned to tests.  I suppose all of the crashmonkey tests should be
> > > > > assigned to a new group, say, "crashmonkey".
> >
> > Wait, let me get it straight. Did crashmonkey produce 300 test cases or
> > did it find 300 bugs? Are all those test cases passing on all filesystems?
> > Some test cases failing on some filesystems?
> 
> CrashMonkey generates 300 workloads, out of which 3 tests result in
> bugs in two file systems (btrfs and F2FS). Others passed clean for
> ext4, xfs, btrfs and F2FS. Given that xfstest is a regression test
> suite, we thought it would be beneficial to add all 300 workloads to
> the generic test -

Perhaps you should port a test or two so we can have a look at what
these tests are before we make any recomendations on the best way to
integrate them. 300 new tests is an awful lot to maintain if we ever
want to change anything....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Amir Goldstein]

On Mon, Oct 22, 2018 at 6:23 AM Jayashree Mohan <jayashree2912@gmail.com> wrote:
>
> On Sun, Oct 21, 2018 at 9:49 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> > Wait, why _scratch_shutdown and not using dm_flakey?
> > Fewer filesystems support _scratch_shutdown and _scratch_shutdown
> > could be buggy and not simulate crash accurately.
>
> Thanks for this note. I was looking at previous xfstest tests
> (something not very old - generic/468 for eg), and it was using
> scratch_shutdown. Hence I used that as an example. I'll use
> flakey_drop_and_remount while writing up future patches.
>
> Doesn't this mean the current tests in xfstest suite might miss bugs
> in current/future kernel versions, because some file systems don't
> support it? There are many crash-consistency tests in xfstest suite
> that still use _scratch_shutdown. In fact generic/468 is one of the
> test cases that could not run on btrfs because it does not support
> _shutdown. But then this is the exact test case required to reveal a
> bug in btrfs - where you lose allocated blocks beyond the eof on
> fallocate. Just wanted to bring this up, in case you did not notice it
> :)
>

xfstests grew up with xfs having shutdown ability, so I suppose some
shutdown tests test this functionality and some shutdown tests are pure
crash consistency tests.
Later, ext4 got shutdown support so it enjoys all this test coverage.
Quite recently, Chengguang Xu added shutdown support for testing
overlayfs over xfs/ext4. Note that this was done without changing kernel
code.

Josef Bacik added the dmflakey infrastructure to xfstests for btrfs crash tests
generic/311,321. Later, I also added Josef's dmlogwrites infrastructure for
crash stress tests. dmflakey and dmlogwrites _require_block_device,
so they don't run on overlayfs.

Now, if you write your tests with dmflakey, overlayfs will not run them
and if you write your tests with shutdown, btrfs won't run them.
Obviously, btrfs is more important in the context of your tests and adding
dmflakey support for overlayfs is something we can do in the future.

Thanks,
Amir.

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Theodore Y. Ts'o]

On Mon, Oct 22, 2018 at 03:02:31PM +1100, Dave Chinner wrote:
> scratch_shutdown is testing shutdown behaviour, whatever the cause.
> Not all filesystems have a shutdown mechanism - it's a specific set
> of operations that the filesystem performs in response to failure
> triggers. Some filesystems just go read-only or have other response
> mechanisms, which aren't compatible with expectetd shutdown
> behaviour.

What Dave said.

I'll note that at the moment ext4 passes dm_flakey power cut tests
reliably, which is *different* from the tests which use the
FS_IOC_SHUTDOWN ioctl.  At the moment ext4, fails FS_IOC_SHUTDOWN
tests around 3% -- 5% of the time --- which is why generic/388, which
repeatedly mounts and call FS_IOC_SHUTDOWN 30 times, is a flaky
(sorry) test for ext4.

So for the crashmonkey tests, if you are specifically trying to test
what happens on a power cut, you need to be using dm_flakey, and not
FS_IOC_SHUTDOWN.  Testing what happens on FS_IOC_SHUTDOWN would also
be useful, but it's a different thing.  (And at least for ext4, I know
what the problems are with ext4's FS_IOC_SHUTDOWN support; the
challenge is how to fix it without imposing a performance regression,
and since FS_IOC_SHUTDOWN is rarely used in production[1], it's been
low priority for me to fix.)

						- Ted

[1] Actually, Google does use FS_IOC_SHUTDOWN in production, but only
for scratch iSCSI volumes which have already disappeared and so we
don't care if the underlying file system might get corrupted, since
the whole point is to forcibly detach the failed iSCSI device while
keeping the iSCSI client system running.  (And since it's a scratch
volume, it's mounted with the journal disabled, which means on
shutdown, even we cared about the state of the scratch volume after
shutdown, by in no journal mode there are no guarantees about file
system consistency after a shutdown or power cut anyway.)

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Eric Sandeen]

On 10/21/18 11:15 PM, Dave Chinner wrote:
> On Sun, Oct 21, 2018 at 10:09:01PM -0500, Jayashree Mohan wrote:
>> On Sun, Oct 21, 2018 at 9:44 PM Amir Goldstein <amir73il@gmail.com> wrote:
>>>
>>>>>
>>>>>> See the file xfstests-dev/tests/generic/group to see how groups get
>>>>>> assigned to tests.  I suppose all of the crashmonkey tests should be
>>>>>> assigned to a new group, say, "crashmonkey".
>>>
>>> Wait, let me get it straight. Did crashmonkey produce 300 test cases or
>>> did it find 300 bugs? Are all those test cases passing on all filesystems?
>>> Some test cases failing on some filesystems?
>>
>> CrashMonkey generates 300 workloads, out of which 3 tests result in
>> bugs in two file systems (btrfs and F2FS). Others passed clean for
>> ext4, xfs, btrfs and F2FS. Given that xfstest is a regression test
>> suite, we thought it would be beneficial to add all 300 workloads to
>> the generic test -
> 
> Perhaps you should port a test or two so we can have a look at what
> these tests are before we make any recomendations on the best way to
> integrate them. 300 new tests is an awful lot to maintain if we ever
> want to change anything....

I agree with this, do a couple and let people take a look.

-Eric

Re: Submitting patches to xfstests based on OSDI '18 paper (CrashMonkey)

[Jayashree Mohan]

> Perhaps you should port a test or two so we can have a look at what
> these tests are before we make any recomendations on the best way to
> integrate them. 300 new tests is an awful lot to maintain if we ever
> want to change anything....

I have written a patch for testing simple "hard link" behaviour. This
patch batches 37 test cases generated by CrashMonkey into one  xfstest
test, all of them testing link behaviour.  These 37 tests have been
grouped based on the similarities, there are comments in the patch to
describe what each group is testing for. On a high-level, we are
testing the creation of hard links between files in same directory and
files across two directories, while allowing fsync of either the files
involved, their parent directories, or unrelated sibling files.

We aim to convert the other CrashMonkey tests in the same way, batched
by the type of file-system operation we test. This particular test
took 10 seconds to run on my VM (Ubuntu 16.04, 2GB RAM, single core)
on 100MB scratch partition. I have added per test case timer in this
patch, which shows each test case takes about 0.25 seconds (to run,
flakey_remount, test and cleanup). This test passes clean on ext4, xfs
and F2FS. It will show three failed cases as of kernel 4.16 on btrfs
(I think it's not yet patched on 4.19, so you should see the failure
on the newest kernel as well).

Thinking more about it, none of the CrashMonkey test cases check for
resource exhaustion and hence we don't need more than 100MB of scratch
device. If the per-test external overhead due to fsck/scrub etc
depends on the scratch partition size, we can speed up the CrashMonkey
tests by allowing a new device like scratch(much smaller in size -
about 100-200MB). Additionally, our tests also do not need fsck to be
run at the end. Is there a way to skip performing fsck after each test
(if its something configurable in the test file) ?

If this sort of patch seems fine to you, I can go ahead and port the
other CrashMonkey tests in the same way. After batching, I hope there
would be around 10-12 test files like the one attached here.

Patch below:
---

diff --git a/tests/generic/517-link b/tests/generic/517-link
new file mode 100755
index 0000000..791b6c0
--- /dev/null
+++ b/tests/generic/517-link
@@ -0,0 +1,162 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 The University of Texas at Austin.  All Rights Reserved.
+#
+# FS QA Test 517-link
+#
+# Test if we create a hard link to a file and persist either of the
files, all the names persist.
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1    # failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    _cleanup_flakey
+    cd /
+    rm -f $tmp.*
+}
+
+init_start_time=$(date +%s.%N)
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmflakey
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+_supported_fs generic
+_supported_os Linux
+_require_scratch
+_require_dm_target flakey
+
+# initialize scratch device
+_scratch_mkfs >>$seqres.full 2>&1
+_require_metadata_journaling $SCRATCH_DEV
+_init_flakey
+_mount_flakey
+init_end_time=$(date +%s.%N)
+init_time_elapsed=$(echo "$init_end_time-$init_start_time" | bc)
+echo "Initialization time = $init_time_elapsed" >> $seqres.full
+
+stat_opt='-c "blocks: %b size: %s inode: %i links: %h"'
+test_num=0
+total_time=0
+
+cleanup()
+{
+    rm -rf $SCRATCH_MNT/*
+    sync
+}
+
+# create a hard link $2 to file $1, and fsync $3, followed by power-cu
+test_link()
+{
+    start_time=$(date +%s.%N)
+    test_num=$((test_num + 1))
+    sibling=0
+    before=""
+    after=""
+    echo -ne "\n=== Test $test_num :  link $1 $2 " | tee -a $seqres.full
+
+    # now execute the workload
+    mkdir -p "${1%/*}" && mkdir -p "${2%/*}" && touch "$1"
+    ln $1 $2
+
+    if [ -z "$3" ]
+    then
+        echo -ne " with sync ===\n"
+        sync
+        before=`stat "$stat_opt" $1`
+    else
+        echo " with fsync $3 ==="
+
+        # If the file being persisted is a sibling, create it first
+        if [ ! -f $3 ]
+        then
+            sibling=1
+            touch $3
+        fi
+
+        $XFS_IO_PROG -c "fsync" $3 | _filter_xfs_io
+
+        if [ $sibling -ne 1 ]
+        then
+            before=`stat "$stat_opt" $1`
+        fi
+    fi
+
+    _flakey_drop_and_remount | tee -a $seqres.full
+
+    if [ -f $1 ]
+    then
+        after=`stat "$stat_opt" $1`
+    fi
+
+    if [ "$before" != "$after" ] && [ $sibling -ne 1 ]
+    then
+        echo "Before: $before" | tee -a $seqres.full
+        echo "After: $after" | tee -a $seqres.full
+    fi
+
+    cleanup
+    end_time=$(date +%s.%N)
+    time_elapsed=$(echo "$end_time-$start_time" | bc)
+    echo " Elapsed time : $time_elapsed" >> $seqres.full
+    total_time=$(echo "$total_time+$time_elapsed" | bc)
+    echo " Total time : $total_time" >> $seqres.full
+}
+
+# run the link test for different combinations
+
+test_start_time=$(date +%s.%N)
+# Group 1: Both files within root directory
+for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar; do
+    test_link $SCRATCH_MNT/foo $SCRATCH_MNT/bar $file_name
+done
+test_link $SCRATCH_MNT/foo $SCRATCH_MNT/bar
+
+# Group 2: Create hard link in a sub directory
+for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
$SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
+    test_link $SCRATCH_MNT/foo $SCRATCH_MNT/A/bar $file_name
+done
+test_link $SCRATCH_MNT/foo $SCRATCH_MNT/A/bar
+
+# Group 3: Create hard link in parent directory
+for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
$SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
+    test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/bar $file_name
+done
+test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/bar
+
+# Group 4: Both files within a directory other than root
+for file_name in $SCRATCH_MNT $SCRATCH_MNT/A $SCRATCH_MNT/A/bar
$SCRATCH_MNT/A/foo; do
+    test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/A/bar $file_name
+done
+test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/A/bar
+
+#Group 5: Exercise name reuse : Link file in sub-directory
+for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
$SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
+    test_link $SCRATCH_MNT/bar $SCRATCH_MNT/A/bar $file_name
+done
+test_link $SCRATCH_MNT/bar $SCRATCH_MNT/A/bar
+
+#Group 6: Exercise name reuse : Link file in parent directory
+for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
$SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
+    test_link $SCRATCH_MNT/A/bar $SCRATCH_MNT/bar $file_name
+done
+test_link $SCRATCH_MNT/A/bar $SCRATCH_MNT/bar
+
+test_end_time=$(date +%s.%N)
+test_time_elapsed=$(echo "$test_end_time-$test_start_time" | bc)
+echo "Test Elapsed time : $test_time_elapsed" >> $seqres.full
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/517-link.out b/tests/generic/517-link.out
new file mode 100644
index 0000000..381b644
--- /dev/null
+++ b/tests/generic/517-link.out
@@ -0,0 +1,75 @@
+QA output created by 517-link
+
+=== Test 1 :  link /mnt/scratch/foo /mnt/scratch/bar  with fsync
/mnt/scratch ===
+
+=== Test 2 :  link /mnt/scratch/foo /mnt/scratch/bar  with fsync
/mnt/scratch/foo ===
+
+=== Test 3 :  link /mnt/scratch/foo /mnt/scratch/bar  with fsync
/mnt/scratch/bar ===
+
+=== Test 4 :  link /mnt/scratch/foo /mnt/scratch/bar  with sync ===
+
+=== Test 5 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch ===
+
+=== Test 6 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/foo ===
+
+=== Test 7 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/bar ===
+
+=== Test 8 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/A ===
+
+=== Test 9 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/A/bar ===
+
+=== Test 10 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/A/foo ===
+
+=== Test 11 :  link /mnt/scratch/foo /mnt/scratch/A/bar  with sync ===
+
+=== Test 12 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with fsync
/mnt/scratch ===
+
+=== Test 13 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with fsync
/mnt/scratch/foo ===
+
+=== Test 14 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with fsync
/mnt/scratch/bar ===
+
+=== Test 15 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with fsync
/mnt/scratch/A ===
+
+=== Test 16 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with fsync
/mnt/scratch/A/bar ===
+
+=== Test 17 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with fsync
/mnt/scratch/A/foo ===
+
+=== Test 18 :  link /mnt/scratch/A/foo /mnt/scratch/bar  with sync ===
+
+=== Test 19 :  link /mnt/scratch/A/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch ===
+
+=== Test 20 :  link /mnt/scratch/A/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/A ===
+
+=== Test 21 :  link /mnt/scratch/A/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/A/bar ===
+
+=== Test 22 :  link /mnt/scratch/A/foo /mnt/scratch/A/bar  with fsync
/mnt/scratch/A/foo ===
+
+=== Test 23 :  link /mnt/scratch/A/foo /mnt/scratch/A/bar  with sync ===
+
+=== Test 24 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with fsync
/mnt/scratch ===
+
+=== Test 25 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with fsync
/mnt/scratch/foo ===
+
+=== Test 26 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with fsync
/mnt/scratch/bar ===
+
+=== Test 27 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with fsync
/mnt/scratch/A ===
+
+=== Test 28 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with fsync
/mnt/scratch/A/bar ===
+
+=== Test 29 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with fsync
/mnt/scratch/A/foo ===
+
+=== Test 30 :  link /mnt/scratch/bar /mnt/scratch/A/bar  with sync ===
+
+=== Test 31 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with fsync
/mnt/scratch ===
+
+=== Test 32 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with fsync
/mnt/scratch/foo ===
+
+=== Test 33 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with fsync
/mnt/scratch/bar ===
+
+=== Test 34 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with fsync
/mnt/scratch/A ===
+
+=== Test 35 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with fsync
/mnt/scratch/A/bar ===
+
+=== Test 36 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with fsync
/mnt/scratch/A/foo ===
+
+=== Test 37 :  link /mnt/scratch/A/bar /mnt/scratch/bar  with sync ===
diff --git a/tests/generic/group b/tests/generic/group
index 47de978..dc04152 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -519,3 +519,4 @@
 514 auto quick clone
 515 auto quick clone
 516 auto quick dedupe clone
+517-link crash