diff mbox

xfstests: fix wrong return check for case 022

Message ID 1388995737-3480-1-git-send-email-wangsl.fnst@cn.fujitsu.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Wang Shilong Jan. 6, 2014, 8:08 a.m. UTC
Here we expect 0 as return value, fix it.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
Cc: Josef Bacik <jbacik@fb.com>
---
 tests/btrfs/022 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 mode change 100644 => 100755 tests/btrfs/022

Comments

Tsutomu Itoh Jan. 6, 2014, 8:23 a.m. UTC | #1
On 2014/01/06 17:08, Wang Shilong wrote:
> Here we expect 0 as return value, fix it.
> 
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> Cc: Josef Bacik <jbacik@fb.com>
> ---
>   tests/btrfs/022 | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>   mode change 100644 => 100755 tests/btrfs/022
> 
> diff --git a/tests/btrfs/022 b/tests/btrfs/022
> old mode 100644
> new mode 100755
> index 5b18643..142aaf9
> --- a/tests/btrfs/022
> +++ b/tests/btrfs/022
> @@ -106,7 +106,7 @@ _limit_test()
>   	[ $? -ne 0 ] || _fail "quota should have limited us"
>   	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
>   		$seqres.full 2>&1
> -	[ $? -eq 0 ] || _fail "should have been allowed to write"
> +	[ $? -ne 0 ] || _fail "should have been allowed to write"

from "man bash",
       An OR list has the form
              command1 || command2
       command2 is executed if and only if command1 returns a non-zero exit
       status.

So, original code is OK, I think.

Thanks,
Tsutomu

>   }
>   
>   _scratch_mkfs > /dev/null 2>&1
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang Shilong Jan. 6, 2014, 8:48 a.m. UTC | #2
Itoh San,

On 01/06/2014 04:23 PM, Tsutomu Itoh wrote:
> On 2014/01/06 17:08, Wang Shilong wrote:
>> Here we expect 0 as return value, fix it.
>>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> Cc: Josef Bacik <jbacik@fb.com>
>> ---
>>   tests/btrfs/022 | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>   mode change 100644 => 100755 tests/btrfs/022
>>
>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>> old mode 100644
>> new mode 100755
>> index 5b18643..142aaf9
>> --- a/tests/btrfs/022
>> +++ b/tests/btrfs/022
>> @@ -106,7 +106,7 @@ _limit_test()
>>   	[ $? -ne 0 ] || _fail "quota should have limited us"
>>   	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
>>   		$seqres.full 2>&1
>> -	[ $? -eq 0 ] || _fail "should have been allowed to write"
>> +	[ $? -ne 0 ] || _fail "should have been allowed to write"
> from "man bash",
>        An OR list has the form
>               command1 || command2
>        command2 is executed if and only if command1 returns a non-zero exit
>        status.
>
> So, original code is OK, I think.

You are absolutely right, during my test i found we fail this case.
I took a careful think about this:

The problem is that we should clear the data we have written before,
Because we may write some data before, for example we limit subvolume to 5M
then we try this:

# dd if=/dev/zero of=/mnt/data bs=10M count=1

Although we try to write 10M once, in kernel, we may split it into
several request,
at the end, we can still write some data actually, you can try the
following command
after the above command.

# du -sh /mnt/data

Here i think the best way to fix this problem is we disable quota and
retry with 4m.

# btrfs qgroup limit none /mnt <------------- in case we are nearly full
and we can not delete file.
# rm -rf /mnt/data && btrfs filesystem sync /mnt
# btrfs qgroup limit 5m /mnt
# dd if=/dev/zero of=/mnt bs=4m count=1

Please correct me if i miss something here.^_^

Thanks,
Wang
> Thanks,
> Tsutomu
>
>>   }
>>   
>>   _scratch_mkfs > /dev/null 2>&1
>>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tsutomu Itoh Jan. 7, 2014, 1:11 a.m. UTC | #3
On 2014/01/06 17:48, Wang Shilong wrote:
> Itoh San,
> 
> On 01/06/2014 04:23 PM, Tsutomu Itoh wrote:
>> On 2014/01/06 17:08, Wang Shilong wrote:
>>> Here we expect 0 as return value, fix it.
>>>
>>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>> Cc: Josef Bacik <jbacik@fb.com>
>>> ---
>>>    tests/btrfs/022 | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>    mode change 100644 => 100755 tests/btrfs/022
>>>
>>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>>> old mode 100644
>>> new mode 100755
>>> index 5b18643..142aaf9
>>> --- a/tests/btrfs/022
>>> +++ b/tests/btrfs/022
>>> @@ -106,7 +106,7 @@ _limit_test()
>>>    	[ $? -ne 0 ] || _fail "quota should have limited us"
>>>    	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
>>>    		$seqres.full 2>&1
>>> -	[ $? -eq 0 ] || _fail "should have been allowed to write"
>>> +	[ $? -ne 0 ] || _fail "should have been allowed to write"
>> from "man bash",
>>         An OR list has the form
>>                command1 || command2
>>         command2 is executed if and only if command1 returns a non-zero exit
>>         status.
>>
>> So, original code is OK, I think.
> 
> You are absolutely right, during my test i found we fail this case.
> I took a careful think about this:
> 
> The problem is that we should clear the data we have written before,
> Because we may write some data before, for example we limit subvolume to 5M
> then we try this:
> 
> # dd if=/dev/zero of=/mnt/data bs=10M count=1
> 
> Although we try to write 10M once, in kernel, we may split it into
> several request,
> at the end, we can still write some data actually, you can try the
> following command
> after the above command.
> 
> # du -sh /mnt/data
> 
> Here i think the best way to fix this problem is we disable quota and
> retry with 4m.

Umm... I don't think so.
Writing (or truncating) succeeds if sync is executed before writing 4MB.
Therefore, I think that there are some problems in quota.

===============================================================

# mkfs.btrfs -f /dev/sdc2

WARNING! - Btrfs v3.12 IS EXPERIMENTAL
WARNING! - see http://btrfs.wiki.kernel.org before using

Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
fs created label (null) on /dev/sdc2
        nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
Btrfs v3.12
# mount /dev/sdc2 /test8
# btrfs quota enable /test8
# btrfs qgroup limit 5M /test8
# dd if=/dev/zero of=/test8/file1 bs=10M count=1
dd: error writing '/test8/file1': Disk quota exceeded
1+0 records in
0+0 records out
5160960 bytes (5.2 MB) copied, 0.012274 s, 420 MB/s
# ls -ls /test8/file1
5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
# truncate -s 0 /test8/file1
truncate: failed to truncate '/test8/file1' at 0 bytes: Disk quota exceeded
# ls -ls /test8/file1
5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
# dd if=/dev/zero of=/test8/file1 bs=4M count=1
dd: failed to open '/test8/file1': Disk quota exceeded
# ls -ls /test8/file1
5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
#

---------------------------------------------------------------

# mkfs.btrfs -f /dev/sdc2

WARNING! - Btrfs v3.12 IS EXPERIMENTAL
WARNING! - see http://btrfs.wiki.kernel.org before using

Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
fs created label (null) on /dev/sdc2
        nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
Btrfs v3.12
# mount /dev/sdc2 /test8
# btrfs quota enable /test8
# btrfs qgroup limit 5M /test8
# dd if=/dev/zero of=/test8/file1 bs=10M count=1
dd: error writing '/test8/file1': Disk quota exceeded
1+0 records in
0+0 records out
5160960 bytes (5.2 MB) copied, 0.0120232 s, 429 MB/s
# ls -ls /test8/file1
5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
# btrfs filesystem sync /test8
FSSync '/test8'
# truncate -s 0 /test8/file1
# ls -ls /test8/file1
0 -rw-r--r-- 1 root root 0 Jan  7 09:54 /test8/file1
# dd if=/dev/zero of=/test8/file1 bs=4M count=1
1+0 records in
1+0 records out
4194304 bytes (4.2 MB) copied, 0.00734557 s, 571 MB/s
# ls -ls /test8/file1
4096 -rw-r--r-- 1 root root 4194304 Jan  7 09:54 /test8/file1
#

===============================================================

Thanks,
Tsutomu

> 
> # btrfs qgroup limit none /mnt <------------- in case we are nearly full
> and we can not delete file.
> # rm -rf /mnt/data && btrfs filesystem sync /mnt
> # btrfs qgroup limit 5m /mnt
> # dd if=/dev/zero of=/mnt bs=4m count=1
> 
> Please correct me if i miss something here.^_^
> 
> Thanks,
> Wang
>> Thanks,
>> Tsutomu
>>
>>>    }
>>>    
>>>    _scratch_mkfs > /dev/null 2>&1
>>>
>>
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang Shilong Jan. 7, 2014, 2:19 a.m. UTC | #4
On 01/07/2014 09:11 AM, Tsutomu Itoh wrote:
> On 2014/01/06 17:48, Wang Shilong wrote:
>> Itoh San,
>>
>> On 01/06/2014 04:23 PM, Tsutomu Itoh wrote:
>>> On 2014/01/06 17:08, Wang Shilong wrote:
>>>> Here we expect 0 as return value, fix it.
>>>>
>>>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>>> Cc: Josef Bacik <jbacik@fb.com>
>>>> ---
>>>>    tests/btrfs/022 | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>    mode change 100644 => 100755 tests/btrfs/022
>>>>
>>>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>>>> old mode 100644
>>>> new mode 100755
>>>> index 5b18643..142aaf9
>>>> --- a/tests/btrfs/022
>>>> +++ b/tests/btrfs/022
>>>> @@ -106,7 +106,7 @@ _limit_test()
>>>>    	[ $? -ne 0 ] || _fail "quota should have limited us"
>>>>    	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
>>>>    		$seqres.full 2>&1
>>>> -	[ $? -eq 0 ] || _fail "should have been allowed to write"
>>>> +	[ $? -ne 0 ] || _fail "should have been allowed to write"
>>> from "man bash",
>>>         An OR list has the form
>>>                command1 || command2
>>>         command2 is executed if and only if command1 returns a non-zero exit
>>>         status.
>>>
>>> So, original code is OK, I think.
>> You are absolutely right, during my test i found we fail this case.
>> I took a careful think about this:
>>
>> The problem is that we should clear the data we have written before,
>> Because we may write some data before, for example we limit subvolume to 5M
>> then we try this:
>>
>> # dd if=/dev/zero of=/mnt/data bs=10M count=1
>>
>> Although we try to write 10M once, in kernel, we may split it into
>> several request,
>> at the end, we can still write some data actually, you can try the
>> following command
>> after the above command.
>>
>> # du -sh /mnt/data
>>
>> Here i think the best way to fix this problem is we disable quota and
>> retry with 4m.
> Umm... I don't think so.
> Writing (or truncating) succeeds if sync is executed before writing 4MB.
> Therefore, I think that there are some problems in quota.
Itoh-San,

The problem is that 'btrfs filesystem sync' will flush and commit
transaction
which will release some space, and this may give us enough space to reserve
metadata space to truncate/remove file.

For the below example:

# dd if=/dev/zero of=/mnt/data bs=10M count=1
# sync
# btrfs qgroup show /mnt

qgroupid rfer excl
-------- ---- ----
0/5 5177344 5177344

Limit is 5M which means we still leave 65536 bytes space(4 leafsize).

If you try:
# rm -f /mnt/data

It must fail, because, unlink a file will first try to
reserve 5 nodesize(see __unlink_start_trans())

Let's see why truncate can succeed!

# btrfs-debug-tree /dev/sda

fs tree key (FS_TREE ROOT_ITEM 0)
leaf 29966336 items 7 free space 15641 generation 11 owner 5
fs uuid 2b854d8d-710c-43c5-a6d5-0cb8b6b232ea
chunk uuid 9b65e155-33b3-44d3-a413-bbc97af700a7
item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160
inode generation 3 transid 11 size 8 block group 0 mode 40755 links 1
item 1 key (256 INODE_REF 256) itemoff 16111 itemsize 12
inode ref index 0 namelen 2 name: ..
item 2 key (256 DIR_ITEM 2355243158) itemoff 16077 itemsize 34
location key (259 INODE_ITEM 0) type FILE
namelen 4 datalen 0 name: data
item 3 key (256 DIR_INDEX 4) itemoff 16043 itemsize 34
location key (259 INODE_ITEM 0) type FILE
namelen 4 datalen 0 name: data
item 4 key (259 INODE_ITEM 0) itemoff 15883 itemsize 160
inode generation 11 transid 11 size 5160960 block group 0 mode 100644
links 1
item 5 key (259 INODE_REF 256) itemoff 15869 itemsize 14
inode ref index 4 namelen 4 name: data
item 6 key (259 EXTENT_DATA 0) itemoff 15816 itemsize 53
extent data disk byte 12713984 nr 5160960
extent data offset 0 nr 5160960 ram 5160960
extent compression 0


For truncating inode, we will reserve 2 leafsize every time and and then
free qgroup
reserved space in btrfs_end_transaction(), however, for this case, we
only do one loop.
So command 'truncate -s 0 /mnt/data' should return successfully.

Please correct me if i miss something here ^_^

Thanks,
Wang
>
> ===============================================================
>
> # mkfs.btrfs -f /dev/sdc2
>
> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
> WARNING! - see http://btrfs.wiki.kernel.org before using
>
> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
> fs created label (null) on /dev/sdc2
>         nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
> Btrfs v3.12
> # mount /dev/sdc2 /test8
> # btrfs quota enable /test8
> # btrfs qgroup limit 5M /test8
> # dd if=/dev/zero of=/test8/file1 bs=10M count=1
> dd: error writing '/test8/file1': Disk quota exceeded
> 1+0 records in
> 0+0 records out
> 5160960 bytes (5.2 MB) copied, 0.012274 s, 420 MB/s
> # ls -ls /test8/file1
> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
> # truncate -s 0 /test8/file1
> truncate: failed to truncate '/test8/file1' at 0 bytes: Disk quota exceeded
> # ls -ls /test8/file1
> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
> # dd if=/dev/zero of=/test8/file1 bs=4M count=1
> dd: failed to open '/test8/file1': Disk quota exceeded
> # ls -ls /test8/file1
> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
> #
>
> ---------------------------------------------------------------
>
> # mkfs.btrfs -f /dev/sdc2
>
> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
> WARNING! - see http://btrfs.wiki.kernel.org before using
>
> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
> fs created label (null) on /dev/sdc2
>         nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
> Btrfs v3.12
> # mount /dev/sdc2 /test8
> # btrfs quota enable /test8
> # btrfs qgroup limit 5M /test8
> # dd if=/dev/zero of=/test8/file1 bs=10M count=1
> dd: error writing '/test8/file1': Disk quota exceeded
> 1+0 records in
> 0+0 records out
> 5160960 bytes (5.2 MB) copied, 0.0120232 s, 429 MB/s
> # ls -ls /test8/file1
> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
> # btrfs filesystem sync /test8
> FSSync '/test8'
> # truncate -s 0 /test8/file1
> # ls -ls /test8/file1
> 0 -rw-r--r-- 1 root root 0 Jan  7 09:54 /test8/file1
> # dd if=/dev/zero of=/test8/file1 bs=4M count=1
> 1+0 records in
> 1+0 records out
> 4194304 bytes (4.2 MB) copied, 0.00734557 s, 571 MB/s
> # ls -ls /test8/file1
> 4096 -rw-r--r-- 1 root root 4194304 Jan  7 09:54 /test8/file1
> #
>
> ===============================================================
>
> Thanks,
> Tsutomu
>
>> # btrfs qgroup limit none /mnt <------------- in case we are nearly full
>> and we can not delete file.
>> # rm -rf /mnt/data && btrfs filesystem sync /mnt
>> # btrfs qgroup limit 5m /mnt
>> # dd if=/dev/zero of=/mnt bs=4m count=1
>>
>> Please correct me if i miss something here.^_^
>>
>> Thanks,
>> Wang
>>> Thanks,
>>> Tsutomu
>>>
>>>>    }
>>>>    
>>>>    _scratch_mkfs > /dev/null 2>&1
>>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tsutomu Itoh Jan. 7, 2014, 3:24 a.m. UTC | #5
On 2014/01/07 11:19, Wang Shilong wrote:
> On 01/07/2014 09:11 AM, Tsutomu Itoh wrote:
>> On 2014/01/06 17:48, Wang Shilong wrote:
>>> Itoh San,
>>>
>>> On 01/06/2014 04:23 PM, Tsutomu Itoh wrote:
>>>> On 2014/01/06 17:08, Wang Shilong wrote:
>>>>> Here we expect 0 as return value, fix it.
>>>>>
>>>>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>>>> Cc: Josef Bacik <jbacik@fb.com>
>>>>> ---
>>>>>     tests/btrfs/022 | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>     mode change 100644 => 100755 tests/btrfs/022
>>>>>
>>>>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>>>>> old mode 100644
>>>>> new mode 100755
>>>>> index 5b18643..142aaf9
>>>>> --- a/tests/btrfs/022
>>>>> +++ b/tests/btrfs/022
>>>>> @@ -106,7 +106,7 @@ _limit_test()
>>>>>     	[ $? -ne 0 ] || _fail "quota should have limited us"
>>>>>     	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
>>>>>     		$seqres.full 2>&1
>>>>> -	[ $? -eq 0 ] || _fail "should have been allowed to write"
>>>>> +	[ $? -ne 0 ] || _fail "should have been allowed to write"
>>>> from "man bash",
>>>>          An OR list has the form
>>>>                 command1 || command2
>>>>          command2 is executed if and only if command1 returns a non-zero exit
>>>>          status.
>>>>
>>>> So, original code is OK, I think.
>>> You are absolutely right, during my test i found we fail this case.
>>> I took a careful think about this:
>>>
>>> The problem is that we should clear the data we have written before,
>>> Because we may write some data before, for example we limit subvolume to 5M
>>> then we try this:
>>>
>>> # dd if=/dev/zero of=/mnt/data bs=10M count=1
>>>
>>> Although we try to write 10M once, in kernel, we may split it into
>>> several request,
>>> at the end, we can still write some data actually, you can try the
>>> following command
>>> after the above command.
>>>
>>> # du -sh /mnt/data
>>>
>>> Here i think the best way to fix this problem is we disable quota and
>>> retry with 4m.
>> Umm... I don't think so.
>> Writing (or truncating) succeeds if sync is executed before writing 4MB.
>> Therefore, I think that there are some problems in quota.
> Itoh-San,
> 
> The problem is that 'btrfs filesystem sync' will flush and commit
> transaction
> which will release some space, and this may give us enough space to reserve
> metadata space to truncate/remove file.

