[Top] [All Lists]

Re: [PATCH] xfstests 016: Do not discard blocks at the mkfs time

To: Boris Ranto <branto@xxxxxxxxxx>
Subject: Re: [PATCH] xfstests 016: Do not discard blocks at the mkfs time
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Thu, 27 Oct 2011 12:04:34 -0400
Cc: xfs <xfs@xxxxxxxxxxx>, Eric Sandeen <esandeen@xxxxxxxxxx>
In-reply-to: <1319625983.3074.28.camel@xxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <1319625983.3074.28.camel@xxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Oct 26, 2011 at 12:46:23PM +0200, Boris Ranto wrote:
> The test 016 fills scratch device with some data and then creates xfs fs
> on the scratch device. Later, the test assumes that the previously
> written data are still in there and checks for them at specific
> locations. On ssd drive this will lead to failure since the blocks are
> discarded by default when the mkfs command is run.
> This simple patch that adds -K to stop the discarding (if the mkfs
> command supports it) fixed the issue for me:
> Signed-off-by: Boris Ranto <branto@xxxxxxxxxx>
> diff --git a/016 b/016
> index 9275ade..db76398 100755
> --- a/016
> +++ b/016
> @@ -65,6 +65,8 @@ _init()
>      $here/src/devzero -b 2048 -n 50 -v 198 $SCRATCH_DEV
>      echo "*** mkfs"
>      force_opts="-dsize=50m -lsize=$log_size"
> +    # Do not discard blocks, we need them for further reads
> +    _scratch_mkfs_xfs -N -K $force_opts >/dev/null 2>&1 &&
> force_opts="-K $force_opts"
>      echo mkfs_xfs $force_opts $SCRATCH_DEV >>$seq.full
>      _scratch_mkfs_xfs $force_opts >$tmp.mkfs0 2>&1

It took me very long understanding why you do mkfs.xfs calls here,
but I suspect now that it is to detect if -K is supported?

If so please document it in a comment, and maybe also write the
code a bit more verbose, e.g.

        # Do not discard blocks as we check for patterns in freespace.
        # Given that older xfsprogs versions do not have the -K option
        # make sure it works first.
        if _scratch_mkfs_xfs -N -K $force_opts >/dev/null 2>&1; then
            force_opts="-K $force_opts"

Otherwise the test looks good and will fix the 016 failure on TP / TRIM
capable devices that I've been seeing for a while.

Thanks a lot for doing this!

<Prev in Thread] Current Thread [Next in Thread>