Linux-NVDIMM Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Eryu Guan <eguan@redhat.com>
To: Xiong Zhou <xzhou@redhat.com>
Cc: "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH] test: add fio test for device-dax
Date: Thu, 30 Mar 2017 16:09:21 +0800	[thread overview]
Message-ID: <20170330080921.GG22845@eguan.usersys.redhat.com> (raw)
In-Reply-To: <20170330061600.rmzagx2wcukw6no6@XZHOUW.usersys.redhat.com>

On Thu, Mar 30, 2017 at 02:16:00PM +0800, Xiong Zhou wrote:
> Ccing Eryu
> 
> On Wed, Mar 29, 2017 at 02:12:25PM -0700, Dan Williams wrote:
> > On Wed, Mar 29, 2017 at 2:04 PM, Jeff Moyer <jmoyer@redhat.com> wrote:
> > > Dan Williams <dan.j.williams@intel.com> writes:
> > >
> > >>>>>>> Can we stop with this kernel version checking, please?  Test to see if
> > >>>>>>> you can create a device dax instance.  If not, skip the test.  If so,
> > >>>>>>> and if you have a kernel that isn't fixed, so be it, you'll get
> > >>>>>>> failures.
> > >>>>>>
> > >>>>>> I'd rather not. It helps me keep track of what went in where. If you
> > >>>>>> want to run all the tests on a random kernel just do:
> > >>>>>>
> > >>>>>>     KVER="4.11.0" make check
> > >>>>>
> > >>>>> This, of course, breaks completely with distro kernels.
> > >>>>
> > >>>> Why does this break distro kernels? The KVER variable overrides "uname -r"
> > >>>
> > >>> Because some features may not exist in the distro kernels.  It's the
> > >>> same problem you outline with xfstests, below.
> > >>>
> > >>
> > >> Right, but you started off with suggesting that just running the test
> > >> and failing was ok, and that's the behavior you get with KVER=.
> > >
> > > Well, the goal was to be somewhat smart about it, by not even trying to
> > > test things that aren't implemented or configured into the current
> > > kernel (such as device dax, for example).  Upon thinking about it
> > > further, I don't think that gets us very far.  However, that does raise
> > > a use case that is not distro-specific.  If you don't enable device dax,
> > > your test suite will still try to run those tests.  ;-)
> > 
> > The other part of the equation is that I'm lazy and don't want to do
> > the extra work of validating the environment for each test. So just do
> > a quick version check and if the test fails you get to figure out what
> > configuration you failed to enable. The most common case being that
> > you failed to install the nfit_test modules in which case we do have a
> > capability check for that.
> > 
> > >>>>> You don't see this kind of checking in xfstests, for example.  git
> > >>>>> helps you keep track of what changes went in where (see git describe
> > >>>>> --contains).  It's weird to track that via your test harness.  So, I
> > >>>>> would definitely prefer to move to a model where we check for
> > >>>>> features instead of kernel versions.
> > >>>>
> > >>>> I see this as a deficiency of xfstests. We have had to go through and
> > >>>> qualify and track each xfstest and why it may fail with random
> > >>>> combinations of kernel, xfsprogs, or e2fsprogs versions. I'd much
> > >>>> prefer that upstream xfstests track the minimum versions of components
> > >>>> to make a given test pass so we can stop doing it out of tree.
> > >>>
> > >>> Yes, that's a good point.  I can't think of a good compromise, either.
> > >>
> > >> Maybe we can at least get our annotated blacklist upstream so other
> > >> people can start contributing to it?
> > >
> > > Are you referring to xfstests?  Yeah, that's a good idea.  Right now I
> > > just add tests to the dangerous group as I encounter known issues.  ;-)
> > > So, my list probably isn't very helpful in its current form.
> > 
> > Yes, xfstests. We have entries in our blacklist like this:
> 
> Just a thought.
> 
> How about:
>   0, Adding infrastructure to teach xfstests querying know issues
> before reporting pass or fail.
>   1, Formatted known issue files. xfstests may maintain some in tree,
> while people can have theire own in hand.

