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

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

[Boris Ranto]

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@redhat.com>

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
     [ $? -ne 0 ] && \

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

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

[Christoph Hellwig]

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@redhat.com>
> 
> 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!

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

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

[Boris Ranto]

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@redhat.com>
> > 
> > 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.

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

[PATCH] xfstests 016: Do not discard blocks at the mkfs time, more verbose version

[Boris Ranto]

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 written there and checks for them at specific
locations. On ssd drive this will lead to a failure since the blocks are
discarded by default when the mkfs command is run.
This is a more verbose version of the previous patch.
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@redhat.com>

diff --git a/016 b/016
index 9275ade..08a73f4 100755
--- a/016
+++ b/016
@@ -65,6 +65,15 @@ _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 as we check for patterns in free space.
+    # 
+    # First, make sure that mkfs supports '-K' option by using its
+    # dry run (-N option) and then add it to the force_opts.
+    #
+    if _scratch_mkfs_xfs -N -K $force_opts >/dev/null 2>&1; then
+        force_opts="-K $force_opts"
+    fi
     echo mkfs_xfs $force_opts $SCRATCH_DEV >>$seq.full
     _scratch_mkfs_xfs $force_opts >$tmp.mkfs0 2>&1
     [ $? -ne 0 ] && \

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

Re: [PATCH] xfstests 016: Do not discard blocks at the mkfs time, more verbose version

[Christoph Hellwig]

On Fri, Oct 28, 2011 at 05:38:06AM -0400, 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 written there and checks for them at specific
> locations. On ssd drive this will lead to a failure since the blocks are
> discarded by default when the mkfs command is run.
> This is a more verbose version of the previous patch.
> 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@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

Re: [PATCH] xfstests 016: Do not discard blocks at the mkfs time, more verbose version

[Eric Sandeen]

On 10/28/11 4:53 AM, Christoph Hellwig wrote:
> On Fri, Oct 28, 2011 at 05:38:06AM -0400, 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 written there and checks for them at specific
>> locations. On ssd drive this will lead to a failure since the blocks are
>> discarded by default when the mkfs command is run.
>> This is a more verbose version of the previous patch.
>> 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@redhat.com>
> 
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Seems this one got lost; we should still merge it, yes?

-Eric

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

Re: [PATCH] xfstests 016: Do not discard blocks at the mkfs time, more verbose version

[Christoph Hellwig]

On Tue, May 29, 2012 at 11:54:31AM -0500, Eric Sandeen wrote:
> Seems this one got lost; we should still merge it, yes?

Please do.

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