Re: [PATCH 2/3] Btrfs: rework qgroup accounting
On 01/08/2014 09:33 AM, David Sterba wrote:
On Wed, Dec 18, 2013 at 04:07:28PM -0500, Josef Bacik wrote:
/*
- * btrfs_qgroup_record_ref is called when the ref is added or deleted. it puts
- * the modification into a list that's later used by btrfs_end_transaction to
- * pass the recorded modifications on to btrfs_qgroup_account_ref.
+ * Record a quota operation for processing later on.
+ * @trans: the transaction we are adding the delayed op to.
+ * @fs_info: the fs_info for this fs.
+ * @ref_root: the root of the reference we are acting on,
+ * @num_bytes: the number of bytes in the reference.
+ * @parent: if we are removing a shared ref then this will be set.
+ * @type: the type of operation this is.
+ *
+ * We just add it to our trans qgroup_ref_list and carry on and process these
+ * operations in order at some later point. If the reference root isn't a fs
+ * root then we don't bother with doing anything.
+ *
+ * MUST BE HOLDING THE REF LOCK.
*/
int btrfs_qgroup_record_ref(struct btrfs_trans_handle *trans,
- struct btrfs_delayed_ref_node *node,
- struct btrfs_delayed_extent_op *extent_op)
+ struct btrfs_fs_info *fs_info, u64 ref_root,
+ u64 bytenr, u64 num_bytes, u64 parent,
+ enum btrfs_qgroup_operation_type type)
{
- struct qgroup_update *u;
+ struct btrfs_qgroup_operation *oper;
+ int ret;
- BUG_ON(!trans-delayed_ref_elem.seq);
- u = kmalloc(sizeof(*u), GFP_NOFS);
- if (!u)
+ if (!is_fstree(ref_root) || !fs_info-quota_enabled)
+ return 0;
+
+ oper = kmalloc(sizeof(*oper), GFP_NOFS);
Must use GFP_ATOMIC then, ohterwise spits some warnings:
This is because I'm still holding the tree lock, the ref_lock is just a
range lock so you can allocate under it. I'll fix this up, thanks,
Josef
--
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
Re: [PATCH 2/3] Btrfs: rework qgroup accounting
On Wed, Dec 18, 2013 at 04:07:28PM -0500, Josef Bacik wrote:
/*
- * btrfs_qgroup_record_ref is called when the ref is added or deleted. it
puts
- * the modification into a list that's later used by btrfs_end_transaction to
- * pass the recorded modifications on to btrfs_qgroup_account_ref.
+ * Record a quota operation for processing later on.
+ * @trans: the transaction we are adding the delayed op to.
+ * @fs_info: the fs_info for this fs.
+ * @ref_root: the root of the reference we are acting on,
+ * @num_bytes: the number of bytes in the reference.
+ * @parent: if we are removing a shared ref then this will be set.
+ * @type: the type of operation this is.
+ *
+ * We just add it to our trans qgroup_ref_list and carry on and process these
+ * operations in order at some later point. If the reference root isn't a fs
+ * root then we don't bother with doing anything.
+ *
+ * MUST BE HOLDING THE REF LOCK.
*/
int btrfs_qgroup_record_ref(struct btrfs_trans_handle *trans,
- struct btrfs_delayed_ref_node *node,
- struct btrfs_delayed_extent_op *extent_op)
+ struct btrfs_fs_info *fs_info, u64 ref_root,
+ u64 bytenr, u64 num_bytes, u64 parent,
+ enum btrfs_qgroup_operation_type type)
{
- struct qgroup_update *u;
+ struct btrfs_qgroup_operation *oper;
+ int ret;
- BUG_ON(!trans-delayed_ref_elem.seq);
- u = kmalloc(sizeof(*u), GFP_NOFS);
- if (!u)
+ if (!is_fstree(ref_root) || !fs_info-quota_enabled)
+ return 0;
+
+ oper = kmalloc(sizeof(*oper), GFP_NOFS);
Must use GFP_ATOMIC then, ohterwise spits some warnings:
BUG: sleeping function called from invalid context at mm/slab.c:2923
[ 5845.017803] [819ce6c3] dump_stack+0x51/0x6e
[ 5845.017811] [8108285a] __might_sleep+0xda/0x100
[ 5845.017816] [81159699] kmem_cache_alloc_trace+0xd9/0x1f0
[ 5845.017854] [a01a6f89] btrfs_qgroup_record_ref+0x89/0x310 [btrfs]
[ 5845.017872] [a0125a98] __btrfs_inc_extent_ref+0x328/0x570 [btrfs]
...
+/*
+ * Record a shared ref in the most recent qgroup operation added.
+ * @trans: the trans handle for this operation.
+ * @ref_root: the ref_root this operation was for, this is to make sure we
+ * actually need to do anything at all. Use the same ref_root we called
+ * btrfs_qgroup_record_ref with.
+ * @parent: the parent of the shared ref.
+ *
+ * This is meant to be called directly after calling btrfs_qgroup_record_ref
+ * for the ref we care about, it does no searching to make sure we're on the
+ * right qgroup operation.
+ *
+ * MUST BE HOLDING THE REF LOCK.
+ */
+int btrfs_qgroup_add_shared_ref(struct btrfs_trans_handle *trans, u64
ref_root,
+ u64 parent)
+{
+ struct qgroup_shared_ref *ref;
+ struct btrfs_qgroup_operation *oper;
+ int ret = 0;
+
+ if (!is_fstree(ref_root))
+ return 0;
+ ref = kmalloc(sizeof(*ref), GFP_NOFS);
same here
+ if (!ref)
+ return -ENOMEM;
+ ref-parent = parent;
+ ref-ref_root = 0;
+ ASSERT(!list_empty(trans-qgroup_ref_list));
+ oper = list_entry(trans-qgroup_ref_list.prev,
+ struct btrfs_qgroup_operation, list);
+ list_add_tail(ref-list, oper-shared_refs);
+ return ret;
+}
--
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
Re: [PATCH 2/3] Btrfs: rework qgroup accounting
On 12/21/2013 03:56 AM, Wang Shilong wrote: Hello Josef, Though i know there are still problems related to qgroup(for example removing snapshot will beak qgroup accounting).I did a simple test about your patch.. # btrfs quota enable /mnt # dd if=/dev/zero of=/mnt/data bs=4k count=102400 oflag=direct # btrfs sub snapshot /mnt/ /mnt/snap1 # btrfs sub snapshot /mnt /mnt/snap2 # btrfs sub delete /mnt/snap1 /mnt/snap2 # sync # rm -rf /mnt/data # sync # dmesg # btrfs qgroup show /mnt Firstly, qgroup accounting is wrong, this is maybe expected because efficient fs tree removal. However, from dmesg, i get the WARNING: WARNING: CPU: 1 PID: 2650 at fs/btrfs/qgroup.c:1486 btrfs_delayed_qgroup_accounting I did not take a deep look at codes, but i think you will be interested in taking a look at this. ^_^ Ok I finally sat down to look at this and it is because subvol deletion screws up quota accounting no matter what. I just added this WARN_ON() to catch actual mistakes, it just so happens it gets tripped when the snapshot deletion stuff happens too. I'm going to put this on the I'll deal with it later list since it is an existing issue with or without my patch. Thanks, Josef -- 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
Re: [PATCH 2/3] Btrfs: rework qgroup accounting
Hello Josef, Though i know there are still problems related to qgroup(for example removing snapshot will beak qgroup accounting).I did a simple test about your patch.. # btrfs quota enable /mnt # dd if=/dev/zero of=/mnt/data bs=4k count=102400 oflag=direct # btrfs sub snapshot /mnt/ /mnt/snap1 # btrfs sub snapshot /mnt /mnt/snap2 # btrfs sub delete /mnt/snap1 /mnt/snap2 # sync # rm -rf /mnt/data # sync # dmesg # btrfs qgroup show /mnt Firstly, qgroup accounting is wrong, this is maybe expected because efficient fs tree removal. However, from dmesg, i get the WARNING: WARNING: CPU: 1 PID: 2650 at fs/btrfs/qgroup.c:1486 btrfs_delayed_qgroup_accounting I did not take a deep look at codes, but i think you will be interested in taking a look at this. ^_^ Thanks, Wang-- 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
Re: [PATCH 2/3] Btrfs: rework qgroup accounting
On 12/21/2013 03:01 AM, Wang Shilong wrote: Hi Josef, I compie btrfs-next in my 32-bit machine, i get the following warnings: fs/btrfs/qgroup.c: In function ‘qgroup_excl_accounting’: fs/btrfs/qgroup.c:1503:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] qgroup = (struct btrfs_qgroup *)unode-aux; ^ fs/btrfs/qgroup.c: In function ‘qgroup_calc_old_refcnt’: fs/btrfs/qgroup.c:1571:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] qg = (struct btrfs_qgroup *)tmp_unode-aux; ^ fs/btrfs/qgroup.c: In function ‘qgroup_account_deleted_refs’: fs/btrfs/qgroup.c:1665:8: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] qg = (struct btrfs_qgroup *)unode-aux; ^ fs/btrfs/qgroup.c: In function ‘qgroup_calc_new_refcnt’: fs/btrfs/qgroup.c:1705:8: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] qg = (struct btrfs_qgroup *)unode-aux; ^ fs/btrfs/qgroup.c: In function ‘qgroup_adjust_counters’: fs/btrfs/qgroup.c:1767:8: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] qg = (struct btrfs_qgroup *)unode-aux; this patch is newly added into btrfs-next, so i think it is better that you fix these warnings locally .^_^ Crap I fixed part of these but not the other part, I'll fix it up and push it out Monday. Thanks, Josef -- 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
Re: [PATCH 2/3] Btrfs: rework qgroup accounting
On 12/21/2013 03:56 AM, Wang Shilong wrote: Hello Josef, Though i know there are still problems related to qgroup(for example removing snapshot will beak qgroup accounting).I did a simple test about your patch.. # btrfs quota enable /mnt # dd if=/dev/zero of=/mnt/data bs=4k count=102400 oflag=direct # btrfs sub snapshot /mnt/ /mnt/snap1 # btrfs sub snapshot /mnt /mnt/snap2 # btrfs sub delete /mnt/snap1 /mnt/snap2 # sync # rm -rf /mnt/data # sync # dmesg # btrfs qgroup show /mnt Firstly, qgroup accounting is wrong, this is maybe expected because efficient fs tree removal. However, from dmesg, i get the WARNING: WARNING: CPU: 1 PID: 2650 at fs/btrfs/qgroup.c:1486 btrfs_delayed_qgroup_accounting I did not take a deep look at codes, but i think you will be interested in taking a look at this. ^_^ Yup we shouldn't be warning, but I didn't change anything wrt quotas and snapshot deletion, that's a whole other issue I don't care about right now ;). Thanks, Josef -- 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