I've posted similar RFC patch to fstests before.

[RFC PATCH] fstests: add known issue support
https://www.spinics.net/lists/fstests/msg02344.html

And Dave rejected it:
https://www.spinics.net/lists/fstests/msg02345.html

So the short answer is: xfstests should only control whether a test
should be run or not, the known issues information should be maintained
outside the test harness.

And I tend to agree with Dave now :)

> 
> Questions:
>   0, Tracking components, like kernel, xfstests, e2fsprogs, xfsprogs,
> fio, etc
>   1, Tracking versions of components, different distributions,
> upstream versions.
>   2, Versions of xfstests itself. We don't have many tags now,
> however if we start to do this, it will not be an issue.
>   3, Tracking architectures/platforms, x86_64, ppc64, etc
>   4, Is this matrix too big ? :)
>   5, Is this failure now the same faliure that I've got from
> the known issues?

I know the results handling of xfstests need more work, and the progress
is slow, but we do have some processes, e.g. configurable $RESULTDIR,
recent tools from Eric to compare failures across runs
(tools/compare-failures).

I've been thinking about Dave's suggestions for a long time, but due to
the complexity of known issues (see above questions :) and other
tasks/jobs, I'm not able to work out a solution yet. I'll look into it
again, thanks for the reminder :)

Thanks,
Eryu

> 
> Thanks,
> Xiong
> 
> > 
> > # needs xfsprogs fix
> > # c8dc42356142 xfs_db: fix the 'source' command when passed as a -c option
> > # Last checked:
> > # - xfsprogs-4.9.0-1.fc25.x86_64
> > xfs/138
> > 
> > # needs xfsprogs fix
> > # 3297e0caa25a xfs: replace xfs_mode_to_ftype table with switch statement
> > # Last checked:
> > # - xfsprogs-4.9.0-1.fc25.x86_64
> > xfs/348
> > 
> > # see "[BUG] generic/232 hangs on XFS DAX mount" thread on xfs mailing
> > # list
> > generic/232
> > 
> > # failed on emulated pmem without dax, may be impacted by the same fix
> > # as the one for generic/270. The generic/270 failure is tracked in this
> > # thread on the xfs mailing list: "XFS kernel BUG during generic/270
> > # with v4.10". Re-test on v4.11 with fa7f138 ("xfs: clear delalloc and
> > # cache on buffered write failure")
> > generic/269
> > generic/270
> > _______________________________________________
> > Linux-nvdimm mailing list
> > Linux-nvdimm@lists.01.org
> > https://lists.01.org/mailman/listinfo/linux-nvdimm
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

  reply	other threads:[~2017-03-30  8:09 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-28  4:03 [PATCH] test: add fio test for device-dax Dan Williams
2017-03-29 20:02 ` Jeff Moyer
2017-03-29 20:07   ` Dan Williams
2017-03-29 20:19     ` Jeff Moyer
2017-03-29 20:30       ` Dan Williams
2017-03-29 20:49         ` Jeff Moyer
2017-03-29 20:56           ` Dan Williams
2017-03-29 21:04             ` Jeff Moyer
2017-03-29 21:12               ` Dan Williams
2017-03-30  6:16                 ` Xiong Zhou
2017-03-30  8:09                   ` Eryu Guan [this message]
2017-03-31 15:50                     ` Dan Williams
2017-03-30  8:47               ` Johannes Thumshirn
2017-03-30 16:08         ` Linda Knippers
2017-03-30 16:56           ` Dan Williams
2017-03-30 17:06             ` Linda Knippers
2017-03-30 17:12               ` Dan Williams
2017-03-30 17:19                 ` Linda Knippers
2017-03-30 17:59                   ` Dan Williams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170330080921.GG22845@eguan.usersys.redhat.com \
    --to=eguan@redhat.com \
    --cc=linux-nvdimm@lists.01.org \
    --cc=xzhou@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox