xfs
[Top] [All Lists]

Re: [PATCH] xfstests: fix wrong return check for case 022

To: Wang Shilong <wangsl.fnst@xxxxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests: fix wrong return check for case 022
From: Tsutomu Itoh <t-itoh@xxxxxxxxxxxxxx>
Date: Tue, 07 Jan 2014 12:24:56 +0900
Cc: xfs@xxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, Josef Bacik <jbacik@xxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <52CB642C.10500@xxxxxxxxxxxxxx>
References: <1388995737-3480-1-git-send-email-wangsl.fnst@xxxxxxxxxxxxxx> <52CA67F2.9010108@xxxxxxxxxxxxxx> <52CA6DDF.3080306@xxxxxxxxxxxxxx> <52CB5441.3030004@xxxxxxxxxxxxxx> <52CB642C.10500@xxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:24.0) Gecko/20100101 Thunderbird/24.2.0
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@xxxxxxxxxxxxxx>
>>>>> Cc: Josef Bacik <jbacik@xxxxxx>
>>>>> ---
>>>>>     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
>>>>>
>>


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