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 <[email protected]> > > To: Lukáš Czerner <[email protected]> > > Cc: Eryu Guan <[email protected]>, [email protected] > > 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 <[email protected]> > > > > To: Eryu Guan <[email protected]> > > > > Cc: [email protected], [email protected] > > > > 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 <[email protected]> > > > > > --- > > > > > > > > > > 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*. Cheers, Dave. -- Dave Chinner [email protected] -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
