public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Lukas Czerner <lczerner@redhat.com>
Cc: fstests@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: [PATCH 2/2] xfstests: Check fs consistency on TEST_DEV only when needed
Date: Thu, 10 Jul 2014 11:07:11 +1000	[thread overview]
Message-ID: <20140710010711.GJ4453@dastard> (raw)
In-Reply-To: <1403617014-17870-2-git-send-email-lczerner@redhat.com>

On Tue, Jun 24, 2014 at 03:36:54PM +0200, Lukas Czerner wrote:
> Currently we're checking file system consistency on TEST_DEV after every
> successful test run even though the TEST_DEV might not even be used in
> that test.

It gets mounted/unmounted for each test, so even if it is not used,
it might still get modified...

> Fix it by checking for number of write io on TEST_DEV device before the
> test and compare it with the value after the test.

Doesn't work for ram disks:

# ./setup 
TEST: DIR=/mnt/test DEV=/dev/ram0 rt=[] log=[]
TAPE: dev=[] rmt=[] rmtirix=[@]
SCRATCH: MNT=/mnt/scr DEV=/dev/ram1 rt=[] log=[]
VARIABLES: external=no largeblk=no fstyp=xfs
           large_scratch_dev=no attrsecure=no
# ls -l /dev/ram0
brw-rw---- 1 root disk 1, 0 Jul  9 15:51 /dev/ram0
# cat /sys/dev/block/1\:0/stat
       0        0        0        0        0        0        0        0        0        0        0
#

Nor does it work on loop devices.

> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
>  check     |  6 +++++-
>  common/rc | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/check b/check
> index 40f99d7..66d0f72 100755
> --- a/check
> +++ b/check
> @@ -544,6 +544,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
>  		rm -f core $seqres.notrun
>  
>  		scratch_atime=`stat -c %X $SCRATCH_DEV`
> +		test_write_io=`_get_write_io_count $TEST_DEV`
>  		start=`_wallclock`
>  		$timestamp && echo -n "	["`date "+%T"`"]"
>  		[ ! -x $seq ] && chmod u+x $seq # ensure we can run it
> @@ -626,7 +627,10 @@ for section in $HOST_OPTIONS_SECTIONS; do
>  	    then
>  		try="$try $seqnum"
>  		n_try=`expr $n_try + 1`
> -		_check_test_fs
> +		write_io=`_get_write_io_count $TEST_DEV`
> +		if [ -z $test_write_io ] || [ $write_io -ne $test_write_io ]; then
> +			_check_test_fs
> +		fi

Which you work around without mentioning the reason for the zero
value check. ;)

Personally, I want the test devices on my systems checked regardless
of whether the test tries to modify them or not. I, quite frankly,
don't trust the block device stats because they often get broken or
are inaccurate. Indeed, there were recent virtio block device
changes in 3.14-rc1 (or was it 3.15?) that broke the device stats
and that took several weeks to get fixed properly.

Hence I don't think using the block device stats for something as
critical as determining whether we should check a filesystem or not.

Reducing runtime of the test harness is a good idea, but we should
not do that if the cost is that it reduces the reliability of error
detection.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-07-10  1:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-24 13:36 [PATCH 1/2] xfstests: Check the file system consistency on SCRATCH_DEV Lukas Czerner
2014-06-24 13:36 ` [PATCH 2/2] xfstests: Check fs consistency on TEST_DEV only when needed Lukas Czerner
2014-07-10  1:07   ` Dave Chinner [this message]
2014-07-10  1:52     ` Eric Sandeen
2014-07-10  8:38     ` Lukáš Czerner
2014-07-10 20:50       ` Dave Chinner

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=20140710010711.GJ4453@dastard \
    --to=david@fromorbit.com \
    --cc=fstests@vger.kernel.org \
    --cc=lczerner@redhat.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