Hi Yang, On 2015/01/05 15:16, Dongsheng Yang wrote: > Hi Josef and others, > > This patch set is about enhancing qgroup. > > [1/3]: fix a bug about qgroup leak when we exceed quota limit, > It is reviewd by Josef. > [2/3]: introduce a new accounter in qgroup to close a window where > user will exceed the limit by qgroup. It "looks good" to Josef. > [3/3]: a new patch to fix a bug reported by Satoru.
I tested your the patchset v3. Although it's far better than the patchset v2, there is still one problem in this patchset. When I wrote 1.5GiB to a subvolume with 1.0 GiB limit, 1.0GiB - 139 block (in this case, 1KiB/block) was written. I consider user should be able to write just 1.0GiB in this case. * Test result =============================================================================== + mkfs.btrfs -f /dev/vdb Btrfs v3.17 See http://btrfs.wiki.kernel.org for more information. Turning ON incompat feature 'extref': increased hardlink limit per file to 65536 fs created label (null) on /dev/vdb nodesize 16384 leafsize 16384 sectorsize 4096 size 30.00GiB + mount /dev/vdb /root/btrfs-auto-test/ + ret=0 + btrfs quota enable /root/btrfs-auto-test/ + btrfs subvolume create /root/btrfs-auto-test//sub Create subvolume '/root/btrfs-auto-test/sub' + btrfs qgroup limit 1G /root/btrfs-auto-test//sub + dd if=/dev/zero of=/root/btrfs-auto-test//sub/file bs=1024 count=1500000 dd: error writing '/root/btrfs-auto-test//sub/file': Disk quota exceeded 1048438+0 records in # Tried to write 1GiB - 138 KiB 1048437+0 records out # Succeeded to write 1GiB - 139 KiB 1073599488 bytes (1.1 GB) copied, 19.0247 s, 56.4 MB/s =============================================================================== * note I tried to run the reproducer five times and the result is a bit different for each time. ========================= # Written ------------------------- 1 1GiB - 139 KiB 2 1GiB - 139 KiB 3 1GiB - 145 KiB 4 1GiB - 135 KiB 5 1GiB - 135 KiB ========================== So I consider it's a problem comes from timing. If I changed the block size from 1KiB to 1 MiB, the difference in bytes got larger. ============================ # Written ---------------------------- 1 1GiB - 1 MiB 2 1GiB - 1 MiB 3 1GiB - 1 MiB 4 1GiB - 1 MiB 5 1GiB - 1 MiB ============================ Thanks, Satoru > > BTW, I have some other plan about qgroup in my TODO list: > > Kernel: > a). adjust the accounters in parent qgroup when we move > the child qgroup. > Currently, when we move a qgroup, the parent qgroup > will not updated at the same time. This will cause some wrong > numbers in qgroup. > > b). add a ioctl to show the qgroup info. > Command "btrfs qgroup show" is showing the qgroup info > read from qgroup tree. But there is some information in memory > which is not synced into device. Then it will show some outdate > number. > > c). limit and account size in 3 modes, data, metadata and both. > qgroup is accounting the size both of data and metadata > togather, but to a user, the data size is the most useful to them. > > d). remove a subvolume related qgroup when subvolume is deleted and > there is no other reference to it. > > user-tool: > a). Add the unit of B/K/M/G to btrfs qgroup show. > b). get the information via ioctl rather than reading it from > btree. Will keep the old way as a fallback for compatiblity. > > Any comment and sugguestion is welcome. :) > > Yang > > Dongsheng Yang (3): > Btrfs: qgroup: free reserved in exceeding quota. > Btrfs: qgroup: Introduce a may_use to account > space_info->bytes_may_use. > Btrfs: qgroup, Account data space in more proper timings. > > fs/btrfs/extent-tree.c | 41 +++++++++++++++++++++++------- > fs/btrfs/file.c | 9 ------- > fs/btrfs/inode.c | 18 ++++++++++++- > fs/btrfs/qgroup.c | 68 > +++++++++++++++++++++++++++++++++++++++++++++++--- > fs/btrfs/qgroup.h | 4 +++ > 5 files changed, 117 insertions(+), 23 deletions(-) > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