Yes, I understand internal logic. But execution result is different when sync
was executed or not executed. This operation confuses the user, I think.
What do you think about this?

Thanks,
Tsutomu

> 
> For the below example:
> 
> # dd if=/dev/zero of=/mnt/data bs=10M count=1
> # sync
> # btrfs qgroup show /mnt
> 
> qgroupid rfer excl
> -------- ---- ----
> 0/5 5177344 5177344
> 
> Limit is 5M which means we still leave 65536 bytes space(4 leafsize).
> 
> If you try:
> # rm -f /mnt/data
> 
> It must fail, because, unlink a file will first try to
> reserve 5 nodesize(see __unlink_start_trans())
> 
> Let's see why truncate can succeed!
> 
> # btrfs-debug-tree /dev/sda
> 
> fs tree key (FS_TREE ROOT_ITEM 0)
> leaf 29966336 items 7 free space 15641 generation 11 owner 5
> fs uuid 2b854d8d-710c-43c5-a6d5-0cb8b6b232ea
> chunk uuid 9b65e155-33b3-44d3-a413-bbc97af700a7
> item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160
> inode generation 3 transid 11 size 8 block group 0 mode 40755 links 1
> item 1 key (256 INODE_REF 256) itemoff 16111 itemsize 12
> inode ref index 0 namelen 2 name: ..
> item 2 key (256 DIR_ITEM 2355243158) itemoff 16077 itemsize 34
> location key (259 INODE_ITEM 0) type FILE
> namelen 4 datalen 0 name: data
> item 3 key (256 DIR_INDEX 4) itemoff 16043 itemsize 34
> location key (259 INODE_ITEM 0) type FILE
> namelen 4 datalen 0 name: data
> item 4 key (259 INODE_ITEM 0) itemoff 15883 itemsize 160
> inode generation 11 transid 11 size 5160960 block group 0 mode 100644
> links 1
> item 5 key (259 INODE_REF 256) itemoff 15869 itemsize 14
> inode ref index 4 namelen 4 name: data
> item 6 key (259 EXTENT_DATA 0) itemoff 15816 itemsize 53
> extent data disk byte 12713984 nr 5160960
> extent data offset 0 nr 5160960 ram 5160960
> extent compression 0
> 
> 
> For truncating inode, we will reserve 2 leafsize every time and and then
> free qgroup
> reserved space in btrfs_end_transaction(), however, for this case, we
> only do one loop.
> So command 'truncate -s 0 /mnt/data' should return successfully.
> 
> Please correct me if i miss something here ^_^
> 
> Thanks,
> Wang
>>
>> ===============================================================
>>
>> # mkfs.btrfs -f /dev/sdc2
>>
>> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>
>> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
>> fs created label (null) on /dev/sdc2
>>          nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
>> Btrfs v3.12
>> # mount /dev/sdc2 /test8
>> # btrfs quota enable /test8
>> # btrfs qgroup limit 5M /test8
>> # dd if=/dev/zero of=/test8/file1 bs=10M count=1
>> dd: error writing '/test8/file1': Disk quota exceeded
>> 1+0 records in
>> 0+0 records out
>> 5160960 bytes (5.2 MB) copied, 0.012274 s, 420 MB/s
>> # ls -ls /test8/file1
>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>> # truncate -s 0 /test8/file1
>> truncate: failed to truncate '/test8/file1' at 0 bytes: Disk quota exceeded
>> # ls -ls /test8/file1
>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>> # dd if=/dev/zero of=/test8/file1 bs=4M count=1
>> dd: failed to open '/test8/file1': Disk quota exceeded
>> # ls -ls /test8/file1
>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>> #
>>
>> ---------------------------------------------------------------
>>
>> # mkfs.btrfs -f /dev/sdc2
>>
>> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>
>> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
>> fs created label (null) on /dev/sdc2
>>          nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
>> Btrfs v3.12
>> # mount /dev/sdc2 /test8
>> # btrfs quota enable /test8
>> # btrfs qgroup limit 5M /test8
>> # dd if=/dev/zero of=/test8/file1 bs=10M count=1
>> dd: error writing '/test8/file1': Disk quota exceeded
>> 1+0 records in
>> 0+0 records out
>> 5160960 bytes (5.2 MB) copied, 0.0120232 s, 429 MB/s
>> # ls -ls /test8/file1
>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>> # btrfs filesystem sync /test8
>> FSSync '/test8'
>> # truncate -s 0 /test8/file1
>> # ls -ls /test8/file1
>> 0 -rw-r--r-- 1 root root 0 Jan  7 09:54 /test8/file1
>> # dd if=/dev/zero of=/test8/file1 bs=4M count=1
>> 1+0 records in
>> 1+0 records out
>> 4194304 bytes (4.2 MB) copied, 0.00734557 s, 571 MB/s
>> # ls -ls /test8/file1
>> 4096 -rw-r--r-- 1 root root 4194304 Jan  7 09:54 /test8/file1
>> #
>>
>> ===============================================================
>>
>> Thanks,
>> Tsutomu
>>
>>> # btrfs qgroup limit none /mnt <------------- in case we are nearly full
>>> and we can not delete file.
>>> # rm -rf /mnt/data && btrfs filesystem sync /mnt
>>> # btrfs qgroup limit 5m /mnt
>>> # dd if=/dev/zero of=/mnt bs=4m count=1
>>>
>>> Please correct me if i miss something here.^_^
>>>
>>> Thanks,
>>> Wang
>>>> Thanks,
>>>> Tsutomu
>>>>
>>>>>     }
>>>>>     
>>>>>     _scratch_mkfs > /dev/null 2>&1
>>>>>
>>


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wang Shilong Jan. 7, 2014, 3:48 a.m. UTC | #6
On 01/07/2014 11:24 AM, Tsutomu Itoh wrote:
> On 2014/01/07 11:19, Wang Shilong wrote:
>> On 01/07/2014 09:11 AM, Tsutomu Itoh wrote:
>>> On 2014/01/06 17:48, Wang Shilong wrote:
>>>> Itoh San,
>>>>
>>>> On 01/06/2014 04:23 PM, Tsutomu Itoh wrote:
>>>>> On 2014/01/06 17:08, Wang Shilong wrote:
>>>>>> Here we expect 0 as return value, fix it.
>>>>>>
>>>>>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>>>>> Cc: Josef Bacik <jbacik@fb.com>
>>>>>> ---
>>>>>>     tests/btrfs/022 | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>     mode change 100644 => 100755 tests/btrfs/022
>>>>>>
>>>>>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>>>>>> old mode 100644
>>>>>> new mode 100755
>>>>>> index 5b18643..142aaf9
>>>>>> --- a/tests/btrfs/022
>>>>>> +++ b/tests/btrfs/022
>>>>>> @@ -106,7 +106,7 @@ _limit_test()
>>>>>>     	[ $? -ne 0 ] || _fail "quota should have limited us"
>>>>>>     	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
>>>>>>     		$seqres.full 2>&1
>>>>>> -	[ $? -eq 0 ] || _fail "should have been allowed to write"
>>>>>> +	[ $? -ne 0 ] || _fail "should have been allowed to write"
>>>>> from "man bash",
>>>>>          An OR list has the form
>>>>>                 command1 || command2
>>>>>          command2 is executed if and only if command1 returns a non-zero exit
>>>>>          status.
>>>>>
>>>>> So, original code is OK, I think.
>>>> You are absolutely right, during my test i found we fail this case.
>>>> I took a careful think about this:
>>>>
>>>> The problem is that we should clear the data we have written before,
>>>> Because we may write some data before, for example we limit subvolume to 5M
>>>> then we try this:
>>>>
>>>> # dd if=/dev/zero of=/mnt/data bs=10M count=1
>>>>
>>>> Although we try to write 10M once, in kernel, we may split it into
>>>> several request,
>>>> at the end, we can still write some data actually, you can try the
>>>> following command
>>>> after the above command.
>>>>
>>>> # du -sh /mnt/data
>>>>
>>>> Here i think the best way to fix this problem is we disable quota and
>>>> retry with 4m.
>>> Umm... I don't think so.
>>> Writing (or truncating) succeeds if sync is executed before writing 4MB.
>>> Therefore, I think that there are some problems in quota.
>> Itoh-San,
>>
>> The problem is that 'btrfs filesystem sync' will flush and commit
>> transaction
>> which will release some space, and this may give us enough space to reserve
>> metadata space to truncate/remove file.
> Yes, I understand internal logic. But execution result is different when sync
> was executed or not executed. This operation confuses the user, I think.
> What do you think about this?
Yeah, it is a little confused. But like btrfs normal Reservations,
Users will encounter ENOSPC, but if they dose something like 'sync', user
can still get some space.

