From: Dave Chinner <david@fromorbit.com>
To: Ben Myers <bpm@sgi.com>
Cc: Mark Tinguely <tinguely@sgi.com>, xfs@oss.sgi.com
Subject: Re: [RFC] [PATCH 0/18] xfstests: move tests out of top level
Date: Fri, 24 Aug 2012 14:07:24 +1000 [thread overview]
Message-ID: <20120824040724.GA19235@dastard> (raw)
In-Reply-To: <20120823170025.GG29979@sgi.com>
On Thu, Aug 23, 2012 at 12:00:25PM -0500, Ben Myers wrote:
> On Thu, Aug 23, 2012 at 09:42:19AM +1000, Dave Chinner wrote:
> > On Wed, Aug 22, 2012 at 02:16:42PM -0500, Ben Myers wrote:
> > > On Wed, Aug 22, 2012 at 08:09:26AM +1000, Dave Chinner wrote:
> > > > > > For it to be useful in an automated test environment, it would need
> > > > > > to be re-implemented from scratch with reliable recording of results
> > > > > > and the ability to determine if a result is unusual or not. None of
> > > > > > this exists - it's just a framework to run a couple of benchmarks
> > > > > > and dump some output to stdout using the xfstests machine config
> > > > > > files....
> > > > > >
> > > > > > I have tried integrating other benchmarks into xfstests a while back
> > > > > > (e.g. compile bench, fsmark, etc) and using the results for some
> > > > > > kind of meaningful performance regression test. I rapidly came to
> > > > > > the conclusion that the infrastructure was not up to scratch and
> > > > > > that my simple handwritten standalone test scripts to iterate
> > > > > > through benchmarks and capture results was much easier to use and
> > > > > > modify than to jump through the weird bench infrastructure hoops.
> > > > > >
> > > > > > So, no, I don't think it's worth keeping at all.
> > > > >
> > > > > You've already made it clear that you feel the current bench implementation is
> > > > > not worth keeping. Once a suitable replacement for the bench infrastructure
> > > > > has been implemented we can remove the old one. Until then we prefer to keep
> > > > > what we have in the tree.
> > > >
> > > > That's not how the process works
> > >
> > > That is exactly how the process works. You posted an RFC and Mark and the XFS
> > > team at SGI walked through your patch set. Mark subsequently posted the
> > > commentary in reply to your RFC. Cruft or not, the removal of a feature goes
> > > through the same review process as everything else.
> >
> > Sure, but you need to justify your arguments for keeping something
> > with evidence and logic - handwaving about wanting something is, and
> > always has been, insufficient justification. That's the part of the
> > process I'm talking about - that statements of need require
> > evidence, especially when you agreed to the removal at LSF in San
> > Fransisco a few months ago. My arguments at the time were:
> >
> > a) nobody is actually using it,
> > b) it has effectively been unmaintained since 2003
> > c) it has no regression analysis or detection capability
> > d) it shares *very little* of xfstests
> > e) it gets in the way of cleaning up xfstests
> > f) there are far better workload generators that are being
> > actively maintained.
> >
> > And AFAIA, nothing has changed in the past few months.
>
> "In this case, SGI would like to keep the benchmark capability in xfstests in
> order have a better chance of catching performance regressions." There has
> been a been performance regression in the past few months (and there will be
> more in the future), we have had performance regressions internally too, and
> this has brought the value of having benchmarks in xfstests into sharp focus.
I heard you the first time - it didn't answer the first questions I
asked, Repeating it doesn't answer the second set of questions,
either, which could be answered with "yes" or "no". That is: are you
using bench *right now* for perforamnce regression testing?
The information I'm after is whether removing it breaks your current
test environment. Given you are suggesting moving it out of the way
rather than removal, I think the answer is "no" but I'd like a yes
or no confirming that.
> > OK, so moving it to revision history will be just fine until patches
> > are written some time in the future to make it work again in a
> > subdirectory.
> >
> > But before anything major gets done with bench, there needs to be a
> > coherent development plan produced.
>
> Doesn't removing bench fall in to the category 'major'?
Not really, because it's all of 5 minutes work in a larger project.
But for the sake of argument, let's say that it is and so I need to
communicate and develop a plan....
> Did you develop a
> coherent development plan on how to replace it with something better?
Yes, I communicated and developed a plan, and got agreement on it,
too. The plan was to remove it as there are other benchmark/test
suites better suited to performance regression testing than
xfstests. We discussed this and a consensus was reached on this at
LSF. Everything in the patchset is in my notes from the LSF
discussion....
> > Then, once we have an idea of what is going to be done, the white
> > elephant can then be addressed: is xfstests the right place for this
> > functionality?
>
> I think it is the perfect place. xfstests already has a wide following with
> linux filesystems folks, so if we get bench cleaned up everyone will have
> access to the same suite automatically. I'd really like the focus to stay on
> improving xfstests as opposed to some other suite, and I prefer not to be doing
> SGI internal-only test suites for benchmarking and testing where possible.
There's no reason why a new performance regression suite would have
to be SGI internal. If you want it to be part of xfstests so the
work is put into a public GPL project, then I think your motivation
for using/keeping bench is all wrong....
Anyway, let's leave it there. Gather requirements (e.g. put out a
request for discussion on linux-fsdevel), research existing tools
that can do the job, develop a plan, then we can discuss how best ot
proceed.
> > FWIW, this is the sort of reporting that a performance regression
> > test suite should produce:
> >
> > http://lists.linux.hp.com/~enw/ext4/3.2/
>
> Yeah, that's really nice. Do you happen to know what tool created it?
IIRC, a relatively simple set of scripts around the outside of ffsb,
lockstat, oprofile and gnuplot. You should probably ask Eric if he
can share them...
>
> > Indeed, why start with bench when you can start with something far
> > more advanced....
>
> I understand that bench is bitrotted, but it still has some value even today.
> Phil has agreed to take this on as a project so the bitrot will be addressed.
> You have good points about needing a better plan in this area. But we should
> come up with a plan before taking the major step of removing benchmarking from
> xfstests entirely. That's not handwaving, it's good sense. ;)
>
> Lets stay focused on improving xfstests...
Yep, I'm trying to do that by removing peripheral, non-core
functionality. ;)
Really, it makes no difference to me whether I remove bench or move
it to a sub-directory in a broken state. If you are really that set
on it being useful, I'll move it to another directory (say
"broken-bench-do-not-sit-down-here" :) and leave it in a
busted state. If it hasn't been fixed 6 months later, I'll post
patches to remove it again....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-08-24 4:06 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-26 9:27 [RFC] [PATCH 0/18] xfstests: move tests out of top level Dave Chinner
2012-07-26 9:27 ` [PATCH 01/18] xfstests: remove remake script Dave Chinner
2012-08-28 19:50 ` Christoph Hellwig
2012-07-26 9:27 ` [PATCH 02/18] xfstests: remove bench infrastructure Dave Chinner
2012-07-26 9:27 ` [PATCH 03/18] xfstests: kill useless test owner fields Dave Chinner
2012-08-28 19:51 ` Christoph Hellwig
2012-07-26 9:27 ` [PATCH 04/18] xfstests: remove stale machine configs Dave Chinner
2012-08-28 19:51 ` Christoph Hellwig
2012-07-26 9:27 ` [PATCH 05/18] xfstests: fold common into check Dave Chinner
2012-08-28 19:52 ` Christoph Hellwig
2012-07-26 9:28 ` [PATCH 06/18] xfstests: clean up and simply check CLI option parsing Dave Chinner
2012-08-28 19:52 ` Christoph Hellwig
2012-07-26 9:28 ` [PATCH 07/18] xfstests: kill hangcheck stuff from check Dave Chinner
2012-08-28 19:53 ` Christoph Hellwig
2012-07-26 9:28 ` [PATCH 08/18] xfstests: remove test expunge file support Dave Chinner
2012-08-28 19:54 ` Christoph Hellwig
2012-07-26 9:28 ` [PATCH 09/18] xfstests: remove undocumented TESTS_REMAINING_LOG Dave Chinner
2012-08-28 19:54 ` Christoph Hellwig
2012-07-26 9:28 ` [PATCH 10/18] xfstests: include test subdirectory support Dave Chinner
2012-07-26 9:28 ` [PATCH 11/18] xfstests: move generic tests out of top level dir Dave Chinner
2012-07-26 9:28 ` [PATCH 12/18] xfstests: move xfs specific tests out of top directory Dave Chinner
2012-07-26 9:28 ` [PATCH 13/18] xfstests: move remaining tests out of top level directory Dave Chinner
2012-07-26 9:28 ` [PATCH 14/18] xfstests: rework CLI individual test specification Dave Chinner
2012-07-26 9:28 ` [PATCH 15/18] xfstests: make exclude groups aware of multiple subdirectories Dave Chinner
2012-07-26 9:28 ` [PATCH 16/18] xfstests: Introduce a results directory Dave Chinner
2012-07-26 9:28 ` [PATCH 17/18] xfstests: convert tests to use new " Dave Chinner
2012-09-05 12:00 ` Boris Ranto
2012-09-05 23:04 ` Dave Chinner
2012-09-06 12:34 ` Boris Ranto
2012-09-06 23:14 ` Dave Chinner
2012-09-07 12:47 ` Boris Ranto
2012-07-26 9:28 ` [PATCH 18/18] xfstests: fix _link_out_file callers Dave Chinner
2012-08-14 21:39 ` [RFC] [PATCH 0/18] xfstests: move tests out of top level Dave Chinner
2012-08-15 17:23 ` Mark Tinguely
2012-08-20 21:27 ` Mark Tinguely
2012-08-20 22:43 ` Dave Chinner
2012-08-21 16:33 ` Ben Myers
2012-08-21 22:09 ` Dave Chinner
2012-08-22 19:16 ` Ben Myers
2012-08-22 23:42 ` Dave Chinner
2012-08-23 17:00 ` Ben Myers
2012-08-24 4:07 ` Dave Chinner [this message]
2012-08-28 17:43 ` Ben Myers
2012-08-28 18:02 ` Christoph Hellwig
2013-02-25 15:50 ` Eric Sandeen
2013-02-25 21:52 ` Dave Chinner
2013-02-26 0:27 ` Theodore Ts'o
2013-02-26 3:18 ` Dave Chinner
2013-02-26 3:22 ` Ben Myers
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=20120824040724.GA19235@dastard \
--to=david@fromorbit.com \
--cc=bpm@sgi.com \
--cc=tinguely@sgi.com \
--cc=xfs@oss.sgi.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