Re: [PATCH v2] generic: concurrent IO test with mixed IO types

["Lukáš Czerner" <lczerner@redhat.com>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4121 bytes --]

On Wed, 10 Jun 2015, Dave Chinner wrote:

> Date: Wed, 10 Jun 2015 23:59:28 +1000
> From: Dave Chinner <david@fromorbit.com>
> To: Lukáš Czerner <lczerner@redhat.com>
> Cc: Eryu Guan <eguan@redhat.com>, fstests@vger.kernel.org
> Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> 
> On Wed, Jun 10, 2015 at 02:22:55PM +0200, Lukáš Czerner wrote:
> > On Wed, 10 Jun 2015, Dave Chinner wrote:
> > 
> > > Date: Wed, 10 Jun 2015 21:11:53 +1000
> > > From: Dave Chinner <david@fromorbit.com>
> > > To: Lukáš Czerner <lczerner@redhat.com>
> > > Cc: Eryu Guan <eguan@redhat.com>, fstests@vger.kernel.org
> > > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > > 
> > > On Wed, Jun 10, 2015 at 11:01:57AM +0200, Lukáš Czerner wrote:
> > > > On Wed, 10 Jun 2015, Dave Chinner wrote:
> > > > 
> > > > > Date: Wed, 10 Jun 2015 08:29:33 +1000
> > > > > From: Dave Chinner <david@fromorbit.com>
> > > > > To: Eryu Guan <eguan@redhat.com>
> > > > > Cc: fstests@vger.kernel.org, lczerner@redhat.com
> > > > > Subject: Re: [PATCH v2] generic: concurrent IO test with mixed IO types
> > > > > 
> > > > > On Mon, Jun 08, 2015 at 08:41:11PM +0800, Eryu Guan wrote:
> > > > > > Test concurrent buffered I/O, DIO, AIO, mmap I/O and splice I/O on the
> > > > > > same files.
> > > > > > 
> > > > > > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > > > > > ---
> > > > > > 
> > > > > > This fio job file has been proven to be potent, it triggers WARNINGs on ext4
> > > > > > and xfs with 4.1-rc6 kernel.
> > > > > > 
> > > > > > ext4: WARNING: at fs/ext4/inode.c:1328
> > > > > > xfs: WARNING: CPU: 7 PID: 3090 at fs/xfs/xfs_file.c:726 xfs_file_dio_aio_write+0x176/0x2a8 [xfs]()
> > > > > > 
> > > > > > The ext4 issue should be fixed by Lukas's patch
> > > > > > ext4: fix reservation release on invalidatepage for delalloc fs
> > > > > > 
> > > > > > And it ever paniced kernel in mm code and hung xfs.
> > > > > > 
> > > > > > I reduced the numjobs and iodepth to reduce the test time(~25s on my test host)
> > > > > > and scale them by $LOAD_FACTOR. And it still could trigger the warning on ext4
> > > > > > and xfs with reduced workload.
> > > > > > 
> > > > > > v2:
> > > > > > - use mktemp to create tmp fio job file
> > > > > ....
> > > > > > +seq=`basename $0`
> > > > > > +seqres=$RESULT_DIR/$seq
> > > > > > +echo "QA output created by $seq"
> > > > > > +
> > > > > > +here=`pwd`
> > > > > > +fio_config=`mktemp`
> > > > > > +status=1	# failure is the default!
> > > > > > +trap "_cleanup; exit \$status" 0 1 2 3 15
> > > > > 
> > > > > By removing the definition of $tmp, you are now dumping all
> > > > > the temporary files the test harnes creates in /.
> > > > 
> > > > What temp files ? Yes we're sometimes using $tmp even though there
> > > > is no obvious definition and if we want to rely on the existence of
> > > > this variable we better define it as environment variable in 'check'
> > > > script.
> > > 
> > > About 80% of the files in the repository use $tmp in some way.
> > > And it's used all over the place in common/*, too. e.g mkfs
> > > and check functions for storing output for parsing....
> > 
> > Ah, come on. There are not that many functions using the $tmp and
> > some of them even removes the file immediately.
> 
> $ git grep -lw "^tmp=" |wc -l
> 484
> $ git grep -lw "\$tmp" |wc -l
> 417
> 
> That's roughly 80% of files that use $tmp /in some way/. The tests
> are all suppose dto define $tmp the same way, and they are all
> expected to clean up after themselves, thereby making it possible to
> safely use $tmp in linrary functions, whether they clean up after
> tehmselves or not.
> 
> You're spending way more time and effort than is necessary on basic
> infrastructure that has worked for 15 years and, quite frankly,
> *doesn't need fixing because it's not broken*.

Strongly disagree and patch like that just proves that it is
broken. But frankly you're right in that I am spending too much time
and effort arguing with you.

Regards,
-Lukas