Aslo this problem only happen when we are nearly to quota limit, so for
current
situations i have not thought a better solution yet for such case. ^_^

Thanks,
Wang
>
> Thanks,
> Tsutomu
>
>> For the below example:
>>
>> # dd if=/dev/zero of=/mnt/data bs=10M count=1
>> # sync
>> # btrfs qgroup show /mnt
>>
>> qgroupid rfer excl
>> -------- ---- ----
>> 0/5 5177344 5177344
>>
>> Limit is 5M which means we still leave 65536 bytes space(4 leafsize).
>>
>> If you try:
>> # rm -f /mnt/data
>>
>> It must fail, because, unlink a file will first try to
>> reserve 5 nodesize(see __unlink_start_trans())
>>
>> Let's see why truncate can succeed!
>>
>> # btrfs-debug-tree /dev/sda
>>
>> fs tree key (FS_TREE ROOT_ITEM 0)
>> leaf 29966336 items 7 free space 15641 generation 11 owner 5
>> fs uuid 2b854d8d-710c-43c5-a6d5-0cb8b6b232ea
>> chunk uuid 9b65e155-33b3-44d3-a413-bbc97af700a7
>> item 0 key (256 INODE_ITEM 0) itemoff 16123 itemsize 160
>> inode generation 3 transid 11 size 8 block group 0 mode 40755 links 1
>> item 1 key (256 INODE_REF 256) itemoff 16111 itemsize 12
>> inode ref index 0 namelen 2 name: ..
>> item 2 key (256 DIR_ITEM 2355243158) itemoff 16077 itemsize 34
>> location key (259 INODE_ITEM 0) type FILE
>> namelen 4 datalen 0 name: data
>> item 3 key (256 DIR_INDEX 4) itemoff 16043 itemsize 34
>> location key (259 INODE_ITEM 0) type FILE
>> namelen 4 datalen 0 name: data
>> item 4 key (259 INODE_ITEM 0) itemoff 15883 itemsize 160
>> inode generation 11 transid 11 size 5160960 block group 0 mode 100644
>> links 1
>> item 5 key (259 INODE_REF 256) itemoff 15869 itemsize 14
>> inode ref index 4 namelen 4 name: data
>> item 6 key (259 EXTENT_DATA 0) itemoff 15816 itemsize 53
>> extent data disk byte 12713984 nr 5160960
>> extent data offset 0 nr 5160960 ram 5160960
>> extent compression 0
>>
>>
>> For truncating inode, we will reserve 2 leafsize every time and and then
>> free qgroup
>> reserved space in btrfs_end_transaction(), however, for this case, we
>> only do one loop.
>> So command 'truncate -s 0 /mnt/data' should return successfully.
>>
>> Please correct me if i miss something here ^_^
>>
>> Thanks,
>> Wang
>>> ===============================================================
>>>
>>> # mkfs.btrfs -f /dev/sdc2
>>>
>>> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
>>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>>
>>> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
>>> fs created label (null) on /dev/sdc2
>>>          nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
>>> Btrfs v3.12
>>> # mount /dev/sdc2 /test8
>>> # btrfs quota enable /test8
>>> # btrfs qgroup limit 5M /test8
>>> # dd if=/dev/zero of=/test8/file1 bs=10M count=1
>>> dd: error writing '/test8/file1': Disk quota exceeded
>>> 1+0 records in
>>> 0+0 records out
>>> 5160960 bytes (5.2 MB) copied, 0.012274 s, 420 MB/s
>>> # ls -ls /test8/file1
>>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>>> # truncate -s 0 /test8/file1
>>> truncate: failed to truncate '/test8/file1' at 0 bytes: Disk quota exceeded
>>> # ls -ls /test8/file1
>>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>>> # dd if=/dev/zero of=/test8/file1 bs=4M count=1
>>> dd: failed to open '/test8/file1': Disk quota exceeded
>>> # ls -ls /test8/file1
>>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>>> #
>>>
>>> ---------------------------------------------------------------
>>>
>>> # mkfs.btrfs -f /dev/sdc2
>>>
>>> WARNING! - Btrfs v3.12 IS EXPERIMENTAL
>>> WARNING! - see http://btrfs.wiki.kernel.org before using
>>>
>>> Turning ON incompat feature 'extref': increased hardlink limit per file to 65536
>>> fs created label (null) on /dev/sdc2
>>>          nodesize 16384 leafsize 16384 sectorsize 4096 size 9.31GiB
>>> Btrfs v3.12
>>> # mount /dev/sdc2 /test8
>>> # btrfs quota enable /test8
>>> # btrfs qgroup limit 5M /test8
>>> # dd if=/dev/zero of=/test8/file1 bs=10M count=1
>>> dd: error writing '/test8/file1': Disk quota exceeded
>>> 1+0 records in
>>> 0+0 records out
>>> 5160960 bytes (5.2 MB) copied, 0.0120232 s, 429 MB/s
>>> # ls -ls /test8/file1
>>> 5040 -rw-r--r-- 1 root root 5160960 Jan  7 09:54 /test8/file1
>>> # btrfs filesystem sync /test8
>>> FSSync '/test8'
>>> # truncate -s 0 /test8/file1
>>> # ls -ls /test8/file1
>>> 0 -rw-r--r-- 1 root root 0 Jan  7 09:54 /test8/file1
>>> # dd if=/dev/zero of=/test8/file1 bs=4M count=1
>>> 1+0 records in
>>> 1+0 records out
>>> 4194304 bytes (4.2 MB) copied, 0.00734557 s, 571 MB/s
>>> # ls -ls /test8/file1
>>> 4096 -rw-r--r-- 1 root root 4194304 Jan  7 09:54 /test8/file1
>>> #
>>>
>>> ===============================================================
>>>
>>> Thanks,
>>> Tsutomu
>>>
>>>> # btrfs qgroup limit none /mnt <------------- in case we are nearly full
>>>> and we can not delete file.
>>>> # rm -rf /mnt/data && btrfs filesystem sync /mnt
>>>> # btrfs qgroup limit 5m /mnt
>>>> # dd if=/dev/zero of=/mnt bs=4m count=1
>>>>
>>>> Please correct me if i miss something here.^_^
>>>>
>>>> Thanks,
>>>> Wang
>>>>> Thanks,
>>>>> Tsutomu
>>>>>
>>>>>>     }
>>>>>>     
>>>>>>     _scratch_mkfs > /dev/null 2>&1
>>>>>>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tests/btrfs/022 b/tests/btrfs/022
old mode 100644
new mode 100755
index 5b18643..142aaf9
--- a/tests/btrfs/022
+++ b/tests/btrfs/022
@@ -106,7 +106,7 @@  _limit_test()
 	[ $? -ne 0 ] || _fail "quota should have limited us"
 	dd if=/dev/urandom of=$SCRATCH_MNT/a/file bs=4M count=1 >> \
 		$seqres.full 2>&1
-	[ $? -eq 0 ] || _fail "should have been allowed to write"
+	[ $? -ne 0 ] || _fail "should have been allowed to write"
 }
 
 _scratch_mkfs > /dev/null 2>&1