[PATCH] xfstests: make 275 pass on ext4

[PATCH] xfstests: make 275 pass on ext4

[Eric Sandeen]

275 was failing on ext4 because it doesn't reliably write
until ENOSPC due to delalloc and crummy ENOSPC handling.
So the attempts to fill the fs would fail, and this test,
which was expecting to have exactly 4k free for the last write
attempt, failed as well, because it was able to write more than
the expected 4k.

Change a few things:

* Tidy up test description & comments
* Keep files on scratch mount for failure analysis
* Add a couple syncs here and there to push out delalloc
* Make a last-ditch effort to fill fs via direct IO
* Provide a little more detail on failure

With this change I can pass the test on ext4.

Fixing ext4's ENOSPC handling is probably another bug, but by
working really hard to fill an ext4 fs, this test can achieve
its specific goal.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/275 b/275
index 214262e..3ebeabe 100755
--- a/275
+++ b/275
@@ -1,8 +1,8 @@
 #! /bin/bash
 # FS QA Test No. 275
 #
-# The posix write test. when write size is larger than disk free size,
-# should write as more as possible
+# The posix write test.  When write size is larger than disk free size,
+# should write as much as possible until ENOSPC.
 #
 #-----------------------------------------------------------------------
 # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
@@ -36,7 +36,6 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _cleanup()
 {
 	cd /
-	rm -f $SCRATCH_MNT/* $tmp.*
 	_scratch_unmount
 }
 
@@ -49,7 +48,7 @@ _supported_os IRIX Linux
 _require_scratch
 
 echo "------------------------------"
-echo "write lack test"
+echo "write until ENOSPC test"
 echo "------------------------------"
 
 rm -f $seq.full
@@ -69,10 +68,15 @@ then
 	exit
 fi
 
+# Attempt to completely fill fs
 dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
+sync
 dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
 sync
+# Last effort, use O_DIRECT to defeat delalloc
+dd if=/dev/zero of=tmp4 bs=4K oflag=direct >/dev/null 2>&1
 
+# Should leave exactly 4k free
 rm -f tmp1
 sync
 
@@ -80,7 +84,7 @@ dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
 _filesize=`du tmp1 | awk '{print $1}'`
 if [ $_filesize -ne 4 ]
 then
-	echo "write file err"
+	echo "write file err: wrote $_filesize kbytes not 4 kbytes"
 	status=1
 	exit
 fi
diff --git a/275.out b/275.out
index 30af43c..69b9d52 100644
--- a/275.out
+++ b/275.out
@@ -1,5 +1,5 @@
 QA output created by 275
 ------------------------------
-write lack test
+write until ENOSPC test
 ------------------------------
 done

Re: [PATCH] xfstests: make 275 pass on ext4

[Eric Sandeen]

On 1/4/12 1:22 PM, Eric Sandeen wrote:
> 275 was failing on ext4 because it doesn't reliably write
> until ENOSPC due to delalloc and crummy ENOSPC handling.
> So the attempts to fill the fs would fail, and this test,
> which was expecting to have exactly 4k free for the last write
> attempt, failed as well, because it was able to write more than
> the expected 4k.

Grr self-NAK this makes it fail on xfs (?!)

-Eric

> Change a few things:
> 
> * Tidy up test description & comments
> * Keep files on scratch mount for failure analysis
> * Add a couple syncs here and there to push out delalloc
> * Make a last-ditch effort to fill fs via direct IO
> * Provide a little more detail on failure
> 
> With this change I can pass the test on ext4.
> 
> Fixing ext4's ENOSPC handling is probably another bug, but by
> working really hard to fill an ext4 fs, this test can achieve
> its specific goal.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/275 b/275
> index 214262e..3ebeabe 100755
> --- a/275
> +++ b/275
> @@ -1,8 +1,8 @@
>  #! /bin/bash
>  # FS QA Test No. 275
>  #
> -# The posix write test. when write size is larger than disk free size,
> -# should write as more as possible
> +# The posix write test.  When write size is larger than disk free size,
> +# should write as much as possible until ENOSPC.
>  #
>  #-----------------------------------------------------------------------
>  # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
> @@ -36,7 +36,6 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>  _cleanup()
>  {
>  	cd /
> -	rm -f $SCRATCH_MNT/* $tmp.*
>  	_scratch_unmount
>  }
>  
> @@ -49,7 +48,7 @@ _supported_os IRIX Linux
>  _require_scratch
>  
>  echo "------------------------------"
> -echo "write lack test"
> +echo "write until ENOSPC test"
>  echo "------------------------------"
>  
>  rm -f $seq.full
> @@ -69,10 +68,15 @@ then
>  	exit
>  fi
>  
> +# Attempt to completely fill fs
>  dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
> +sync
>  dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
>  sync
> +# Last effort, use O_DIRECT to defeat delalloc
> +dd if=/dev/zero of=tmp4 bs=4K oflag=direct >/dev/null 2>&1
>  
> +# Should leave exactly 4k free
>  rm -f tmp1
>  sync
>  
> @@ -80,7 +84,7 @@ dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
>  _filesize=`du tmp1 | awk '{print $1}'`
>  if [ $_filesize -ne 4 ]
>  then
> -	echo "write file err"
> +	echo "write file err: wrote $_filesize kbytes not 4 kbytes"
>  	status=1
>  	exit
>  fi
> diff --git a/275.out b/275.out
> index 30af43c..69b9d52 100644
> --- a/275.out
> +++ b/275.out
> @@ -1,5 +1,5 @@
>  QA output created by 275
>  ------------------------------
> -write lack test
> +write until ENOSPC test
>  ------------------------------
>  done
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2] xfstests: make 275 pass

[Eric Sandeen]

Ok, this is a significant rework of 275, which made too many
assumptions about details of space usage and failed on several
filesystems (it passed on xfs, but only by accident).

This new version tries to leave about 256k free, then tries
a single 1M IO, and fails only if 0 bytes are written.

It also sends a lot more to $seq.full for debugging on failure
and fixes a few other stylistic things.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/275 b/275
index 214262e..8f521f8 100755
--- a/275
+++ b/275
@@ -1,8 +1,8 @@
 #! /bin/bash
 # FS QA Test No. 275
 #
-# The posix write test. when write size is larger than disk free size,
-# should write as more as possible
+# The posix write test.  When write size is larger than disk free size,
+# should write as much as possible until ENOSPC.
 #
 #-----------------------------------------------------------------------
 # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
@@ -30,13 +30,12 @@ echo "QA output created by $seq"
 
 here=`pwd`
 tmp=/tmp/$$
-status=0    # success is the default!
+status=1    # failure is the default!
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
 	cd /
-	rm -f $SCRATCH_MNT/* $tmp.*
 	_scratch_unmount
 }
 
@@ -49,7 +48,7 @@ _supported_os IRIX Linux
 _require_scratch
 
 echo "------------------------------"
-echo "write lack test"
+echo "write until ENOSPC test"
 echo "------------------------------"
 
 rm -f $seq.full
@@ -59,31 +58,38 @@ _scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
 _scratch_mount
 
 rm -rf $SCRATCH_MNT/*
-cd $SCRATCH_MNT
 
-dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
-if [ $? -ne 0 ]
-then
-	echo "create file err"
-	status=1
-	exit
-fi
+dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=256K count=1 >>$seq.full 2>&1
+[ $? -ne 0 ] && _fail "Error creating file"
 
-dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
-dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
+# Attempt to completely fill fs
+dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=1M >>$seq.full 2>&1
 sync
+dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K >>$seq.full 2>&1
+sync
+# Last effort, use O_SYNC
+dd if=/dev/zero of=$SCRATCH_MNT/tmp4 bs=4K oflag=sync >>$seq.full 2>&1
+# Save space usage info to the full file
+echo "Pre rm space:" >> $seq.full
+df $SCRATCH_MNT >>$seq.full 2>&1
 
-rm -f tmp1
+# Should leave approx 256k free
+rm -f $SCRATCH_MNT/tmp1
 sync
+echo "Post rm space:" >> $seq.full
+df $SCRATCH_MNT >>$seq.full 2>&1
+_freespace=`df -k $SCRATCH_MNT | tail -n 1 | awk '{print $4}'`
+[ $_freespace -gt 1024 ] && _fail "could not sufficiently fill filesystem"
+
+# Try a write larger than available space
+dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M count=1 >>$seq.full 2>&1
+echo "Bytes written until ENOSPC:" >>$seq.full
+du $SCRATCH_MNT/tmp1 >>$seq.full
 
-dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
-_filesize=`du tmp1 | awk '{print $1}'`
-if [ $_filesize -ne 4 ]
-then
-	echo "write file err"
-	status=1
-	exit
-fi
+# And at least some of it should succeed.
+_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
+[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
 
 echo "done"
+status=0
 exit
diff --git a/275.out b/275.out
index 30af43c..69b9d52 100644
--- a/275.out
+++ b/275.out
@@ -1,5 +1,5 @@
 QA output created by 275
 ------------------------------
-write lack test
+write until ENOSPC test
 ------------------------------
 done

Re: [PATCH V2] xfstests: make 275 pass

[Dave Chinner]

On Wed, Jan 04, 2012 at 02:54:25PM -0600, Eric Sandeen wrote:
> Ok, this is a significant rework of 275, which made too many
> assumptions about details of space usage and failed on several
> filesystems (it passed on xfs, but only by accident).
> 
> This new version tries to leave about 256k free, then tries
> a single 1M IO, and fails only if 0 bytes are written.
> 
> It also sends a lot more to $seq.full for debugging on failure
> and fixes a few other stylistic things.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

I just had another thought about this, Eric....

> +# And at least some of it should succeed.
> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."

The question that just came to mind was this assumes that allocation
succeeded so therefore the partial write succeeded. But that's not
necessary the case. The partial write might not succeed leaving the
file size as zero, but the underlying FS might not remove all the
blocks it allocated (nothing says that it has to). Hence to
determine if a partial write succeeded, we also need to check that
the file size itself is greater than zero....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2] xfstests: make 275 pass

[Eric Sandeen]

On 1/4/12 5:17 PM, Dave Chinner wrote:
> On Wed, Jan 04, 2012 at 02:54:25PM -0600, Eric Sandeen wrote:
>> Ok, this is a significant rework of 275, which made too many
>> assumptions about details of space usage and failed on several
>> filesystems (it passed on xfs, but only by accident).
>>
>> This new version tries to leave about 256k free, then tries
>> a single 1M IO, and fails only if 0 bytes are written.
>>
>> It also sends a lot more to $seq.full for debugging on failure
>> and fixes a few other stylistic things.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> 
> I just had another thought about this, Eric....
> 
>> +# And at least some of it should succeed.
>> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
>> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
> 
> The question that just came to mind was this assumes that allocation
> succeeded so therefore the partial write succeeded. But that's not
> necessary the case. The partial write might not succeed leaving the
> file size as zero, but the underlying FS might not remove all the
> blocks it allocated (nothing says that it has to). Hence to
> determine if a partial write succeeded, we also need to check that
> the file size itself is greater than zero....

Probably need to read up on what posix says it should do.  I think
what you're saying is that it might leave blocks allocated past EOF?
That'd be surprising to me, but maybe I misunderstand?

Anyway, testing file size as well as space is simple enough.

-Eric

> Cheers,
> 
> Dave.

Re: [PATCH V2] xfstests: make 275 pass

[Dave Chinner]

On Wed, Jan 04, 2012 at 05:21:00PM -0600, Eric Sandeen wrote:
> On 1/4/12 5:17 PM, Dave Chinner wrote:
> > On Wed, Jan 04, 2012 at 02:54:25PM -0600, Eric Sandeen wrote:
> >> Ok, this is a significant rework of 275, which made too many
> >> assumptions about details of space usage and failed on several
> >> filesystems (it passed on xfs, but only by accident).
> >>
> >> This new version tries to leave about 256k free, then tries
> >> a single 1M IO, and fails only if 0 bytes are written.
> >>
> >> It also sends a lot more to $seq.full for debugging on failure
> >> and fixes a few other stylistic things.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > 
> > I just had another thought about this, Eric....
> > 
> >> +# And at least some of it should succeed.
> >> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
> >> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
> > 
> > The question that just came to mind was this assumes that allocation
> > succeeded so therefore the partial write succeeded. But that's not
> > necessary the case. The partial write might not succeed leaving the
> > file size as zero, but the underlying FS might not remove all the
> > blocks it allocated (nothing says that it has to). Hence to
> > determine if a partial write succeeded, we also need to check that
> > the file size itself is greater than zero....
> 
> Probably need to read up on what posix says it should do.  I think
> what you're saying is that it might leave blocks allocated past EOF?
> That'd be surprising to me, but maybe I misunderstand?

There's no guarantee that du is even reporting blocks on disk. e.g
for XFS du will also report reserved (in-memory) delalloc space on
the inode and that includes speculative allocation beyond EOF. We
don't have to remove specultive delalloc ranges when a partial write
occurs, so effectively checking du output to see if a partial write
succeeded is not a sufficient test to determine if the partial write
succeeded or not.

However, if the partial write did succeed then the file size *must*
change to reflect what was written. Hence I suspect all we actually
need here is a file size check...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH V2] xfstests: make 275 pass

[Eric Sandeen]

On 1/4/12 6:39 PM, Dave Chinner wrote:
> On Wed, Jan 04, 2012 at 05:21:00PM -0600, Eric Sandeen wrote:
>> On 1/4/12 5:17 PM, Dave Chinner wrote:
>>> On Wed, Jan 04, 2012 at 02:54:25PM -0600, Eric Sandeen wrote:
>>>> Ok, this is a significant rework of 275, which made too many
>>>> assumptions about details of space usage and failed on several
>>>> filesystems (it passed on xfs, but only by accident).
>>>>
>>>> This new version tries to leave about 256k free, then tries
>>>> a single 1M IO, and fails only if 0 bytes are written.
>>>>
>>>> It also sends a lot more to $seq.full for debugging on failure
>>>> and fixes a few other stylistic things.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>
>>> I just had another thought about this, Eric....
>>>
>>>> +# And at least some of it should succeed.
>>>> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
>>>> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>>>
>>> The question that just came to mind was this assumes that allocation
>>> succeeded so therefore the partial write succeeded. But that's not
>>> necessary the case. The partial write might not succeed leaving the
>>> file size as zero, but the underlying FS might not remove all the
>>> blocks it allocated (nothing says that it has to). Hence to
>>> determine if a partial write succeeded, we also need to check that
>>> the file size itself is greater than zero....
>>
>> Probably need to read up on what posix says it should do.  I think
>> what you're saying is that it might leave blocks allocated past EOF?
>> That'd be surprising to me, but maybe I misunderstand?
> 
> There's no guarantee that du is even reporting blocks on disk. e.g
> for XFS du will also report reserved (in-memory) delalloc space on
> the inode and that includes speculative allocation beyond EOF. We
> don't have to remove specultive delalloc ranges when a partial write
> occurs, so effectively checking du output to see if a partial write
> succeeded is not a sufficient test to determine if the partial write
> succeeded or not.
> 
> However, if the partial write did succeed then the file size *must*
> change to reflect what was written. Hence I suspect all we actually
> need here is a file size check...

Ok; so would you be happy with just this?

# And at least some of it should succeed.
_filesize=`ls -l $SCRATCH_MNT/tmp1 | awk '{print $5}'`
[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."

If so I'll put your reviewed-by on it and get this pushed, ok?

-Eric

> Cheers,
> 
> Dave.

Re: [PATCH V2] xfstests: make 275 pass

[Liu Bo]

On 01/05/2012 04:54 AM, Eric Sandeen wrote:
> Ok, this is a significant rework of 275, which made too many
> assumptions about details of space usage and failed on several
> filesystems (it passed on xfs, but only by accident).
> 
> This new version tries to leave about 256k free, then tries
> a single 1M IO, and fails only if 0 bytes are written.
> 
> It also sends a lot more to $seq.full for debugging on failure
> and fixes a few other stylistic things.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/275 b/275
> index 214262e..8f521f8 100755
> --- a/275
> +++ b/275
> @@ -1,8 +1,8 @@
>  #! /bin/bash
>  # FS QA Test No. 275
>  #
> -# The posix write test. when write size is larger than disk free size,
> -# should write as more as possible
> +# The posix write test.  When write size is larger than disk free size,
> +# should write as much as possible until ENOSPC.
>  #
>  #-----------------------------------------------------------------------
>  # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
> @@ -30,13 +30,12 @@ echo "QA output created by $seq"
>  
>  here=`pwd`
>  tmp=/tmp/$$
> -status=0    # success is the default!
> +status=1    # failure is the default!
>  trap "_cleanup; exit \$status" 0 1 2 3 15
>  
>  _cleanup()
>  {
>  	cd /
> -	rm -f $SCRATCH_MNT/* $tmp.*
>  	_scratch_unmount
>  }
>  
> @@ -49,7 +48,7 @@ _supported_os IRIX Linux
>  _require_scratch
>  
>  echo "------------------------------"
> -echo "write lack test"
> +echo "write until ENOSPC test"
>  echo "------------------------------"
>  
>  rm -f $seq.full
> @@ -59,31 +58,38 @@ _scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>  _scratch_mount
>  
>  rm -rf $SCRATCH_MNT/*
> -cd $SCRATCH_MNT
>  
> -dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
> -if [ $? -ne 0 ]
> -then
> -	echo "create file err"
> -	status=1
> -	exit
> -fi
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=256K count=1 >>$seq.full 2>&1
> +[ $? -ne 0 ] && _fail "Error creating file"
>  
> -dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
> -dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
> +# Attempt to completely fill fs
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=1M >>$seq.full 2>&1
>  sync
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K >>$seq.full 2>&1
> +sync
> +# Last effort, use O_SYNC
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp4 bs=4K oflag=sync >>$seq.full 2>&1
> +# Save space usage info to the full file
> +echo "Pre rm space:" >> $seq.full
> +df $SCRATCH_MNT >>$seq.full 2>&1
>  
> -rm -f tmp1
> +# Should leave approx 256k free
> +rm -f $SCRATCH_MNT/tmp1
>  sync
> +echo "Post rm space:" >> $seq.full
> +df $SCRATCH_MNT >>$seq.full 2>&1
> +_freespace=`df -k $SCRATCH_MNT | tail -n 1 | awk '{print $4}'`
> +[ $_freespace -gt 1024 ] && _fail "could not sufficiently fill filesystem"
> +

I doubt this cause btrfs has a mixed data+meta mode, which means data and metadata can
share space, but this mode is only enabled when the filesystem size is less than 1G.

We can apply the below to avoid it:
-_scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
+_scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1


> +# Try a write larger than available space
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M count=1 >>$seq.full 2>&1
> +echo "Bytes written until ENOSPC:" >>$seq.full
> +du $SCRATCH_MNT/tmp1 >>$seq.full
>  
> -dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
> -_filesize=`du tmp1 | awk '{print $1}'`
> -if [ $_filesize -ne 4 ]
> -then
> -	echo "write file err"
> -	status=1
> -	exit
> -fi
> +# And at least some of it should succeed.
> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>  

And btrfs will check free space first and then decide if it should allocate blocks, so a partial write
will fail anyway.


Other parts looks good to me :)

thanks,
liubo

>  echo "done"
> +status=0
>  exit
> diff --git a/275.out b/275.out
> index 30af43c..69b9d52 100644
> --- a/275.out
> +++ b/275.out
> @@ -1,5 +1,5 @@
>  QA output created by 275
>  ------------------------------
> -write lack test
> +write until ENOSPC test
>  ------------------------------
>  done
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH V2] xfstests: make 275 pass

[Eric Sandeen]

On 1/17/12 8:42 PM, Liu Bo wrote:
> On 01/05/2012 04:54 AM, Eric Sandeen wrote:
>> Ok, this is a significant rework of 275, which made too many
>> assumptions about details of space usage and failed on several
>> filesystems (it passed on xfs, but only by accident).
>>
>> This new version tries to leave about 256k free, then tries
>> a single 1M IO, and fails only if 0 bytes are written.
>>
>> It also sends a lot more to $seq.full for debugging on failure
>> and fixes a few other stylistic things.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/275 b/275
>> index 214262e..8f521f8 100755
>> --- a/275
>> +++ b/275
>> @@ -1,8 +1,8 @@
>>  #! /bin/bash
>>  # FS QA Test No. 275
>>  #
>> -# The posix write test. when write size is larger than disk free size,
>> -# should write as more as possible
>> +# The posix write test.  When write size is larger than disk free size,
>> +# should write as much as possible until ENOSPC.
>>  #
>>  #-----------------------------------------------------------------------
>>  # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
>> @@ -30,13 +30,12 @@ echo "QA output created by $seq"
>>  
>>  here=`pwd`
>>  tmp=/tmp/$$
>> -status=0    # success is the default!
>> +status=1    # failure is the default!
>>  trap "_cleanup; exit \$status" 0 1 2 3 15
>>  
>>  _cleanup()
>>  {
>>  	cd /
>> -	rm -f $SCRATCH_MNT/* $tmp.*
>>  	_scratch_unmount
>>  }
>>  
>> @@ -49,7 +48,7 @@ _supported_os IRIX Linux
>>  _require_scratch
>>  
>>  echo "------------------------------"
>> -echo "write lack test"
>> +echo "write until ENOSPC test"
>>  echo "------------------------------"
>>  
>>  rm -f $seq.full
>> @@ -59,31 +58,38 @@ _scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>>  _scratch_mount
>>  
>>  rm -rf $SCRATCH_MNT/*
>> -cd $SCRATCH_MNT
>>  
>> -dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
>> -if [ $? -ne 0 ]
>> -then
>> -	echo "create file err"
>> -	status=1
>> -	exit
>> -fi
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=256K count=1 >>$seq.full 2>&1
>> +[ $? -ne 0 ] && _fail "Error creating file"
>>  
>> -dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
>> -dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
>> +# Attempt to completely fill fs
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=1M >>$seq.full 2>&1
>>  sync
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K >>$seq.full 2>&1
>> +sync
>> +# Last effort, use O_SYNC
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp4 bs=4K oflag=sync >>$seq.full 2>&1
>> +# Save space usage info to the full file
>> +echo "Pre rm space:" >> $seq.full
>> +df $SCRATCH_MNT >>$seq.full 2>&1
>>  
>> -rm -f tmp1
>> +# Should leave approx 256k free
>> +rm -f $SCRATCH_MNT/tmp1
>>  sync
>> +echo "Post rm space:" >> $seq.full
>> +df $SCRATCH_MNT >>$seq.full 2>&1
>> +_freespace=`df -k $SCRATCH_MNT | tail -n 1 | awk '{print $4}'`
>> +[ $_freespace -gt 1024 ] && _fail "could not sufficiently fill filesystem"
>> +
> 
> I doubt this cause btrfs has a mixed data+meta mode, which means data and metadata can
> share space, but this mode is only enabled when the filesystem size is less than 1G.
> 
> We can apply the below to avoid it:
> -_scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
> +_scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1

Ok, I can do that easily enough (assuming scratch space is likely to be at least 2G...)

> 
>> +# Try a write larger than available space
>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M count=1 >>$seq.full 2>&1
>> +echo "Bytes written until ENOSPC:" >>$seq.full
>> +du $SCRATCH_MNT/tmp1 >>$seq.full
>>  
>> -dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
>> -_filesize=`du tmp1 | awk '{print $1}'`
>> -if [ $_filesize -ne 4 ]
>> -then
>> -	echo "write file err"
>> -	status=1
>> -	exit
>> -fi
>> +# And at least some of it should succeed.
>> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
>> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>>  
> 
> And btrfs will check free space first and then decide if it should allocate blocks, so a partial write
> will fail anyway.

Ok, I'm not quite clear - will the above be a problem for btrfs or is it ok?

Thanks,
-Eric

> 
> Other parts looks good to me :)
> 
> thanks,
> liubo
> 
>>  echo "done"
>> +status=0
>>  exit
>> diff --git a/275.out b/275.out
>> index 30af43c..69b9d52 100644
>> --- a/275.out
>> +++ b/275.out
>> @@ -1,5 +1,5 @@
>>  QA output created by 275
>>  ------------------------------
>> -write lack test
>> +write until ENOSPC test
>>  ------------------------------
>>  done
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

Re: [PATCH V2] xfstests: make 275 pass

[Liu Bo]

On 01/18/2012 12:03 PM, Eric Sandeen wrote:
> On 1/17/12 8:42 PM, Liu Bo wrote:
>> On 01/05/2012 04:54 AM, Eric Sandeen wrote:
>>> Ok, this is a significant rework of 275, which made too many
>>> assumptions about details of space usage and failed on several
>>> filesystems (it passed on xfs, but only by accident).
>>>
>>> This new version tries to leave about 256k free, then tries
>>> a single 1M IO, and fails only if 0 bytes are written.
>>>
>>> It also sends a lot more to $seq.full for debugging on failure
>>> and fixes a few other stylistic things.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>
>>> diff --git a/275 b/275
>>> index 214262e..8f521f8 100755
>>> --- a/275
>>> +++ b/275
>>> @@ -1,8 +1,8 @@
>>>  #! /bin/bash
>>>  # FS QA Test No. 275
>>>  #
>>> -# The posix write test. when write size is larger than disk free size,
>>> -# should write as more as possible
>>> +# The posix write test.  When write size is larger than disk free size,
>>> +# should write as much as possible until ENOSPC.
>>>  #
>>>  #-----------------------------------------------------------------------
>>>  # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
>>> @@ -30,13 +30,12 @@ echo "QA output created by $seq"
>>>  
>>>  here=`pwd`
>>>  tmp=/tmp/$$
>>> -status=0    # success is the default!
>>> +status=1    # failure is the default!
>>>  trap "_cleanup; exit \$status" 0 1 2 3 15
>>>  
>>>  _cleanup()
>>>  {
>>>  	cd /
>>> -	rm -f $SCRATCH_MNT/* $tmp.*
>>>  	_scratch_unmount
>>>  }
>>>  
>>> @@ -49,7 +48,7 @@ _supported_os IRIX Linux
>>>  _require_scratch
>>>  
>>>  echo "------------------------------"
>>> -echo "write lack test"
>>> +echo "write until ENOSPC test"
>>>  echo "------------------------------"
>>>  
>>>  rm -f $seq.full
>>> @@ -59,31 +58,38 @@ _scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>>>  _scratch_mount
>>>  
>>>  rm -rf $SCRATCH_MNT/*
>>> -cd $SCRATCH_MNT
>>>  
>>> -dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
>>> -if [ $? -ne 0 ]
>>> -then
>>> -	echo "create file err"
>>> -	status=1
>>> -	exit
>>> -fi
>>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=256K count=1 >>$seq.full 2>&1
>>> +[ $? -ne 0 ] && _fail "Error creating file"
>>>  
>>> -dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
>>> -dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
>>> +# Attempt to completely fill fs
>>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=1M >>$seq.full 2>&1
>>>  sync
>>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K >>$seq.full 2>&1
>>> +sync
>>> +# Last effort, use O_SYNC
>>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp4 bs=4K oflag=sync >>$seq.full 2>&1
>>> +# Save space usage info to the full file
>>> +echo "Pre rm space:" >> $seq.full
>>> +df $SCRATCH_MNT >>$seq.full 2>&1
>>>  
>>> -rm -f tmp1
>>> +# Should leave approx 256k free
>>> +rm -f $SCRATCH_MNT/tmp1
>>>  sync
>>> +echo "Post rm space:" >> $seq.full
>>> +df $SCRATCH_MNT >>$seq.full 2>&1
>>> +_freespace=`df -k $SCRATCH_MNT | tail -n 1 | awk '{print $4}'`
>>> +[ $_freespace -gt 1024 ] && _fail "could not sufficiently fill filesystem"
>>> +
>> I doubt this cause btrfs has a mixed data+meta mode, which means data and metadata can
>> share space, but this mode is only enabled when the filesystem size is less than 1G.
>>
>> We can apply the below to avoid it:
>> -_scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>> +_scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
> 
> Ok, I can do that easily enough (assuming scratch space is likely to be at least 2G...)
> 
>>> +# Try a write larger than available space
>>> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M count=1 >>$seq.full 2>&1
>>> +echo "Bytes written until ENOSPC:" >>$seq.full
>>> +du $SCRATCH_MNT/tmp1 >>$seq.full
>>>  
>>> -dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
>>> -_filesize=`du tmp1 | awk '{print $1}'`
>>> -if [ $_filesize -ne 4 ]
>>> -then
>>> -	echo "write file err"
>>> -	status=1
>>> -	exit
>>> -fi
>>> +# And at least some of it should succeed.
>>> +_filesize=`du $SCRATCH_MNT/tmp1 | awk '{print $1}'`
>>> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>>>  
>> And btrfs will check free space first and then decide if it should allocate blocks, so a partial write
>> will fail anyway.
> 
> Ok, I'm not quite clear - will the above be a problem for btrfs or is it ok?
> 

It is ok since it depends on the underlying FS IMO.

thanks,
liubo

> Thanks,
> -Eric
> 
>> Other parts looks good to me :)
>>
>> thanks,
>> liubo
>>
>>>  echo "done"
>>> +status=0
>>>  exit
>>> diff --git a/275.out b/275.out
>>> index 30af43c..69b9d52 100644
>>> --- a/275.out
>>> +++ b/275.out
>>> @@ -1,5 +1,5 @@
>>>  QA output created by 275
>>>  ------------------------------
>>> -write lack test
>>> +write until ENOSPC test
>>>  ------------------------------
>>>  done
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH V3] xfstests: make 275 pass

[Eric Sandeen]

Ok, this is a significant rework of 275, which made too many
assumptions about details of space usage and failed on several
filesystems (it passed on xfs, but only by accident).

This new version tries to leave about 256k free, then tries
a single 1M IO, and fails only if 0 bytes are written.

It also sends a lot more to $seq.full for debugging on failure
and fixes a few other stylistic things.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: bunch of fixups from V1 ;)
V3: use ls for size check, and 2g fs size

diff --git a/275 b/275
index 214262e..69b2895 100755
--- a/275
+++ b/275
@@ -1,8 +1,8 @@
 #! /bin/bash
 # FS QA Test No. 275
 #
-# The posix write test. when write size is larger than disk free size,
-# should write as more as possible
+# The posix write test.  When write size is larger than disk free size,
+# should write as much as possible until ENOSPC.
 #
 #-----------------------------------------------------------------------
 # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
@@ -30,13 +30,12 @@ echo "QA output created by $seq"
 
 here=`pwd`
 tmp=/tmp/$$
-status=0    # success is the default!
+status=1    # failure is the default!
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
 	cd /
-	rm -f $SCRATCH_MNT/* $tmp.*
 	_scratch_unmount
 }
 
@@ -49,41 +48,48 @@ _supported_os IRIX Linux
 _require_scratch
 
 echo "------------------------------"
-echo "write lack test"
+echo "write until ENOSPC test"
 echo "------------------------------"
 
 rm -f $seq.full
 
 umount $SCRATCH_DEV 2>/dev/null
-_scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
+_scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
 _scratch_mount
 
 rm -rf $SCRATCH_MNT/*
-cd $SCRATCH_MNT
 
-dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
-if [ $? -ne 0 ]
-then
-	echo "create file err"
-	status=1
-	exit
-fi
+dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=256K count=1 >>$seq.full 2>&1
+[ $? -ne 0 ] && _fail "Error creating file"
 
-dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
-dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
+# Attempt to completely fill fs
+dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=1M >>$seq.full 2>&1
 sync
+dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K >>$seq.full 2>&1
+sync
+# Last effort, use O_SYNC
+dd if=/dev/zero of=$SCRATCH_MNT/tmp4 bs=4K oflag=sync >>$seq.full 2>&1
+# Save space usage info to the full file
+echo "Pre rm space:" >> $seq.full
+df $SCRATCH_MNT >>$seq.full 2>&1
 
-rm -f tmp1
+# Should leave approx 256k free
+rm -f $SCRATCH_MNT/tmp1
 sync
+echo "Post rm space:" >> $seq.full
+df $SCRATCH_MNT >>$seq.full 2>&1
+_freespace=`df -k $SCRATCH_MNT | tail -n 1 | awk '{print $4}'`
+[ $_freespace -gt 1024 ] && _fail "could not sufficiently fill filesystem"
+
+# Try a write larger than available space
+dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M count=1 >>$seq.full 2>&1
+echo "Bytes written until ENOSPC:" >>$seq.full
+du $SCRATCH_MNT/tmp1 >>$seq.full
 
-dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
-_filesize=`du tmp1 | awk '{print $1}'`
-if [ $_filesize -ne 4 ]
-then
-	echo "write file err"
-	status=1
-	exit
-fi
+# And at least some of it should succeed.
+_filesize=`ls -l $SCRATCH_MNT/tmp1 | awk '{print $5}'`
+[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
 
 echo "done"
+status=0
 exit
diff --git a/275.out b/275.out
index 30af43c..69b9d52 100644
--- a/275.out
+++ b/275.out
@@ -1,5 +1,5 @@
 QA output created by 275
 ------------------------------
-write lack test
+write until ENOSPC test
 ------------------------------
 done

Re: [PATCH V3] xfstests: make 275 pass

[Eric Sandeen]

On 1/25/12 3:13 PM, Eric Sandeen wrote:
> Ok, this is a significant rework of 275, which made too many
> assumptions about details of space usage and failed on several
> filesystems (it passed on xfs, but only by accident).
> 
> This new version tries to leave about 256k free, then tries
> a single 1M IO, and fails only if 0 bytes are written.
> 
> It also sends a lot more to $seq.full for debugging on failure
> and fixes a few other stylistic things.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---

Poor Zach ran into this one again.  Can I get a review of this
version, and I'll merge it if it looks good?

Thanks,

-Eric

> V2: bunch of fixups from V1 ;)
> V3: use ls for size check, and 2g fs size
> 
> diff --git a/275 b/275
> index 214262e..69b2895 100755
> --- a/275
> +++ b/275
> @@ -1,8 +1,8 @@
>  #! /bin/bash
>  # FS QA Test No. 275
>  #
> -# The posix write test. when write size is larger than disk free size,
> -# should write as more as possible
> +# The posix write test.  When write size is larger than disk free size,
> +# should write as much as possible until ENOSPC.
>  #
>  #-----------------------------------------------------------------------
>  # Copyright (c) 2011-2012 Fujitsu, Inc.  All Rights Reserved.
> @@ -30,13 +30,12 @@ echo "QA output created by $seq"
>  
>  here=`pwd`
>  tmp=/tmp/$$
> -status=0    # success is the default!
> +status=1    # failure is the default!
>  trap "_cleanup; exit \$status" 0 1 2 3 15
>  
>  _cleanup()
>  {
>  	cd /
> -	rm -f $SCRATCH_MNT/* $tmp.*
>  	_scratch_unmount
>  }
>  
> @@ -49,41 +48,48 @@ _supported_os IRIX Linux
>  _require_scratch
>  
>  echo "------------------------------"
> -echo "write lack test"
> +echo "write until ENOSPC test"
>  echo "------------------------------"
>  
>  rm -f $seq.full
>  
>  umount $SCRATCH_DEV 2>/dev/null
> -_scratch_mkfs_sized $((1 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
> +_scratch_mkfs_sized $((2 * 1024 * 1024 * 1024)) >>$seq.full 2>&1
>  _scratch_mount
>  
>  rm -rf $SCRATCH_MNT/*
> -cd $SCRATCH_MNT
>  
> -dd if=/dev/zero of=tmp1 bs=4K count=1 >/dev/null 2>&1
> -if [ $? -ne 0 ]
> -then
> -	echo "create file err"
> -	status=1
> -	exit
> -fi
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=256K count=1 >>$seq.full 2>&1
> +[ $? -ne 0 ] && _fail "Error creating file"
>  
> -dd if=/dev/zero of=tmp2 bs=1M >/dev/null 2>&1
> -dd if=/dev/zero of=tmp3 bs=4K >/dev/null 2>&1
> +# Attempt to completely fill fs
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp2 bs=1M >>$seq.full 2>&1
>  sync
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp3 bs=4K >>$seq.full 2>&1
> +sync
> +# Last effort, use O_SYNC
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp4 bs=4K oflag=sync >>$seq.full 2>&1
> +# Save space usage info to the full file
> +echo "Pre rm space:" >> $seq.full
> +df $SCRATCH_MNT >>$seq.full 2>&1
>  
> -rm -f tmp1
> +# Should leave approx 256k free
> +rm -f $SCRATCH_MNT/tmp1
>  sync
> +echo "Post rm space:" >> $seq.full
> +df $SCRATCH_MNT >>$seq.full 2>&1
> +_freespace=`df -k $SCRATCH_MNT | tail -n 1 | awk '{print $4}'`
> +[ $_freespace -gt 1024 ] && _fail "could not sufficiently fill filesystem"
> +
> +# Try a write larger than available space
> +dd if=/dev/zero of=$SCRATCH_MNT/tmp1 bs=1M count=1 >>$seq.full 2>&1
> +echo "Bytes written until ENOSPC:" >>$seq.full
> +du $SCRATCH_MNT/tmp1 >>$seq.full
>  
> -dd if=/dev/zero of=tmp1 bs=8K count=1 >/dev/null 2>&1
> -_filesize=`du tmp1 | awk '{print $1}'`
> -if [ $_filesize -ne 4 ]
> -then
> -	echo "write file err"
> -	status=1
> -	exit
> -fi
> +# And at least some of it should succeed.
> +_filesize=`ls -l $SCRATCH_MNT/tmp1 | awk '{print $5}'`
> +[ $_filesize -eq 0 ] && _fail "write file err: Partial write until enospc failed; wrote 0 bytes."
>  
>  echo "done"
> +status=0
>  exit
> diff --git a/275.out b/275.out
> index 30af43c..69b9d52 100644
> --- a/275.out
> +++ b/275.out
> @@ -1,5 +1,5 @@
>  QA output created by 275
>  ------------------------------
> -write lack test
> +write until ENOSPC test
>  ------------------------------
>  done
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

Re: [PATCH V3] xfstests: make 275 pass

[Dave Chinner]

On Fri, Sep 07, 2012 at 02:56:38PM -0500, Eric Sandeen wrote:
> On 1/25/12 3:13 PM, Eric Sandeen wrote:
> > Ok, this is a significant rework of 275, which made too many
> > assumptions about details of space usage and failed on several
> > filesystems (it passed on xfs, but only by accident).
> > 
> > This new version tries to leave about 256k free, then tries
> > a single 1M IO, and fails only if 0 bytes are written.
> > 
> > It also sends a lot more to $seq.full for debugging on failure
> > and fixes a few other stylistic things.
> > 
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> > ---
> 
> Poor Zach ran into this one again.  Can I get a review of this
> version, and I'll merge it if it looks good?

Looks OK.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com