linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Tim.Bird@sony.com
Cc: skhan@linuxfoundation.org, brendanhiggins@google.com,
	yzaikin@google.com, linux-kselftest@vger.kernel.org,
	linux-ext4@vger.kernel.org, adilger.kernel@dilger.ca,
	kunit-dev@googlegroups.com
Subject: Re: [PATCH linux-kselftest/test v2] ext4: add kunit test for decoding extended timestamps
Date: Thu, 17 Oct 2019 21:40:27 -0400	[thread overview]
Message-ID: <20191018014027.GA21137@mit.edu> (raw)
In-Reply-To: <ECADFF3FD767C149AD96A924E7EA6EAF977D00A4@USCULXMSG01.am.sony.com>

On Thu, Oct 17, 2019 at 11:40:01PM +0000, Tim.Bird@sony.com wrote:
> 
> No. Well, the data might be provided at some time independent
> of the test compilation time, but it would not be made up on the fly.
> So the data might be provided at run time, but that shouldn't imply
> that the data is random, or that there is some lengthy fabrication
> process that happens at test execution time.

So how would the data be provided?  Via a mounted file system?  There
is no mounted file system when we're running a kunit test.  One of the
reasons why kunit is fast is because we're not running init scripts,
and we're not mounting a file system.

The fabrication process isn't really lengthy, though.  If I modify
fs/ext4/inode-test.c to add or remove a test, it takes:

Elapsed time: 2.672s total, 0.001s configuring, 2.554s building, 0.116s running

Compare and contrast this with running "kvm-xfstests -c 4k generic/001"

The actual time to run the test generic/001 is 3 seconds.  But there
is a 9 second overhead in starting the VM, for a total test time of 12
seconds.  So sure, with kvm-xfstests I can drop a config file in
/tmp/kvm-xfstests-tytso, which is mounted as /vtmp using 9p, so you
could provide "user provided data" via a text file.  But the overhead
of starting up a full KVM, mounting a file system, starting userspace,
etc., is 9 seconds.

Contrast this with 2.5 seconds to recompile and relink
fs/ext4/inode-test.c into the kunit library.  I wouldn't call that a
"length fabrication process".  Is it really worth it to add in some
super-complex way to feed a data text file into a Kunit test, when
editing the test file and rerunning the test really doesn't take that
long?

> In this case, the cost of parsing the data file does add some overhead,
> but it's not onerous.  I'm not sure how, or whether, kunit handles
> the issue of reading data from a file at test time.  But it doesn't have
> to be a file read.  I'm just talking separating data from code.

It's not the cost of parsing the data file is how to *feed* the data
file into the test file.  How exactly are we supposed to do it?  9p?
Some other mounted file system?  That's where all the complexity and
overhead is going to be.

> Not necessarily.  Maybe the root privilege example is not a good one.
> How about a test that probes the kernel config, and executes
> some variation of the tests based on the config values it detects?

But that's even easier.  We can put "#ifdef CONFIG_xxx" into the
fs/ext4/inode-test.c file.  Again, it doesn't take that long to
recompile and relink the test .c file.

Apologies, but this really seems like complexity in search of a
problem....

						- Ted

  reply	other threads:[~2019-10-18  1:40 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10  2:39 [PATCH linux-kselftest/test v2] ext4: add kunit test for decoding extended timestamps Iurii Zaikin
2019-10-10  3:46 ` Tim.Bird
2019-10-10 16:45   ` Iurii Zaikin
2019-10-10 20:29     ` Tim.Bird
2019-10-10 23:49       ` Iurii Zaikin
2019-10-10 17:11 ` Shuah Khan
2019-10-10 22:13   ` Iurii Zaikin
2019-10-11 10:05     ` Brendan Higgins
2019-10-11 13:19       ` Theodore Y. Ts'o
2019-10-12  2:38         ` Iurii Zaikin
2019-10-16 22:18         ` Brendan Higgins
2019-10-16 23:26           ` Shuah Khan
2019-10-17  0:07             ` Iurii Zaikin
2019-10-17 12:08             ` Theodore Y. Ts'o
2019-10-17 22:25               ` Tim.Bird
2019-10-17 22:56                 ` Theodore Y. Ts'o
2019-10-17 23:40                   ` Tim.Bird
2019-10-18  1:40                     ` Theodore Y. Ts'o [this message]
2019-10-18  2:40                       ` Tim.Bird
2019-10-18 15:27                         ` Theodore Y. Ts'o
2019-10-18 20:24                           ` Shuah Khan
2019-10-24  1:30                             ` Brendan Higgins
2019-10-18  1:12                 ` Brendan Higgins
2019-10-18  1:30                   ` Tim.Bird
2019-10-17 22:49               ` Shuah Khan
2019-10-17 23:07                 ` Iurii Zaikin
2019-10-17 23:12                   ` Shuah Khan
2019-10-17 23:27                     ` Iurii Zaikin
2019-10-17 23:42                       ` Shuah Khan
2019-10-17 23:54                       ` Tim.Bird
2019-10-17 23:59                         ` Shuah Khan
2019-10-18  0:11                         ` Iurii Zaikin
2019-10-18  0:38                           ` Tim.Bird
2019-10-18  1:06                             ` Iurii Zaikin

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=20191018014027.GA21137@mit.edu \
    --to=tytso@mit.edu \
    --cc=Tim.Bird@sony.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=brendanhiggins@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=yzaikin@google.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;
as well as URLs for NNTP newsgroup(s).