xfs
[Top] [All Lists]

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

To: Tsutomu Itoh <t-itoh@xxxxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests: fix wrong return check for case 022
From: Wang Shilong <wangsl.fnst@xxxxxxxxxxxxxx>
Date: Tue, 07 Jan 2014 11:48:25 +0800
Cc: xfs@xxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, Josef Bacik <jbacik@xxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <52CB7388.2040102@xxxxxxxxxxxxxx>
References: <1388995737-3480-1-git-send-email-wangsl.fnst@xxxxxxxxxxxxxx> <52CA67F2.9010108@xxxxxxxxxxxxxx> <52CA6DDF.3080306@xxxxxxxxxxxxxx> <52CB5441.3030004@xxxxxxxxxxxxxx> <52CB642C.10500@xxxxxxxxxxxxxx> <52CB7388.2040102@xxxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130612 Thunderbird/17.0.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@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?
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
>>>>>>
>
>

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