xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests 016: Do not discard blocks at the mkfs time
From: Boris Ranto <branto@xxxxxxxxxx>
Date: Fri, 28 Oct 2011 05:25:16 -0400 (EDT)
Cc: Eric Sandeen <esandeen@xxxxxxxxxx>, xfs <xfs@xxxxxxxxxxx>
In-reply-to: <20111027160434.GA21551@xxxxxxxxxxxxx>
On Thu, Oct 27, 2011, Christoph Hellwig wrote:
> 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"
>       fi
> 
> 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!
> 

Yes, the dry mkfs.xfs call is there just to check whether mkfs supports the 
'-K' option.
I'll resend the patch in its more verbose form to make it more clear.

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