* [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 12:55 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 2/9] btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref Qu Wenruo
` (8 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
Current delayed ref interface has several problems:
- Longer and longer parameter lists
bytenr
num_bytes
parent
---------- so far so good
ref_root
owner
offset
---------- I don't feel good now
- Different interpretation for the same parameter
Above @owner for data ref is inode number (u64),
while for tree ref, it's level (int).
They are even in different size range.
For level we only need 0~8, while for ino it's
BTRFS_FIRST_FREE_OBJECTID~BTRFS_LAST_FREE_OBJECTID.
And @offset doesn't even makes sense for tree ref.
Such parameter reuse may look clever as an hidden union, but it
destroys code readability.
To solve both problems, we introduce a new structure, btrfs_ref to solve
them:
- Structure instead of long parameter list
This makes later expansion easier, and better documented.
- Use btrfs_ref::type to distinguish data and tree ref
- Use proper union to store data/tree ref specific structures.
- Use separate functions to fill data/tree ref data, with a common generic
function to fill common bytenr/num_bytes members.
All parameters will find its place in btrfs_ref, and an extra member,
@real_root, inspired by ref-verify code, is newly introduced for later
qgroup code, to record which tree is triggered this extent modification.
This patch doesn't touch any code, but provides the basis for incoming
refactors.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/delayed-ref.h | 116 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 116 insertions(+)
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index d2af974f68a1..24addc5163bc 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -187,6 +187,90 @@ struct btrfs_delayed_ref_root {
u64 qgroup_to_skip;
};
+enum btrfs_ref_type {
+ BTRFS_REF_NOT_SET,
+ BTRFS_REF_DATA,
+ BTRFS_REF_METADATA,
+ BTRFS_REF_LAST,
+};
+
+struct btrfs_data_ref {
+ /* For EXTENT_DATA_REF */
+
+ /* Root who refers to this data extent */
+ u64 ref_root;
+
+ /* Inode who refers to this data extent */
+ u64 ino;
+
+ /*
+ * file_offset - extent_offset
+ *
+ * file_offset is the key.offset of the EXTENT_DATA key.
+ * extent_offset is btrfs_file_extent_offset() of the EXTENT_DATA data.
+ */
+ u64 offset;
+};
+
+struct btrfs_tree_ref {
+ /*
+ * Level of this tree block
+ *
+ * Shared for skinny (TREE_BLOCK_REF) and normal tree ref.
+ */
+ int level;
+
+ /*
+ * Root who refers to this tree block.
+ *
+ * For TREE_BLOCK_REF (skinny metadata, either inline or keyed)
+ */
+ u64 root;
+
+ /* For non-skinny metadata, no special member needed */
+};
+
+struct btrfs_ref {
+ enum btrfs_ref_type type;
+ int action;
+
+ /*
+ * Only use parent pointers as backref (SHARED_BLOCK_REF or
+ * SHARED_DATA_REF) for this extent and its children.
+ * Set for reloc trees.
+ */
+ bool only_backreferences:1;
+
+ /*
+ * Whether this extent should go through qgroup record.
+ *
+ * Normally false, but for certain case like delayed subtree scan,
+ * setting this flag can hugely reduce qgroup overhead.
+ */
+ bool skip_qgroup:1;
+
+ /*
+ * Optional. To which root this modification is for.
+ * Mostly used for qgroup optimization.
+ *
+ * When unset, data/tree ref init code will populate it.
+ * In certain case, we're modifying reference for a different root.
+ * E.g. Cow fs tree blocks for balance.
+ * In that case, tree_ref::root will be fs tree, but we're doing this
+ * for reloc tree, then we should set @real_root to reloc tree.
+ */
+ u64 real_root;
+ u64 bytenr;
+ u64 len;
+
+ /* Bytenr of the parent tree block */
+ u64 parent;
+ union {
+ struct btrfs_data_ref data_ref;
+ struct btrfs_tree_ref tree_ref;
+ };
+};
+
extern struct kmem_cache *btrfs_delayed_ref_head_cachep;
extern struct kmem_cache *btrfs_delayed_tree_ref_cachep;
extern struct kmem_cache *btrfs_delayed_data_ref_cachep;
@@ -195,6 +279,38 @@ extern struct kmem_cache *btrfs_delayed_extent_op_cachep;
int __init btrfs_delayed_ref_init(void);
void __cold btrfs_delayed_ref_exit(void);
+static inline void btrfs_init_generic_ref(struct btrfs_ref *generic_ref,
+ int action, u64 bytenr, u64 len, u64 parent)
+{
+ generic_ref->action = action;
+ generic_ref->bytenr = bytenr;
+ generic_ref->len = len;
+ generic_ref->parent = parent;
+}
+
+static inline void btrfs_init_tree_ref(struct btrfs_ref *generic_ref,
+ int level, u64 root)
+{
+ /* If @real_root not set, use @root as fallback */
+ if (!generic_ref->real_root)
+ generic_ref->real_root = root;
+ generic_ref->tree_ref.level = level;
+ generic_ref->tree_ref.root = root;
+ generic_ref->type = BTRFS_REF_METADATA;
+}
+
+static inline void btrfs_init_data_ref(struct btrfs_ref *generic_ref,
+ u64 ref_root, u64 ino, u64 offset)
+{
+ /* If @real_root not set, use @root as fallback */
+ if (!generic_ref->real_root)
+ generic_ref->real_root = ref_root;
+ generic_ref->data_ref.ref_root = ref_root;
+ generic_ref->data_ref.ino = ino;
+ generic_ref->data_ref.offset = offset;
+ generic_ref->type = BTRFS_REF_DATA;
+}
+
static inline struct btrfs_delayed_extent_op *
btrfs_alloc_delayed_extent_op(void)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-11 5:16 ` [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures Qu Wenruo
@ 2019-02-11 12:55 ` Nikolay Borisov
2019-02-11 13:23 ` Qu Wenruo
0 siblings, 1 reply; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 12:55 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
> Current delayed ref interface has several problems:
> - Longer and longer parameter lists
> bytenr
> num_bytes
> parent
> ---------- so far so good
> ref_root
> owner
> offset
> ---------- I don't feel good now
>
> - Different interpretation for the same parameter
> Above @owner for data ref is inode number (u64),
> while for tree ref, it's level (int).
>
> They are even in different size range.
> For level we only need 0~8, while for ino it's
> BTRFS_FIRST_FREE_OBJECTID~BTRFS_LAST_FREE_OBJECTID.
>
> And @offset doesn't even makes sense for tree ref.
>
> Such parameter reuse may look clever as an hidden union, but it
> destroys code readability.
>
> To solve both problems, we introduce a new structure, btrfs_ref to solve
> them:
>
> - Structure instead of long parameter list
> This makes later expansion easier, and better documented.
>
> - Use btrfs_ref::type to distinguish data and tree ref
>
> - Use proper union to store data/tree ref specific structures.
>
> - Use separate functions to fill data/tree ref data, with a common generic
> function to fill common bytenr/num_bytes members.
>
> All parameters will find its place in btrfs_ref, and an extra member,
> @real_root, inspired by ref-verify code, is newly introduced for later
> qgroup code, to record which tree is triggered this extent modification.
>
> This patch doesn't touch any code, but provides the basis for incoming
> refactors.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/delayed-ref.h | 116 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 116 insertions(+)
>
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index d2af974f68a1..24addc5163bc 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -187,6 +187,90 @@ struct btrfs_delayed_ref_root {
> u64 qgroup_to_skip;
> };
>
> +enum btrfs_ref_type {
> + BTRFS_REF_NOT_SET,
> + BTRFS_REF_DATA,
> + BTRFS_REF_METADATA,
> + BTRFS_REF_LAST,
> +};
> +
> +struct btrfs_data_ref {
> + /* For EXTENT_DATA_REF */
> +
> + /* Root who refers to this data extent */
nit: s/who/which/
> + u64 ref_root;
> +
> + /* Inode who refers to this data extent */
nit: DITTO
> + u64 ino;
> +
> + /*
> + * file_offset - extent_offset
> + *
> + * file_offset is the key.offset of the EXTENT_DATA key.
> + * extent_offset is btrfs_file_extent_offset() of the EXTENT_DATA data.
> + */
This needs rewording since it's rather cryptic now. Looking at the dev
docs and the description for 'offset' field in btrfs_file_extent_item I
can sort of deduce that this field will only be different than null if
this reference is for an extent which is shared between 2 snapshots.
So if file foo is shared between two snapshots, has 1 extent and in
snapshot2 this extent is partially changed then I'd expect extent_offset
to point to the start in the original (unchanged extent), correct?
> + u64 offset;
> +};
> +
> +struct btrfs_tree_ref {
> + /*
> + * Level of this tree block
> + *
> + * Shared for skinny (TREE_BLOCK_REF) and normal tree ref.
This sentence is also not very clear? You mean this level applies to
tree block refs (irrespective of whether they are shared or normal tree
block refs)?
> + */
> + int level;
> +
> + /*
> + * Root who refers to this tree block.
nit:s/who/which
> + *
> + * For TREE_BLOCK_REF (skinny metadata, either inline or keyed)
> + */
> + u64 root;
> +
> + /* For non-skinny metadata, no special member needed */
> +};
> +
> +struct btrfs_ref {
> + enum btrfs_ref_type type;
> + int action;
> +
> + /*
> + * Only use parent pointers as backref (SHARED_BLOCK_REF or
> + * SHARED_DATA_REF) for this extent and its children.
> + * Set for reloc trees.
> + */
> + bool only_backreferences:1;
> +
> + /*
> + * Whether this extent should go through qgroup record.
> + *
> + * Normally false, but for certain case like delayed subtree scan,
> + * setting this flag can hugely reduce qgroup overhead.
> + */
> + bool skip_qgroup:1;
> +
> + /*
> + * Optional. To which root this modification is for.
> + * Mostly used for qgroup optimization.
> + *
> + * When unset, data/tree ref init code will populate it.
> + * In certain case, we're modifying reference for a different root.
> + * E.g. Cow fs tree blocks for balance.
> + * In that case, tree_ref::root will be fs tree, but we're doing this
> + * for reloc tree, then we should set @real_root to reloc tree.
> + */
> + u64 real_root;
> + u64 bytenr;
> + u64 len;
> +
> + /* Bytenr of the parent tree block */
> + u64 parent;
> + union {
> + struct btrfs_data_ref data_ref;
> + struct btrfs_tree_ref tree_ref;
> + };
> +};
> +
> extern struct kmem_cache *btrfs_delayed_ref_head_cachep;
> extern struct kmem_cache *btrfs_delayed_tree_ref_cachep;
> extern struct kmem_cache *btrfs_delayed_data_ref_cachep;
> @@ -195,6 +279,38 @@ extern struct kmem_cache *btrfs_delayed_extent_op_cachep;
> int __init btrfs_delayed_ref_init(void);
> void __cold btrfs_delayed_ref_exit(void);
>
> +static inline void btrfs_init_generic_ref(struct btrfs_ref *generic_ref,
> + int action, u64 bytenr, u64 len, u64 parent)
> +{
> + generic_ref->action = action;
> + generic_ref->bytenr = bytenr;
> + generic_ref->len = len;
> + generic_ref->parent = parent;
> +}
> +
> +static inline void btrfs_init_tree_ref(struct btrfs_ref *generic_ref,
> + int level, u64 root)
> +{
> + /* If @real_root not set, use @root as fallback */
> + if (!generic_ref->real_root)
> + generic_ref->real_root = root;
> + generic_ref->tree_ref.level = level;
> + generic_ref->tree_ref.root = root;
> + generic_ref->type = BTRFS_REF_METADATA;
> +}
> +
> +static inline void btrfs_init_data_ref(struct btrfs_ref *generic_ref,
> + u64 ref_root, u64 ino, u64 offset)
> +{
> + /* If @real_root not set, use @root as fallback */
> + if (!generic_ref->real_root)
> + generic_ref->real_root = ref_root;
> + generic_ref->data_ref.ref_root = ref_root;
> + generic_ref->data_ref.ino = ino;
> + generic_ref->data_ref.offset = offset;
> + generic_ref->type = BTRFS_REF_DATA;
> +}
> +
> static inline struct btrfs_delayed_extent_op *
> btrfs_alloc_delayed_extent_op(void)
> {
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-11 12:55 ` Nikolay Borisov
@ 2019-02-11 13:23 ` Qu Wenruo
2019-02-11 14:20 ` Nikolay Borisov
0 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 13:23 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2019/2/11 下午8:55, Nikolay Borisov wrote:
>
>
> On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
>> Current delayed ref interface has several problems:
>> - Longer and longer parameter lists
>> bytenr
>> num_bytes
>> parent
>> ---------- so far so good
>> ref_root
>> owner
>> offset
>> ---------- I don't feel good now
>>
>> - Different interpretation for the same parameter
>> Above @owner for data ref is inode number (u64),
>> while for tree ref, it's level (int).
>>
>> They are even in different size range.
>> For level we only need 0~8, while for ino it's
>> BTRFS_FIRST_FREE_OBJECTID~BTRFS_LAST_FREE_OBJECTID.
>>
>> And @offset doesn't even makes sense for tree ref.
>>
>> Such parameter reuse may look clever as an hidden union, but it
>> destroys code readability.
>>
>> To solve both problems, we introduce a new structure, btrfs_ref to solve
>> them:
>>
>> - Structure instead of long parameter list
>> This makes later expansion easier, and better documented.
>>
>> - Use btrfs_ref::type to distinguish data and tree ref
>>
>> - Use proper union to store data/tree ref specific structures.
>>
>> - Use separate functions to fill data/tree ref data, with a common generic
>> function to fill common bytenr/num_bytes members.
>>
>> All parameters will find its place in btrfs_ref, and an extra member,
>> @real_root, inspired by ref-verify code, is newly introduced for later
>> qgroup code, to record which tree is triggered this extent modification.
>>
>> This patch doesn't touch any code, but provides the basis for incoming
>> refactors.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/delayed-ref.h | 116 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 116 insertions(+)
>>
>> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
>> index d2af974f68a1..24addc5163bc 100644
>> --- a/fs/btrfs/delayed-ref.h
>> +++ b/fs/btrfs/delayed-ref.h
>> @@ -187,6 +187,90 @@ struct btrfs_delayed_ref_root {
>> u64 qgroup_to_skip;
>> };
>>
>> +enum btrfs_ref_type {
>> + BTRFS_REF_NOT_SET,
>> + BTRFS_REF_DATA,
>> + BTRFS_REF_METADATA,
>> + BTRFS_REF_LAST,
>> +};
>> +
>> +struct btrfs_data_ref {
>> + /* For EXTENT_DATA_REF */
>> +
>> + /* Root who refers to this data extent */
> nit: s/who/which/
>> + u64 ref_root;
>> +
>> + /* Inode who refers to this data extent */
> nit: DITTO
>> + u64 ino;
>> +
>> + /*
>> + * file_offset - extent_offset
>> + *
>> + * file_offset is the key.offset of the EXTENT_DATA key.
>> + * extent_offset is btrfs_file_extent_offset() of the EXTENT_DATA data.
>> + */
>
> This needs rewording since it's rather cryptic now.
It's cryptic due to the EXTENT_ITEM design from the very beginning.
I'm all ears to improve this description.
> Looking at the dev
> docs and the description for 'offset' field in btrfs_file_extent_item I
> can sort of deduce that this field will only be different than null if
> this reference is for an extent which is shared between 2 snapshots.
Don't forget reflink and data CoW.
Like this:
item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
generation 6 type 1 (regular)
extent data disk byte 13631488 nr 1048576
extent data offset 0 nr 4096 ram 1048576
item 7 key (257 EXTENT_DATA 4096) itemoff 15760 itemsize 53
generation 7 type 1 (regular)
extent data disk byte 14680064 nr 4096
extent data offset 0 nr 4096 ram 4096
item 8 key (257 EXTENT_DATA 8192) itemoff 15707 itemsize 53
generation 6 type 1 (regular)
extent data disk byte 13631488 nr 1048576
extent data offset 8192 nr 1040384 ram 1048576
EXTENT_DATA items at 0 and 8K offset are original from one larger
extent, EXTENT_DATA item at 4K offset is newly written one.
But the current design makes EXTENT_ITEM inline data backref pretty clean:
item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16230 itemsize 53
refs 2 gen 6 flags DATA
extent data backref root FS_TREE objectid 257 offset 0
count 2
No need for an extra inline data backref, just increase the original
count from 1 to 2.
>
> So if file foo is shared between two snapshots, has 1 extent and in
> snapshot2 this extent is partially changed then I'd expect extent_offset
> to point to the start in the original (unchanged extent), correct?
As long as there is some new DATA_EXTENT pointing to the original
unchanged extent, then yes, the 'offset' will change.
Just like the EXTENT_DATA at 8K offset above.
>
>> + u64 offset;
>> +};
>> +
>> +struct btrfs_tree_ref {
>> + /*
>> + * Level of this tree block
>> + *
>> + * Shared for skinny (TREE_BLOCK_REF) and normal tree ref.
>
> This sentence is also not very clear? You mean this level applies to
> tree block refs (irrespective of whether they are shared or normal tree
> block refs)?
This is for any keyed or inlined tree ref who uses skinny metadata
(level stored in key.offset, the common case now) or non-skinny
EXTENT_ITEM who uses btrfs_tree_block_info like:
item 7 key (30507008 EXTENT_ITEM 16384) itemoff 15956 itemsize 51
refs 1 gen 4 flags TREE_BLOCK
tree block key (0 UNKNOWN.0 0) level 0 <<< here.
tree block backref root UUID_TREE
It's possible for extent tree to not include above cases, like the
following case:
item 1 key (12648448 EXTENT_ITEM 16384) itemoff 16235 itemsize 24
refs 9 gen 7 flags TREE_BLOCK
item 2 key (12648448 SHARED_BLOCK_REF 4481024) itemoff 3461
itemsize 0
shared block backref
So I'm not sure how to describe such case clearly.
Thanks,
Qu
>
>> + */
>> + int level;
>> +
>> + /*
>> + * Root who refers to this tree block.
>
> nit:s/who/which
>
>> + *
>> + * For TREE_BLOCK_REF (skinny metadata, either inline or keyed)
>> + */
>> + u64 root;
>> +
>> + /* For non-skinny metadata, no special member needed */
>> +};
>> +
>> +struct btrfs_ref {
>> + enum btrfs_ref_type type;
>> + int action;
>> +
>> + /*
>> + * Only use parent pointers as backref (SHARED_BLOCK_REF or
>> + * SHARED_DATA_REF) for this extent and its children.
>> + * Set for reloc trees.
>> + */
>> + bool only_backreferences:1;
>> +
>> + /*
>> + * Whether this extent should go through qgroup record.
>> + *
>> + * Normally false, but for certain case like delayed subtree scan,
>> + * setting this flag can hugely reduce qgroup overhead.
>> + */
>> + bool skip_qgroup:1;
>> +
>> + /*
>> + * Optional. To which root this modification is for.
>> + * Mostly used for qgroup optimization.
>> + *
>> + * When unset, data/tree ref init code will populate it.
>> + * In certain case, we're modifying reference for a different root.
>> + * E.g. Cow fs tree blocks for balance.
>> + * In that case, tree_ref::root will be fs tree, but we're doing this
>> + * for reloc tree, then we should set @real_root to reloc tree.
>> + */
>> + u64 real_root;
>> + u64 bytenr;
>> + u64 len;
>> +
>> + /* Bytenr of the parent tree block */
>> + u64 parent;
>> + union {
>> + struct btrfs_data_ref data_ref;
>> + struct btrfs_tree_ref tree_ref;
>> + };
>> +};
>> +
>> extern struct kmem_cache *btrfs_delayed_ref_head_cachep;
>> extern struct kmem_cache *btrfs_delayed_tree_ref_cachep;
>> extern struct kmem_cache *btrfs_delayed_data_ref_cachep;
>> @@ -195,6 +279,38 @@ extern struct kmem_cache *btrfs_delayed_extent_op_cachep;
>> int __init btrfs_delayed_ref_init(void);
>> void __cold btrfs_delayed_ref_exit(void);
>>
>> +static inline void btrfs_init_generic_ref(struct btrfs_ref *generic_ref,
>> + int action, u64 bytenr, u64 len, u64 parent)
>> +{
>> + generic_ref->action = action;
>> + generic_ref->bytenr = bytenr;
>> + generic_ref->len = len;
>> + generic_ref->parent = parent;
>> +}
>> +
>> +static inline void btrfs_init_tree_ref(struct btrfs_ref *generic_ref,
>> + int level, u64 root)
>> +{
>> + /* If @real_root not set, use @root as fallback */
>> + if (!generic_ref->real_root)
>> + generic_ref->real_root = root;
>> + generic_ref->tree_ref.level = level;
>> + generic_ref->tree_ref.root = root;
>> + generic_ref->type = BTRFS_REF_METADATA;
>> +}
>> +
>> +static inline void btrfs_init_data_ref(struct btrfs_ref *generic_ref,
>> + u64 ref_root, u64 ino, u64 offset)
>> +{
>> + /* If @real_root not set, use @root as fallback */
>> + if (!generic_ref->real_root)
>> + generic_ref->real_root = ref_root;
>> + generic_ref->data_ref.ref_root = ref_root;
>> + generic_ref->data_ref.ino = ino;
>> + generic_ref->data_ref.offset = offset;
>> + generic_ref->type = BTRFS_REF_DATA;
>> +}
>> +
>> static inline struct btrfs_delayed_extent_op *
>> btrfs_alloc_delayed_extent_op(void)
>> {
>>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-11 13:23 ` Qu Wenruo
@ 2019-02-11 14:20 ` Nikolay Borisov
2019-02-11 14:23 ` Qu Wenruo
0 siblings, 1 reply; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 14:20 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 15:23 ч., Qu Wenruo wrote:
>
>
> On 2019/2/11 下午8:55, Nikolay Borisov wrote:
>>
>>
>> On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
>>> Current delayed ref interface has several problems:
>>> - Longer and longer parameter lists
>>> bytenr
>>> num_bytes
>>> parent
>>> ---------- so far so good
>>> ref_root
>>> owner
>>> offset
>>> ---------- I don't feel good now
>>>
>>> - Different interpretation for the same parameter
>>> Above @owner for data ref is inode number (u64),
>>> while for tree ref, it's level (int).
>>>
>>> They are even in different size range.
>>> For level we only need 0~8, while for ino it's
>>> BTRFS_FIRST_FREE_OBJECTID~BTRFS_LAST_FREE_OBJECTID.
>>>
>>> And @offset doesn't even makes sense for tree ref.
>>>
>>> Such parameter reuse may look clever as an hidden union, but it
>>> destroys code readability.
>>>
>>> To solve both problems, we introduce a new structure, btrfs_ref to solve
>>> them:
>>>
>>> - Structure instead of long parameter list
>>> This makes later expansion easier, and better documented.
>>>
>>> - Use btrfs_ref::type to distinguish data and tree ref
>>>
>>> - Use proper union to store data/tree ref specific structures.
>>>
>>> - Use separate functions to fill data/tree ref data, with a common generic
>>> function to fill common bytenr/num_bytes members.
>>>
>>> All parameters will find its place in btrfs_ref, and an extra member,
>>> @real_root, inspired by ref-verify code, is newly introduced for later
>>> qgroup code, to record which tree is triggered this extent modification.
>>>
>>> This patch doesn't touch any code, but provides the basis for incoming
>>> refactors.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>> fs/btrfs/delayed-ref.h | 116 +++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 116 insertions(+)
>>>
>>> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
>>> index d2af974f68a1..24addc5163bc 100644
>>> --- a/fs/btrfs/delayed-ref.h
>>> +++ b/fs/btrfs/delayed-ref.h
>>> @@ -187,6 +187,90 @@ struct btrfs_delayed_ref_root {
>>> u64 qgroup_to_skip;
>>> };
>>>
>>> +enum btrfs_ref_type {
>>> + BTRFS_REF_NOT_SET,
>>> + BTRFS_REF_DATA,
>>> + BTRFS_REF_METADATA,
>>> + BTRFS_REF_LAST,
>>> +};
>>> +
>>> +struct btrfs_data_ref {
>>> + /* For EXTENT_DATA_REF */
>>> +
>>> + /* Root who refers to this data extent */
>> nit: s/who/which/
>>> + u64 ref_root;
>>> +
>>> + /* Inode who refers to this data extent */
>> nit: DITTO
>>> + u64 ino;
>>> +
>>> + /*
>>> + * file_offset - extent_offset
>>> + *
>>> + * file_offset is the key.offset of the EXTENT_DATA key.
>>> + * extent_offset is btrfs_file_extent_offset() of the EXTENT_DATA data.
>>> + */
>>
>> This needs rewording since it's rather cryptic now.
>
> It's cryptic due to the EXTENT_ITEM design from the very beginning.
> I'm all ears to improve this description.
>
>> Looking at the dev
>> docs and the description for 'offset' field in btrfs_file_extent_item I
>> can sort of deduce that this field will only be different than null if
>> this reference is for an extent which is shared between 2 snapshots.
>
> Don't forget reflink and data CoW.
>
> Like this:
>
> item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
> generation 6 type 1 (regular)
> extent data disk byte 13631488 nr 1048576
> extent data offset 0 nr 4096 ram 1048576
> item 7 key (257 EXTENT_DATA 4096) itemoff 15760 itemsize 53
> generation 7 type 1 (regular)
> extent data disk byte 14680064 nr 4096
> extent data offset 0 nr 4096 ram 4096
> item 8 key (257 EXTENT_DATA 8192) itemoff 15707 itemsize 53
> generation 6 type 1 (regular)
> extent data disk byte 13631488 nr 1048576
> extent data offset 8192 nr 1040384 ram 1048576
>
> EXTENT_DATA items at 0 and 8K offset are original from one larger
> extent, EXTENT_DATA item at 4K offset is newly written one.
Okay this makes sense, however if we take item 8 being inserted then
according to the comments, the 'offset' member for this data ref will be
0 since 8k (from key.offset) - 8k (from btrfs_file_extent_offset)? WHy
is that, shouldn't the offset here be 8k rather than 0?
>
> But the current design makes EXTENT_ITEM inline data backref pretty clean:
>
> item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16230 itemsize 53
> refs 2 gen 6 flags DATA
> extent data backref root FS_TREE objectid 257 offset 0
> count 2
>
> No need for an extra inline data backref, just increase the original
> count from 1 to 2.
>>
>> So if file foo is shared between two snapshots, has 1 extent and in
>> snapshot2 this extent is partially changed then I'd expect extent_offset
>> to point to the start in the original (unchanged extent), correct?
>
> As long as there is some new DATA_EXTENT pointing to the original
> unchanged extent, then yes, the 'offset' will change.
>
> Just like the EXTENT_DATA at 8K offset above.
>
>>
>>> + u64 offset;
>>> +};
>>> +
>>> +struct btrfs_tree_ref {
>>> + /*
>>> + * Level of this tree block
>>> + *
>>> + * Shared for skinny (TREE_BLOCK_REF) and normal tree ref.
>>
>> This sentence is also not very clear? You mean this level applies to
>> tree block refs (irrespective of whether they are shared or normal tree
>> block refs)?
>
> This is for any keyed or inlined tree ref who uses skinny metadata
> (level stored in key.offset, the common case now) or non-skinny
> EXTENT_ITEM who uses btrfs_tree_block_info like:
>
> item 7 key (30507008 EXTENT_ITEM 16384) itemoff 15956 itemsize 51
> refs 1 gen 4 flags TREE_BLOCK
> tree block key (0 UNKNOWN.0 0) level 0 <<< here.
> tree block backref root UUID_TREE
>
>
> It's possible for extent tree to not include above cases, like the
> following case:
> item 1 key (12648448 EXTENT_ITEM 16384) itemoff 16235 itemsize 24
> refs 9 gen 7 flags TREE_BLOCK
> item 2 key (12648448 SHARED_BLOCK_REF 4481024) itemoff 3461
> itemsize 0
> shared block backref
>
> So I'm not sure how to describe such case clearly.
>
> Thanks,
> Qu
>
>>
>>> + */
>>> + int level;
>>> +
>>> + /*
>>> + * Root who refers to this tree block.
>>
>> nit:s/who/which
>>
>>> + *
>>> + * For TREE_BLOCK_REF (skinny metadata, either inline or keyed)
>>> + */
>>> + u64 root;
>>> +
>>> + /* For non-skinny metadata, no special member needed */
>>> +};
>>> +
>>> +struct btrfs_ref {
>>> + enum btrfs_ref_type type;
>>> + int action;
>>> +
>>> + /*
>>> + * Only use parent pointers as backref (SHARED_BLOCK_REF or
>>> + * SHARED_DATA_REF) for this extent and its children.
>>> + * Set for reloc trees.
>>> + */
>>> + bool only_backreferences:1;
>>> +
>>> + /*
>>> + * Whether this extent should go through qgroup record.
>>> + *
>>> + * Normally false, but for certain case like delayed subtree scan,
>>> + * setting this flag can hugely reduce qgroup overhead.
>>> + */
>>> + bool skip_qgroup:1;
>>> +
>>> + /*
>>> + * Optional. To which root this modification is for.
>>> + * Mostly used for qgroup optimization.
>>> + *
>>> + * When unset, data/tree ref init code will populate it.
>>> + * In certain case, we're modifying reference for a different root.
>>> + * E.g. Cow fs tree blocks for balance.
>>> + * In that case, tree_ref::root will be fs tree, but we're doing this
>>> + * for reloc tree, then we should set @real_root to reloc tree.
>>> + */
>>> + u64 real_root;
>>> + u64 bytenr;
>>> + u64 len;
>>> +
>>> + /* Bytenr of the parent tree block */
>>> + u64 parent;
>>> + union {
>>> + struct btrfs_data_ref data_ref;
>>> + struct btrfs_tree_ref tree_ref;
>>> + };
>>> +};
>>> +
>>> extern struct kmem_cache *btrfs_delayed_ref_head_cachep;
>>> extern struct kmem_cache *btrfs_delayed_tree_ref_cachep;
>>> extern struct kmem_cache *btrfs_delayed_data_ref_cachep;
>>> @@ -195,6 +279,38 @@ extern struct kmem_cache *btrfs_delayed_extent_op_cachep;
>>> int __init btrfs_delayed_ref_init(void);
>>> void __cold btrfs_delayed_ref_exit(void);
>>>
>>> +static inline void btrfs_init_generic_ref(struct btrfs_ref *generic_ref,
>>> + int action, u64 bytenr, u64 len, u64 parent)
>>> +{
>>> + generic_ref->action = action;
>>> + generic_ref->bytenr = bytenr;
>>> + generic_ref->len = len;
>>> + generic_ref->parent = parent;
>>> +}
>>> +
>>> +static inline void btrfs_init_tree_ref(struct btrfs_ref *generic_ref,
>>> + int level, u64 root)
>>> +{
>>> + /* If @real_root not set, use @root as fallback */
>>> + if (!generic_ref->real_root)
>>> + generic_ref->real_root = root;
>>> + generic_ref->tree_ref.level = level;
>>> + generic_ref->tree_ref.root = root;
>>> + generic_ref->type = BTRFS_REF_METADATA;
>>> +}
>>> +
>>> +static inline void btrfs_init_data_ref(struct btrfs_ref *generic_ref,
>>> + u64 ref_root, u64 ino, u64 offset)
>>> +{
>>> + /* If @real_root not set, use @root as fallback */
>>> + if (!generic_ref->real_root)
>>> + generic_ref->real_root = ref_root;
>>> + generic_ref->data_ref.ref_root = ref_root;
>>> + generic_ref->data_ref.ino = ino;
>>> + generic_ref->data_ref.offset = offset;
>>> + generic_ref->type = BTRFS_REF_DATA;
>>> +}
>>> +
>>> static inline struct btrfs_delayed_extent_op *
>>> btrfs_alloc_delayed_extent_op(void)
>>> {
>>>
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-11 14:20 ` Nikolay Borisov
@ 2019-02-11 14:23 ` Qu Wenruo
2019-02-18 5:00 ` Qu Wenruo
0 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 14:23 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
[snip]
>>> Looking at the dev
>>> docs and the description for 'offset' field in btrfs_file_extent_item I
>>> can sort of deduce that this field will only be different than null if
>>> this reference is for an extent which is shared between 2 snapshots.
>>
>> Don't forget reflink and data CoW.
>>
>> Like this:
>>
>> item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
>> generation 6 type 1 (regular)
>> extent data disk byte 13631488 nr 1048576
>> extent data offset 0 nr 4096 ram 1048576
>> item 7 key (257 EXTENT_DATA 4096) itemoff 15760 itemsize 53
>> generation 7 type 1 (regular)
>> extent data disk byte 14680064 nr 4096
>> extent data offset 0 nr 4096 ram 4096
>> item 8 key (257 EXTENT_DATA 8192) itemoff 15707 itemsize 53
>> generation 6 type 1 (regular)
>> extent data disk byte 13631488 nr 1048576
>> extent data offset 8192 nr 1040384 ram 1048576
>>
>> EXTENT_DATA items at 0 and 8K offset are original from one larger
>> extent, EXTENT_DATA item at 4K offset is newly written one.
>
> Okay this makes sense, however if we take item 8 being inserted then
> according to the comments, the 'offset' member for this data ref will be
> 0 since 8k (from key.offset) - 8k (from btrfs_file_extent_offset)? WHy
> is that, shouldn't the offset here be 8k rather than 0?
To avoid creating a new data backref item.
I don't like this idea too, it makes btrfs check, especially lowmem
mode, pretty slow.
If I'm going to re-design the on-disk format, this is definitely going
to disappear.
But the design is already here for a long long time, even it caused
problems before, we still need to follow the behavior.
Thanks,
Qu
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-11 14:23 ` Qu Wenruo
@ 2019-02-18 5:00 ` Qu Wenruo
2019-02-18 6:59 ` Su Yue
0 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-18 5:00 UTC (permalink / raw)
To: Qu Wenruo, Nikolay Borisov, linux-btrfs
On 2019/2/11 下午10:23, Qu Wenruo wrote:
> [snip]
>>>> Looking at the dev
>>>> docs and the description for 'offset' field in btrfs_file_extent_item I
>>>> can sort of deduce that this field will only be different than null if
>>>> this reference is for an extent which is shared between 2 snapshots.
>>>
>>> Don't forget reflink and data CoW.
>>>
>>> Like this:
>>>
>>> item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
>>> generation 6 type 1 (regular)
>>> extent data disk byte 13631488 nr 1048576
>>> extent data offset 0 nr 4096 ram 1048576
>>> item 7 key (257 EXTENT_DATA 4096) itemoff 15760 itemsize 53
>>> generation 7 type 1 (regular)
>>> extent data disk byte 14680064 nr 4096
>>> extent data offset 0 nr 4096 ram 4096
>>> item 8 key (257 EXTENT_DATA 8192) itemoff 15707 itemsize 53
>>> generation 6 type 1 (regular)
>>> extent data disk byte 13631488 nr 1048576
>>> extent data offset 8192 nr 1040384 ram 1048576
>>>
>>> EXTENT_DATA items at 0 and 8K offset are original from one larger
>>> extent, EXTENT_DATA item at 4K offset is newly written one.
>>
>> Okay this makes sense, however if we take item 8 being inserted then
>> according to the comments, the 'offset' member for this data ref will be
>> 0 since 8k (from key.offset) - 8k (from btrfs_file_extent_offset)? WHy
>> is that, shouldn't the offset here be 8k rather than 0?
>
> To avoid creating a new data backref item.
>
> I don't like this idea too, it makes btrfs check, especially lowmem
> mode, pretty slow.
>
> If I'm going to re-design the on-disk format, this is definitely going
> to disappear.
> But the design is already here for a long long time, even it caused
> problems before, we still need to follow the behavior.
Is there any extra suggestion on the wording about the anti-initiative
offset used in data backref?
Thanks,
Qu
>
> Thanks,
> Qu
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures
2019-02-18 5:00 ` Qu Wenruo
@ 2019-02-18 6:59 ` Su Yue
0 siblings, 0 replies; 24+ messages in thread
From: Su Yue @ 2019-02-18 6:59 UTC (permalink / raw)
To: Qu Wenruo, Qu Wenruo, Nikolay Borisov, linux-btrfs
On 2/18/19 1:00 PM, Qu Wenruo wrote:
>
>
> On 2019/2/11 下午10:23, Qu Wenruo wrote:
>> [snip]
>>>>> Looking at the dev
>>>>> docs and the description for 'offset' field in btrfs_file_extent_item I
>>>>> can sort of deduce that this field will only be different than null if
>>>>> this reference is for an extent which is shared between 2 snapshots.
>>>>
>>>> Don't forget reflink and data CoW.
>>>>
>>>> Like this:
>>>>
>>>> item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
>>>> generation 6 type 1 (regular)
>>>> extent data disk byte 13631488 nr 1048576
>>>> extent data offset 0 nr 4096 ram 1048576
>>>> item 7 key (257 EXTENT_DATA 4096) itemoff 15760 itemsize 53
>>>> generation 7 type 1 (regular)
>>>> extent data disk byte 14680064 nr 4096
>>>> extent data offset 0 nr 4096 ram 4096
>>>> item 8 key (257 EXTENT_DATA 8192) itemoff 15707 itemsize 53
>>>> generation 6 type 1 (regular)
>>>> extent data disk byte 13631488 nr 1048576
>>>> extent data offset 8192 nr 1040384 ram 1048576
>>>>
>>>> EXTENT_DATA items at 0 and 8K offset are original from one larger
>>>> extent, EXTENT_DATA item at 4K offset is newly written one.
>>>
>>> Okay this makes sense, however if we take item 8 being inserted then
>>> according to the comments, the 'offset' member for this data ref will be
>>> 0 since 8k (from key.offset) - 8k (from btrfs_file_extent_offset)? WHy
>>> is that, shouldn't the offset here be 8k rather than 0?
>>
>> To avoid creating a new data backref item.
>>
>> I don't like this idea too, it makes btrfs check, especially lowmem
>> mode, pretty slow.
>>
>> If I'm going to re-design the on-disk format, this is definitely going
>> to disappear.
>> But the design is already here for a long long time, even it caused
>> problems before, we still need to follow the behavior.
>
> Is there any extra suggestion on the wording about the anti-initiative
> offset used in data backref?
>
My personal suggestion is doing simplification first like your patches,
then to discuss "design" in detail.
> Thanks,
> Qu
>
>>
>> Thanks,
>> Qu
>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 2/9] btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 3/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref() Qu Wenruo
` (7 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov, Johannes Thumshirn
The process_func function pointer is local to __btrfs_mod_ref() and
points to either btrfs_inc_extent_ref() or btrfs_free_extent().
Open code it to make later delayed ref refactor easier, so we can
refactor btrfs_inc_extent_ref() and btrfs_free_extent() in different
patches.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
---
fs/btrfs/extent-tree.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 9f012c2facbe..606187ede47b 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3252,10 +3252,6 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
int i;
int level;
int ret = 0;
- int (*process_func)(struct btrfs_trans_handle *,
- struct btrfs_root *,
- u64, u64, u64, u64, u64, u64);
-
if (btrfs_is_testing(fs_info))
return 0;
@@ -3267,11 +3263,6 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state) && level == 0)
return 0;
- if (inc)
- process_func = btrfs_inc_extent_ref;
- else
- process_func = btrfs_free_extent;
-
if (full_backref)
parent = buf->start;
else
@@ -3293,16 +3284,27 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
num_bytes = btrfs_file_extent_disk_num_bytes(buf, fi);
key.offset -= btrfs_file_extent_offset(buf, fi);
- ret = process_func(trans, root, bytenr, num_bytes,
- parent, ref_root, key.objectid,
- key.offset);
+ if (inc)
+ ret = btrfs_inc_extent_ref(trans, root, bytenr,
+ num_bytes, parent, ref_root,
+ key.objectid, key.offset);
+ else
+ ret = btrfs_free_extent(trans, root, bytenr,
+ num_bytes, parent, ref_root,
+ key.objectid, key.offset);
if (ret)
goto fail;
} else {
bytenr = btrfs_node_blockptr(buf, i);
num_bytes = fs_info->nodesize;
- ret = process_func(trans, root, bytenr, num_bytes,
- parent, ref_root, level - 1, 0);
+ if (inc)
+ ret = btrfs_inc_extent_ref(trans, root, bytenr,
+ num_bytes, parent, ref_root,
+ level - 1, 0);
+ else
+ ret = btrfs_free_extent(trans, root, bytenr,
+ num_bytes, parent, ref_root,
+ level - 1, 0);
if (ret)
goto fail;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 3/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref()
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 1/9] btrfs: delayed-ref: Introduce better documented delayed ref structures Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 2/9] btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 12:58 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 4/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref() Qu Wenruo
` (6 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
btrfs_add_delayed_tree_ref() has a longer and longer parameter list, and
some caller like btrfs_inc_extent_ref() are using @owner as level for
delayed tree ref.
Instead of making the parameter list longer and longer, use btrfs_ref to
refactor it, so each parameter assignment should be self-explaining
without dirty level/owner trick, and provides the basis for later refactor.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/delayed-ref.c | 24 +++++++++++++++--------
fs/btrfs/delayed-ref.h | 3 +--
fs/btrfs/extent-tree.c | 44 +++++++++++++++++++++++++-----------------
3 files changed, 43 insertions(+), 28 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index cad36c99a483..5c864ce2f959 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -742,8 +742,7 @@ static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
* transaction commits.
*/
int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
- u64 bytenr, u64 num_bytes, u64 parent,
- u64 ref_root, int level, int action,
+ struct btrfs_ref *generic_ref,
struct btrfs_delayed_extent_op *extent_op,
int *old_ref_mod, int *new_ref_mod)
{
@@ -753,10 +752,16 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
struct btrfs_delayed_ref_root *delayed_refs;
struct btrfs_qgroup_extent_record *record = NULL;
int qrecord_inserted;
- bool is_system = (ref_root == BTRFS_CHUNK_TREE_OBJECTID);
+ bool is_system = (generic_ref->real_root == BTRFS_CHUNK_TREE_OBJECTID);
+ int action = generic_ref->action;
+ int level = generic_ref->tree_ref.level;
int ret;
+ u64 bytenr = generic_ref->bytenr;
+ u64 num_bytes = generic_ref->len;
+ u64 parent = generic_ref->parent;
u8 ref_type;
+ ASSERT(generic_ref->type == BTRFS_REF_METADATA && generic_ref->action);
BUG_ON(extent_op && extent_op->is_data);
ref = kmem_cache_alloc(btrfs_delayed_tree_ref_cachep, GFP_NOFS);
if (!ref)
@@ -769,8 +774,10 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
}
if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags) &&
- is_fstree(ref_root)) {
- record = kmalloc(sizeof(*record), GFP_NOFS);
+ is_fstree(generic_ref->real_root) &&
+ is_fstree(generic_ref->tree_ref.root) &&
+ !generic_ref->skip_qgroup) {
+ record = kzalloc(sizeof(*record), GFP_NOFS);
if (!record) {
kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref);
kmem_cache_free(btrfs_delayed_ref_head_cachep, head_ref);
@@ -784,13 +791,14 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
ref_type = BTRFS_TREE_BLOCK_REF_KEY;
init_delayed_ref_common(fs_info, &ref->node, bytenr, num_bytes,
- ref_root, action, ref_type);
- ref->root = ref_root;
+ generic_ref->tree_ref.root, action, ref_type);
+ ref->root = generic_ref->tree_ref.root;
ref->parent = parent;
ref->level = level;
init_delayed_ref_head(head_ref, record, bytenr, num_bytes,
- ref_root, 0, action, false, is_system);
+ generic_ref->tree_ref.root, 0, action, false,
+ is_system);
head_ref->extent_op = extent_op;
delayed_refs = &trans->transaction->delayed_refs;
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index 24addc5163bc..70bb080bf3ef 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -351,8 +351,7 @@ static inline void btrfs_put_delayed_ref_head(struct btrfs_delayed_ref_head *hea
}
int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
- u64 bytenr, u64 num_bytes, u64 parent,
- u64 ref_root, int level, int action,
+ struct btrfs_ref *generic_ref,
struct btrfs_delayed_extent_op *extent_op,
int *old_ref_mod, int *new_ref_mod);
int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 606187ede47b..b01fad9a56ff 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2048,6 +2048,7 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
u64 root_objectid, u64 owner, u64 offset)
{
struct btrfs_fs_info *fs_info = root->fs_info;
+ struct btrfs_ref generic_ref = { 0 };
int old_ref_mod, new_ref_mod;
int ret;
@@ -2057,12 +2058,13 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, root_objectid,
owner, offset, BTRFS_ADD_DELAYED_REF);
+ btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
+ num_bytes, parent);
+ generic_ref.real_root = root->root_key.objectid;
if (owner < BTRFS_FIRST_FREE_OBJECTID) {
- ret = btrfs_add_delayed_tree_ref(trans, bytenr,
- num_bytes, parent,
- root_objectid, (int)owner,
- BTRFS_ADD_DELAYED_REF, NULL,
- &old_ref_mod, &new_ref_mod);
+ btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
+ ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
+ NULL, &old_ref_mod, &new_ref_mod);
} else {
ret = btrfs_add_delayed_data_ref(trans, bytenr,
num_bytes, parent,
@@ -7277,9 +7279,15 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
u64 parent, int last_ref)
{
struct btrfs_fs_info *fs_info = root->fs_info;
+ struct btrfs_ref generic_ref = { 0 };
int pin = 1;
int ret;
+ btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF,
+ buf->start, buf->len, parent);
+ btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf),
+ root->root_key.objectid);
+
if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
int old_ref_mod, new_ref_mod;
@@ -7287,11 +7295,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
root->root_key.objectid,
btrfs_header_level(buf), 0,
BTRFS_DROP_DELAYED_REF);
- ret = btrfs_add_delayed_tree_ref(trans, buf->start,
- buf->len, parent,
- root->root_key.objectid,
- btrfs_header_level(buf),
- BTRFS_DROP_DELAYED_REF, NULL,
+ ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
&old_ref_mod, &new_ref_mod);
BUG_ON(ret); /* -ENOMEM */
pin = old_ref_mod >= 0 && new_ref_mod < 0;
@@ -7344,6 +7348,7 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
u64 owner, u64 offset)
{
struct btrfs_fs_info *fs_info = root->fs_info;
+ struct btrfs_ref generic_ref = { 0 };
int old_ref_mod, new_ref_mod;
int ret;
@@ -7355,6 +7360,9 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
root_objectid, owner, offset,
BTRFS_DROP_DELAYED_REF);
+ btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF, bytenr,
+ num_bytes, parent);
+ generic_ref.real_root = root->root_key.objectid;
/*
* tree log blocks never actually go into the extent allocation
* tree, just update pinning info and exit early.
@@ -7366,10 +7374,8 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
old_ref_mod = new_ref_mod = 0;
ret = 0;
} else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
- ret = btrfs_add_delayed_tree_ref(trans, bytenr,
- num_bytes, parent,
- root_objectid, (int)owner,
- BTRFS_DROP_DELAYED_REF, NULL,
+ btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
+ ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
&old_ref_mod, &new_ref_mod);
} else {
ret = btrfs_add_delayed_data_ref(trans, bytenr,
@@ -8687,6 +8693,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
struct btrfs_block_rsv *block_rsv;
struct extent_buffer *buf;
struct btrfs_delayed_extent_op *extent_op;
+ struct btrfs_ref generic_ref = { 0 };
u64 flags = 0;
int ret;
u32 blocksize = fs_info->nodesize;
@@ -8744,10 +8751,11 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
btrfs_ref_tree_mod(root, ins.objectid, ins.offset, parent,
root_objectid, level, 0,
BTRFS_ADD_DELAYED_EXTENT);
- ret = btrfs_add_delayed_tree_ref(trans, ins.objectid,
- ins.offset, parent,
- root_objectid, level,
- BTRFS_ADD_DELAYED_EXTENT,
+ btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
+ ins.objectid, ins.offset, parent);
+ generic_ref.real_root = root->root_key.objectid;
+ btrfs_init_tree_ref(&generic_ref, level, root_objectid);
+ ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
extent_op, NULL, NULL);
if (ret)
goto out_free_delayed;
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 3/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref()
2019-02-11 5:16 ` [PATCH v3 3/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref() Qu Wenruo
@ 2019-02-11 12:58 ` Nikolay Borisov
0 siblings, 0 replies; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 12:58 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
> btrfs_add_delayed_tree_ref() has a longer and longer parameter list, and
> some caller like btrfs_inc_extent_ref() are using @owner as level for
> delayed tree ref.
>
> Instead of making the parameter list longer and longer, use btrfs_ref to
> refactor it, so each parameter assignment should be self-explaining
> without dirty level/owner trick, and provides the basis for later refactor.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/delayed-ref.c | 24 +++++++++++++++--------
> fs/btrfs/delayed-ref.h | 3 +--
> fs/btrfs/extent-tree.c | 44 +++++++++++++++++++++++++-----------------
> 3 files changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index cad36c99a483..5c864ce2f959 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -742,8 +742,7 @@ static void init_delayed_ref_common(struct btrfs_fs_info *fs_info,
> * transaction commits.
> */
> int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> - u64 bytenr, u64 num_bytes, u64 parent,
> - u64 ref_root, int level, int action,
> + struct btrfs_ref *generic_ref,
> struct btrfs_delayed_extent_op *extent_op,
> int *old_ref_mod, int *new_ref_mod)
> {
> @@ -753,10 +752,16 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> struct btrfs_delayed_ref_root *delayed_refs;
> struct btrfs_qgroup_extent_record *record = NULL;
> int qrecord_inserted;
> - bool is_system = (ref_root == BTRFS_CHUNK_TREE_OBJECTID);
> + bool is_system = (generic_ref->real_root == BTRFS_CHUNK_TREE_OBJECTID);
> + int action = generic_ref->action;
> + int level = generic_ref->tree_ref.level;
> int ret;
> + u64 bytenr = generic_ref->bytenr;
> + u64 num_bytes = generic_ref->len;
> + u64 parent = generic_ref->parent;
> u8 ref_type;
>
> + ASSERT(generic_ref->type == BTRFS_REF_METADATA && generic_ref->action);
> BUG_ON(extent_op && extent_op->is_data);
> ref = kmem_cache_alloc(btrfs_delayed_tree_ref_cachep, GFP_NOFS);
> if (!ref)
> @@ -769,8 +774,10 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> }
>
> if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags) &&
> - is_fstree(ref_root)) {
> - record = kmalloc(sizeof(*record), GFP_NOFS);
> + is_fstree(generic_ref->real_root) &&
> + is_fstree(generic_ref->tree_ref.root) &&
> + !generic_ref->skip_qgroup) {
> + record = kzalloc(sizeof(*record), GFP_NOFS);
> if (!record) {
> kmem_cache_free(btrfs_delayed_tree_ref_cachep, ref);
> kmem_cache_free(btrfs_delayed_ref_head_cachep, head_ref);
> @@ -784,13 +791,14 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> ref_type = BTRFS_TREE_BLOCK_REF_KEY;
>
> init_delayed_ref_common(fs_info, &ref->node, bytenr, num_bytes,
> - ref_root, action, ref_type);
> - ref->root = ref_root;
> + generic_ref->tree_ref.root, action, ref_type);
> + ref->root = generic_ref->tree_ref.root;
> ref->parent = parent;
> ref->level = level;
>
> init_delayed_ref_head(head_ref, record, bytenr, num_bytes,
> - ref_root, 0, action, false, is_system);
> + generic_ref->tree_ref.root, 0, action, false,
> + is_system);
> head_ref->extent_op = extent_op;
>
> delayed_refs = &trans->transaction->delayed_refs;
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index 24addc5163bc..70bb080bf3ef 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -351,8 +351,7 @@ static inline void btrfs_put_delayed_ref_head(struct btrfs_delayed_ref_head *hea
> }
>
> int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> - u64 bytenr, u64 num_bytes, u64 parent,
> - u64 ref_root, int level, int action,
> + struct btrfs_ref *generic_ref,
> struct btrfs_delayed_extent_op *extent_op,
> int *old_ref_mod, int *new_ref_mod);
> int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 606187ede47b..b01fad9a56ff 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2048,6 +2048,7 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> u64 root_objectid, u64 owner, u64 offset)
> {
> struct btrfs_fs_info *fs_info = root->fs_info;
> + struct btrfs_ref generic_ref = { 0 };
> int old_ref_mod, new_ref_mod;
> int ret;
>
> @@ -2057,12 +2058,13 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, root_objectid,
> owner, offset, BTRFS_ADD_DELAYED_REF);
>
> + btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
> + num_bytes, parent);
> + generic_ref.real_root = root->root_key.objectid;
> if (owner < BTRFS_FIRST_FREE_OBJECTID) {
> - ret = btrfs_add_delayed_tree_ref(trans, bytenr,
> - num_bytes, parent,
> - root_objectid, (int)owner,
> - BTRFS_ADD_DELAYED_REF, NULL,
> - &old_ref_mod, &new_ref_mod);
> + btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
> + ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
> + NULL, &old_ref_mod, &new_ref_mod);
> } else {
> ret = btrfs_add_delayed_data_ref(trans, bytenr,
> num_bytes, parent,
> @@ -7277,9 +7279,15 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> u64 parent, int last_ref)
> {
> struct btrfs_fs_info *fs_info = root->fs_info;
> + struct btrfs_ref generic_ref = { 0 };
> int pin = 1;
> int ret;
>
> + btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF,
> + buf->start, buf->len, parent);
> + btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf),
> + root->root_key.objectid);
> +
> if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
> int old_ref_mod, new_ref_mod;
>
> @@ -7287,11 +7295,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> root->root_key.objectid,
> btrfs_header_level(buf), 0,
> BTRFS_DROP_DELAYED_REF);
> - ret = btrfs_add_delayed_tree_ref(trans, buf->start,
> - buf->len, parent,
> - root->root_key.objectid,
> - btrfs_header_level(buf),
> - BTRFS_DROP_DELAYED_REF, NULL,
> + ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
> &old_ref_mod, &new_ref_mod);
> BUG_ON(ret); /* -ENOMEM */
> pin = old_ref_mod >= 0 && new_ref_mod < 0;
> @@ -7344,6 +7348,7 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
> u64 owner, u64 offset)
> {
> struct btrfs_fs_info *fs_info = root->fs_info;
> + struct btrfs_ref generic_ref = { 0 };
> int old_ref_mod, new_ref_mod;
> int ret;
>
> @@ -7355,6 +7360,9 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
> root_objectid, owner, offset,
> BTRFS_DROP_DELAYED_REF);
>
> + btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF, bytenr,
> + num_bytes, parent);
> + generic_ref.real_root = root->root_key.objectid;
> /*
> * tree log blocks never actually go into the extent allocation
> * tree, just update pinning info and exit early.
> @@ -7366,10 +7374,8 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
> old_ref_mod = new_ref_mod = 0;
> ret = 0;
> } else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
> - ret = btrfs_add_delayed_tree_ref(trans, bytenr,
> - num_bytes, parent,
> - root_objectid, (int)owner,
> - BTRFS_DROP_DELAYED_REF, NULL,
> + btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
> + ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
> &old_ref_mod, &new_ref_mod);
> } else {
> ret = btrfs_add_delayed_data_ref(trans, bytenr,
> @@ -8687,6 +8693,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
> struct btrfs_block_rsv *block_rsv;
> struct extent_buffer *buf;
> struct btrfs_delayed_extent_op *extent_op;
> + struct btrfs_ref generic_ref = { 0 };
> u64 flags = 0;
> int ret;
> u32 blocksize = fs_info->nodesize;
> @@ -8744,10 +8751,11 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
> btrfs_ref_tree_mod(root, ins.objectid, ins.offset, parent,
> root_objectid, level, 0,
> BTRFS_ADD_DELAYED_EXTENT);
> - ret = btrfs_add_delayed_tree_ref(trans, ins.objectid,
> - ins.offset, parent,
> - root_objectid, level,
> - BTRFS_ADD_DELAYED_EXTENT,
> + btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
> + ins.objectid, ins.offset, parent);
> + generic_ref.real_root = root->root_key.objectid;
> + btrfs_init_tree_ref(&generic_ref, level, root_objectid);
> + ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
> extent_op, NULL, NULL);
> if (ret)
> goto out_free_delayed;
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 4/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref()
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (2 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 3/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_tree_ref() Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 12:59 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 5/9] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod() Qu Wenruo
` (5 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
Just like btrfs_add_delayed_tree_ref(), use btrfs_ref to refactor
btrfs_add_delayed_data_ref().
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/delayed-ref.c | 20 ++++++++++++++------
fs/btrfs/delayed-ref.h | 7 +++----
fs/btrfs/extent-tree.c | 23 ++++++++++-------------
3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 5c864ce2f959..d4245ccd1afb 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -837,10 +837,9 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
* add a delayed data ref. it's similar to btrfs_add_delayed_tree_ref.
*/
int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
- u64 bytenr, u64 num_bytes,
- u64 parent, u64 ref_root,
- u64 owner, u64 offset, u64 reserved, int action,
- int *old_ref_mod, int *new_ref_mod)
+ struct btrfs_ref *generic_ref,
+ u64 reserved, int *old_ref_mod,
+ int *new_ref_mod)
{
struct btrfs_fs_info *fs_info = trans->fs_info;
struct btrfs_delayed_data_ref *ref;
@@ -848,9 +847,17 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
struct btrfs_delayed_ref_root *delayed_refs;
struct btrfs_qgroup_extent_record *record = NULL;
int qrecord_inserted;
+ int action = generic_ref->action;
int ret;
+ u64 bytenr = generic_ref->bytenr;
+ u64 num_bytes = generic_ref->len;
+ u64 parent = generic_ref->parent;
+ u64 ref_root = generic_ref->data_ref.ref_root;
+ u64 owner = generic_ref->data_ref.ino;
+ u64 offset = generic_ref->data_ref.offset;
u8 ref_type;
+ ASSERT(generic_ref->type == BTRFS_REF_DATA && action);
ref = kmem_cache_alloc(btrfs_delayed_data_ref_cachep, GFP_NOFS);
if (!ref)
return -ENOMEM;
@@ -874,8 +881,9 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
}
if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags) &&
- is_fstree(ref_root)) {
- record = kmalloc(sizeof(*record), GFP_NOFS);
+ is_fstree(ref_root) && is_fstree(generic_ref->real_root) &&
+ !generic_ref->skip_qgroup) {
+ record = kzalloc(sizeof(*record), GFP_NOFS);
if (!record) {
kmem_cache_free(btrfs_delayed_data_ref_cachep, ref);
kmem_cache_free(btrfs_delayed_ref_head_cachep,
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index 70bb080bf3ef..9037a7de18b7 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -355,10 +355,9 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
struct btrfs_delayed_extent_op *extent_op,
int *old_ref_mod, int *new_ref_mod);
int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
- u64 bytenr, u64 num_bytes,
- u64 parent, u64 ref_root,
- u64 owner, u64 offset, u64 reserved, int action,
- int *old_ref_mod, int *new_ref_mod);
+ struct btrfs_ref *generic_ref,
+ u64 reserved, int *old_ref_mod,
+ int *new_ref_mod);
int btrfs_add_delayed_extent_op(struct btrfs_fs_info *fs_info,
struct btrfs_trans_handle *trans,
u64 bytenr, u64 num_bytes,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index b01fad9a56ff..36516b33d07f 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2066,10 +2066,8 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
NULL, &old_ref_mod, &new_ref_mod);
} else {
- ret = btrfs_add_delayed_data_ref(trans, bytenr,
- num_bytes, parent,
- root_objectid, owner, offset,
- 0, BTRFS_ADD_DELAYED_REF,
+ btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
+ ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
&old_ref_mod, &new_ref_mod);
}
@@ -7378,10 +7376,8 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
&old_ref_mod, &new_ref_mod);
} else {
- ret = btrfs_add_delayed_data_ref(trans, bytenr,
- num_bytes, parent,
- root_objectid, owner, offset,
- 0, BTRFS_DROP_DELAYED_REF,
+ btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
+ ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
&old_ref_mod, &new_ref_mod);
}
@@ -8489,6 +8485,7 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
u64 offset, u64 ram_bytes,
struct btrfs_key *ins)
{
+ struct btrfs_ref generic_ref = { 0 };
int ret;
BUG_ON(root->root_key.objectid == BTRFS_TREE_LOG_OBJECTID);
@@ -8497,11 +8494,11 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
root->root_key.objectid, owner, offset,
BTRFS_ADD_DELAYED_EXTENT);
- ret = btrfs_add_delayed_data_ref(trans, ins->objectid,
- ins->offset, 0,
- root->root_key.objectid, owner,
- offset, ram_bytes,
- BTRFS_ADD_DELAYED_EXTENT, NULL, NULL);
+ btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
+ ins->objectid, ins->offset, 0);
+ btrfs_init_data_ref(&generic_ref, root->root_key.objectid, owner, offset);
+ ret = btrfs_add_delayed_data_ref(trans, &generic_ref,
+ ram_bytes, NULL, NULL);
return ret;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 4/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref()
2019-02-11 5:16 ` [PATCH v3 4/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref() Qu Wenruo
@ 2019-02-11 12:59 ` Nikolay Borisov
0 siblings, 0 replies; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 12:59 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
> Just like btrfs_add_delayed_tree_ref(), use btrfs_ref to refactor
> btrfs_add_delayed_data_ref().
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/delayed-ref.c | 20 ++++++++++++++------
> fs/btrfs/delayed-ref.h | 7 +++----
> fs/btrfs/extent-tree.c | 23 ++++++++++-------------
> 3 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index 5c864ce2f959..d4245ccd1afb 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -837,10 +837,9 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> * add a delayed data ref. it's similar to btrfs_add_delayed_tree_ref.
> */
> int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
> - u64 bytenr, u64 num_bytes,
> - u64 parent, u64 ref_root,
> - u64 owner, u64 offset, u64 reserved, int action,
> - int *old_ref_mod, int *new_ref_mod)
> + struct btrfs_ref *generic_ref,
> + u64 reserved, int *old_ref_mod,
> + int *new_ref_mod)
> {
> struct btrfs_fs_info *fs_info = trans->fs_info;
> struct btrfs_delayed_data_ref *ref;
> @@ -848,9 +847,17 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
> struct btrfs_delayed_ref_root *delayed_refs;
> struct btrfs_qgroup_extent_record *record = NULL;
> int qrecord_inserted;
> + int action = generic_ref->action;
> int ret;
> + u64 bytenr = generic_ref->bytenr;
> + u64 num_bytes = generic_ref->len;
> + u64 parent = generic_ref->parent;
> + u64 ref_root = generic_ref->data_ref.ref_root;
> + u64 owner = generic_ref->data_ref.ino;
> + u64 offset = generic_ref->data_ref.offset;
> u8 ref_type;
>
> + ASSERT(generic_ref->type == BTRFS_REF_DATA && action);
> ref = kmem_cache_alloc(btrfs_delayed_data_ref_cachep, GFP_NOFS);
> if (!ref)
> return -ENOMEM;
> @@ -874,8 +881,9 @@ int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
> }
>
> if (test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags) &&
> - is_fstree(ref_root)) {
> - record = kmalloc(sizeof(*record), GFP_NOFS);
> + is_fstree(ref_root) && is_fstree(generic_ref->real_root) &&
> + !generic_ref->skip_qgroup) {
> + record = kzalloc(sizeof(*record), GFP_NOFS);
> if (!record) {
> kmem_cache_free(btrfs_delayed_data_ref_cachep, ref);
> kmem_cache_free(btrfs_delayed_ref_head_cachep,
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index 70bb080bf3ef..9037a7de18b7 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -355,10 +355,9 @@ int btrfs_add_delayed_tree_ref(struct btrfs_trans_handle *trans,
> struct btrfs_delayed_extent_op *extent_op,
> int *old_ref_mod, int *new_ref_mod);
> int btrfs_add_delayed_data_ref(struct btrfs_trans_handle *trans,
> - u64 bytenr, u64 num_bytes,
> - u64 parent, u64 ref_root,
> - u64 owner, u64 offset, u64 reserved, int action,
> - int *old_ref_mod, int *new_ref_mod);
> + struct btrfs_ref *generic_ref,
> + u64 reserved, int *old_ref_mod,
> + int *new_ref_mod);
> int btrfs_add_delayed_extent_op(struct btrfs_fs_info *fs_info,
> struct btrfs_trans_handle *trans,
> u64 bytenr, u64 num_bytes,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index b01fad9a56ff..36516b33d07f 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2066,10 +2066,8 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
> NULL, &old_ref_mod, &new_ref_mod);
> } else {
> - ret = btrfs_add_delayed_data_ref(trans, bytenr,
> - num_bytes, parent,
> - root_objectid, owner, offset,
> - 0, BTRFS_ADD_DELAYED_REF,
> + btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
> + ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
> &old_ref_mod, &new_ref_mod);
> }
>
> @@ -7378,10 +7376,8 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
> ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
> &old_ref_mod, &new_ref_mod);
> } else {
> - ret = btrfs_add_delayed_data_ref(trans, bytenr,
> - num_bytes, parent,
> - root_objectid, owner, offset,
> - 0, BTRFS_DROP_DELAYED_REF,
> + btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
> + ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
> &old_ref_mod, &new_ref_mod);
> }
>
> @@ -8489,6 +8485,7 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
> u64 offset, u64 ram_bytes,
> struct btrfs_key *ins)
> {
> + struct btrfs_ref generic_ref = { 0 };
> int ret;
>
> BUG_ON(root->root_key.objectid == BTRFS_TREE_LOG_OBJECTID);
> @@ -8497,11 +8494,11 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
> root->root_key.objectid, owner, offset,
> BTRFS_ADD_DELAYED_EXTENT);
>
> - ret = btrfs_add_delayed_data_ref(trans, ins->objectid,
> - ins->offset, 0,
> - root->root_key.objectid, owner,
> - offset, ram_bytes,
> - BTRFS_ADD_DELAYED_EXTENT, NULL, NULL);
> + btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
> + ins->objectid, ins->offset, 0);
> + btrfs_init_data_ref(&generic_ref, root->root_key.objectid, owner, offset);
> + ret = btrfs_add_delayed_data_ref(trans, &generic_ref,
> + ram_bytes, NULL, NULL);
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 5/9] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (3 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 4/9] btrfs: delayed-ref: Use btrfs_ref to refactor btrfs_add_delayed_data_ref() Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 13:00 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 6/9] btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes() Qu Wenruo
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
It's a perfect match for btrfs_ref_tree_mod() to use btrfs_ref, as
btrfs_ref describes a metadata/data reference update comprehensively.
Now we have one less function use confusing owner/level trick.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent-tree.c | 27 +++++++--------------
fs/btrfs/ref-verify.c | 53 ++++++++++++++++++++++++------------------
fs/btrfs/ref-verify.h | 10 ++++----
3 files changed, 42 insertions(+), 48 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 36516b33d07f..fea552f92a2c 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2055,9 +2055,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
BUG_ON(owner < BTRFS_FIRST_FREE_OBJECTID &&
root_objectid == BTRFS_TREE_LOG_OBJECTID);
- btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, root_objectid,
- owner, offset, BTRFS_ADD_DELAYED_REF);
-
btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
num_bytes, parent);
generic_ref.real_root = root->root_key.objectid;
@@ -2071,6 +2068,8 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
&old_ref_mod, &new_ref_mod);
}
+ btrfs_ref_tree_mod(fs_info, &generic_ref);
+
if (ret == 0 && old_ref_mod < 0 && new_ref_mod >= 0) {
bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
@@ -7289,10 +7288,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
int old_ref_mod, new_ref_mod;
- btrfs_ref_tree_mod(root, buf->start, buf->len, parent,
- root->root_key.objectid,
- btrfs_header_level(buf), 0,
- BTRFS_DROP_DELAYED_REF);
+ btrfs_ref_tree_mod(fs_info, &generic_ref);
ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
&old_ref_mod, &new_ref_mod);
BUG_ON(ret); /* -ENOMEM */
@@ -7353,11 +7349,6 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
if (btrfs_is_testing(fs_info))
return 0;
- if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
- btrfs_ref_tree_mod(root, bytenr, num_bytes, parent,
- root_objectid, owner, offset,
- BTRFS_DROP_DELAYED_REF);
-
btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF, bytenr,
num_bytes, parent);
generic_ref.real_root = root->root_key.objectid;
@@ -7381,6 +7372,9 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
&old_ref_mod, &new_ref_mod);
}
+ if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
+ btrfs_ref_tree_mod(fs_info, &generic_ref);
+
if (ret == 0 && old_ref_mod >= 0 && new_ref_mod < 0) {
bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
@@ -8490,13 +8484,10 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
BUG_ON(root->root_key.objectid == BTRFS_TREE_LOG_OBJECTID);
- btrfs_ref_tree_mod(root, ins->objectid, ins->offset, 0,
- root->root_key.objectid, owner, offset,
- BTRFS_ADD_DELAYED_EXTENT);
-
btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
ins->objectid, ins->offset, 0);
btrfs_init_data_ref(&generic_ref, root->root_key.objectid, owner, offset);
+ btrfs_ref_tree_mod(root->fs_info, &generic_ref);
ret = btrfs_add_delayed_data_ref(trans, &generic_ref,
ram_bytes, NULL, NULL);
return ret;
@@ -8745,13 +8736,11 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
extent_op->is_data = false;
extent_op->level = level;
- btrfs_ref_tree_mod(root, ins.objectid, ins.offset, parent,
- root_objectid, level, 0,
- BTRFS_ADD_DELAYED_EXTENT);
btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
ins.objectid, ins.offset, parent);
generic_ref.real_root = root->root_key.objectid;
btrfs_init_tree_ref(&generic_ref, level, root_objectid);
+ btrfs_ref_tree_mod(fs_info, &generic_ref);
ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
extent_op, NULL, NULL);
if (ret)
diff --git a/fs/btrfs/ref-verify.c b/fs/btrfs/ref-verify.c
index d09b6cdb785a..4a0f6289ef17 100644
--- a/fs/btrfs/ref-verify.c
+++ b/fs/btrfs/ref-verify.c
@@ -670,36 +670,43 @@ static void dump_block_entry(struct btrfs_fs_info *fs_info,
/*
* btrfs_ref_tree_mod: called when we modify a ref for a bytenr
- * @root: the root we are making this modification from.
- * @bytenr: the bytenr we are modifying.
- * @num_bytes: number of bytes.
- * @parent: the parent bytenr.
- * @ref_root: the original root owner of the bytenr.
- * @owner: level in the case of metadata, inode in the case of data.
- * @offset: 0 for metadata, file offset for data.
- * @action: the action that we are doing, this is the same as the delayed ref
- * action.
*
* This will add an action item to the given bytenr and do sanity checks to make
* sure we haven't messed something up. If we are making a new allocation and
* this block entry has history we will delete all previous actions as long as
* our sanity checks pass as they are no longer needed.
*/
-int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
- u64 parent, u64 ref_root, u64 owner, u64 offset,
- int action)
+int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
+ struct btrfs_ref *generic_ref)
{
- struct btrfs_fs_info *fs_info = root->fs_info;
struct ref_entry *ref = NULL, *exist;
struct ref_action *ra = NULL;
struct block_entry *be = NULL;
struct root_entry *re = NULL;
+ int action = generic_ref->action;
int ret = 0;
- bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
+ bool metadata;
+ u64 bytenr = generic_ref->bytenr;
+ u64 num_bytes = generic_ref->len;
+ u64 parent = generic_ref->parent;
+ u64 ref_root;
+ u64 owner;
+ u64 offset;
- if (!btrfs_test_opt(root->fs_info, REF_VERIFY))
+ if (!btrfs_test_opt(fs_info, REF_VERIFY))
return 0;
+ if (generic_ref->type == BTRFS_REF_METADATA) {
+ ref_root = generic_ref->tree_ref.root;
+ owner = generic_ref->tree_ref.level;
+ offset = 0;
+ } else {
+ ref_root = generic_ref->data_ref.ref_root;
+ owner = generic_ref->data_ref.ino;
+ offset = generic_ref->data_ref.offset;
+ }
+ metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
+
ref = kzalloc(sizeof(struct ref_entry), GFP_NOFS);
ra = kmalloc(sizeof(struct ref_action), GFP_NOFS);
if (!ra || !ref) {
@@ -732,7 +739,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
INIT_LIST_HEAD(&ra->list);
ra->action = action;
- ra->root = root->root_key.objectid;
+ ra->root = generic_ref->real_root;
/*
* This is an allocation, preallocate the block_entry in case we haven't
@@ -745,7 +752,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
* is and the new root objectid, so let's not treat the passed
* in root as if it really has a ref for this bytenr.
*/
- be = add_block_entry(root->fs_info, bytenr, num_bytes, ref_root);
+ be = add_block_entry(fs_info, bytenr, num_bytes, ref_root);
if (IS_ERR(be)) {
kfree(ra);
ret = PTR_ERR(be);
@@ -787,13 +794,13 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
* one we want to lookup below when we modify the
* re->num_refs.
*/
- ref_root = root->root_key.objectid;
- re->root_objectid = root->root_key.objectid;
+ ref_root = generic_ref->real_root;
+ re->root_objectid = generic_ref->real_root;
re->num_refs = 0;
}
- spin_lock(&root->fs_info->ref_verify_lock);
- be = lookup_block_entry(&root->fs_info->block_tree, bytenr);
+ spin_lock(&fs_info->ref_verify_lock);
+ be = lookup_block_entry(&fs_info->block_tree, bytenr);
if (!be) {
btrfs_err(fs_info,
"trying to do action %d to bytenr %llu num_bytes %llu but there is no existing entry!",
@@ -862,7 +869,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
* didn't think of some other corner case.
*/
btrfs_err(fs_info, "failed to find root %llu for %llu",
- root->root_key.objectid, be->bytenr);
+ generic_ref->real_root, be->bytenr);
dump_block_entry(fs_info, be);
dump_ref_action(fs_info, ra);
kfree(ra);
@@ -881,7 +888,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
list_add_tail(&ra->list, &be->actions);
ret = 0;
out_unlock:
- spin_unlock(&root->fs_info->ref_verify_lock);
+ spin_unlock(&fs_info->ref_verify_lock);
out:
if (ret)
btrfs_clear_opt(fs_info->mount_opt, REF_VERIFY);
diff --git a/fs/btrfs/ref-verify.h b/fs/btrfs/ref-verify.h
index b7d2a4edfdb7..855de37719b5 100644
--- a/fs/btrfs/ref-verify.h
+++ b/fs/btrfs/ref-verify.h
@@ -9,9 +9,8 @@
#ifdef CONFIG_BTRFS_FS_REF_VERIFY
int btrfs_build_ref_tree(struct btrfs_fs_info *fs_info);
void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info);
-int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
- u64 parent, u64 ref_root, u64 owner, u64 offset,
- int action);
+int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
+ struct btrfs_ref *generic_ref);
void btrfs_free_ref_tree_range(struct btrfs_fs_info *fs_info, u64 start,
u64 len);
@@ -30,9 +29,8 @@ static inline void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info)
{
}
-static inline int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr,
- u64 num_bytes, u64 parent, u64 ref_root,
- u64 owner, u64 offset, int action)
+static inline int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
+ struct btrfs_ref *generic_ref)
{
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 5/9] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
2019-02-11 5:16 ` [PATCH v3 5/9] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod() Qu Wenruo
@ 2019-02-11 13:00 ` Nikolay Borisov
0 siblings, 0 replies; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 13:00 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
> It's a perfect match for btrfs_ref_tree_mod() to use btrfs_ref, as
> btrfs_ref describes a metadata/data reference update comprehensively.
>
> Now we have one less function use confusing owner/level trick.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/extent-tree.c | 27 +++++++--------------
> fs/btrfs/ref-verify.c | 53 ++++++++++++++++++++++++------------------
> fs/btrfs/ref-verify.h | 10 ++++----
> 3 files changed, 42 insertions(+), 48 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 36516b33d07f..fea552f92a2c 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2055,9 +2055,6 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> BUG_ON(owner < BTRFS_FIRST_FREE_OBJECTID &&
> root_objectid == BTRFS_TREE_LOG_OBJECTID);
>
> - btrfs_ref_tree_mod(root, bytenr, num_bytes, parent, root_objectid,
> - owner, offset, BTRFS_ADD_DELAYED_REF);
> -
> btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
> num_bytes, parent);
> generic_ref.real_root = root->root_key.objectid;
> @@ -2071,6 +2068,8 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> &old_ref_mod, &new_ref_mod);
> }
>
> + btrfs_ref_tree_mod(fs_info, &generic_ref);
> +
> if (ret == 0 && old_ref_mod < 0 && new_ref_mod >= 0) {
> bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
>
> @@ -7289,10 +7288,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
> int old_ref_mod, new_ref_mod;
>
> - btrfs_ref_tree_mod(root, buf->start, buf->len, parent,
> - root->root_key.objectid,
> - btrfs_header_level(buf), 0,
> - BTRFS_DROP_DELAYED_REF);
> + btrfs_ref_tree_mod(fs_info, &generic_ref);
> ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
> &old_ref_mod, &new_ref_mod);
> BUG_ON(ret); /* -ENOMEM */
> @@ -7353,11 +7349,6 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
> if (btrfs_is_testing(fs_info))
> return 0;
>
> - if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
> - btrfs_ref_tree_mod(root, bytenr, num_bytes, parent,
> - root_objectid, owner, offset,
> - BTRFS_DROP_DELAYED_REF);
> -
> btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF, bytenr,
> num_bytes, parent);
> generic_ref.real_root = root->root_key.objectid;
> @@ -7381,6 +7372,9 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
> &old_ref_mod, &new_ref_mod);
> }
>
> + if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
> + btrfs_ref_tree_mod(fs_info, &generic_ref);
> +
> if (ret == 0 && old_ref_mod >= 0 && new_ref_mod < 0) {
> bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
>
> @@ -8490,13 +8484,10 @@ int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
>
> BUG_ON(root->root_key.objectid == BTRFS_TREE_LOG_OBJECTID);
>
> - btrfs_ref_tree_mod(root, ins->objectid, ins->offset, 0,
> - root->root_key.objectid, owner, offset,
> - BTRFS_ADD_DELAYED_EXTENT);
> -
> btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
> ins->objectid, ins->offset, 0);
> btrfs_init_data_ref(&generic_ref, root->root_key.objectid, owner, offset);
> + btrfs_ref_tree_mod(root->fs_info, &generic_ref);
> ret = btrfs_add_delayed_data_ref(trans, &generic_ref,
> ram_bytes, NULL, NULL);
> return ret;
> @@ -8745,13 +8736,11 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
> extent_op->is_data = false;
> extent_op->level = level;
>
> - btrfs_ref_tree_mod(root, ins.objectid, ins.offset, parent,
> - root_objectid, level, 0,
> - BTRFS_ADD_DELAYED_EXTENT);
> btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_EXTENT,
> ins.objectid, ins.offset, parent);
> generic_ref.real_root = root->root_key.objectid;
> btrfs_init_tree_ref(&generic_ref, level, root_objectid);
> + btrfs_ref_tree_mod(fs_info, &generic_ref);
> ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
> extent_op, NULL, NULL);
> if (ret)
> diff --git a/fs/btrfs/ref-verify.c b/fs/btrfs/ref-verify.c
> index d09b6cdb785a..4a0f6289ef17 100644
> --- a/fs/btrfs/ref-verify.c
> +++ b/fs/btrfs/ref-verify.c
> @@ -670,36 +670,43 @@ static void dump_block_entry(struct btrfs_fs_info *fs_info,
>
> /*
> * btrfs_ref_tree_mod: called when we modify a ref for a bytenr
> - * @root: the root we are making this modification from.
> - * @bytenr: the bytenr we are modifying.
> - * @num_bytes: number of bytes.
> - * @parent: the parent bytenr.
> - * @ref_root: the original root owner of the bytenr.
> - * @owner: level in the case of metadata, inode in the case of data.
> - * @offset: 0 for metadata, file offset for data.
> - * @action: the action that we are doing, this is the same as the delayed ref
> - * action.
> *
> * This will add an action item to the given bytenr and do sanity checks to make
> * sure we haven't messed something up. If we are making a new allocation and
> * this block entry has history we will delete all previous actions as long as
> * our sanity checks pass as they are no longer needed.
> */
> -int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
> - u64 parent, u64 ref_root, u64 owner, u64 offset,
> - int action)
> +int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
> + struct btrfs_ref *generic_ref)
> {
> - struct btrfs_fs_info *fs_info = root->fs_info;
> struct ref_entry *ref = NULL, *exist;
> struct ref_action *ra = NULL;
> struct block_entry *be = NULL;
> struct root_entry *re = NULL;
> + int action = generic_ref->action;
> int ret = 0;
> - bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
> + bool metadata;
> + u64 bytenr = generic_ref->bytenr;
> + u64 num_bytes = generic_ref->len;
> + u64 parent = generic_ref->parent;
> + u64 ref_root;
> + u64 owner;
> + u64 offset;
>
> - if (!btrfs_test_opt(root->fs_info, REF_VERIFY))
> + if (!btrfs_test_opt(fs_info, REF_VERIFY))
> return 0;
>
> + if (generic_ref->type == BTRFS_REF_METADATA) {
> + ref_root = generic_ref->tree_ref.root;
> + owner = generic_ref->tree_ref.level;
> + offset = 0;
> + } else {
> + ref_root = generic_ref->data_ref.ref_root;
> + owner = generic_ref->data_ref.ino;
> + offset = generic_ref->data_ref.offset;
> + }
> + metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
> +
> ref = kzalloc(sizeof(struct ref_entry), GFP_NOFS);
> ra = kmalloc(sizeof(struct ref_action), GFP_NOFS);
> if (!ra || !ref) {
> @@ -732,7 +739,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
>
> INIT_LIST_HEAD(&ra->list);
> ra->action = action;
> - ra->root = root->root_key.objectid;
> + ra->root = generic_ref->real_root;
>
> /*
> * This is an allocation, preallocate the block_entry in case we haven't
> @@ -745,7 +752,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
> * is and the new root objectid, so let's not treat the passed
> * in root as if it really has a ref for this bytenr.
> */
> - be = add_block_entry(root->fs_info, bytenr, num_bytes, ref_root);
> + be = add_block_entry(fs_info, bytenr, num_bytes, ref_root);
> if (IS_ERR(be)) {
> kfree(ra);
> ret = PTR_ERR(be);
> @@ -787,13 +794,13 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
> * one we want to lookup below when we modify the
> * re->num_refs.
> */
> - ref_root = root->root_key.objectid;
> - re->root_objectid = root->root_key.objectid;
> + ref_root = generic_ref->real_root;
> + re->root_objectid = generic_ref->real_root;
> re->num_refs = 0;
> }
>
> - spin_lock(&root->fs_info->ref_verify_lock);
> - be = lookup_block_entry(&root->fs_info->block_tree, bytenr);
> + spin_lock(&fs_info->ref_verify_lock);
> + be = lookup_block_entry(&fs_info->block_tree, bytenr);
> if (!be) {
> btrfs_err(fs_info,
> "trying to do action %d to bytenr %llu num_bytes %llu but there is no existing entry!",
> @@ -862,7 +869,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
> * didn't think of some other corner case.
> */
> btrfs_err(fs_info, "failed to find root %llu for %llu",
> - root->root_key.objectid, be->bytenr);
> + generic_ref->real_root, be->bytenr);
> dump_block_entry(fs_info, be);
> dump_ref_action(fs_info, ra);
> kfree(ra);
> @@ -881,7 +888,7 @@ int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
> list_add_tail(&ra->list, &be->actions);
> ret = 0;
> out_unlock:
> - spin_unlock(&root->fs_info->ref_verify_lock);
> + spin_unlock(&fs_info->ref_verify_lock);
> out:
> if (ret)
> btrfs_clear_opt(fs_info->mount_opt, REF_VERIFY);
> diff --git a/fs/btrfs/ref-verify.h b/fs/btrfs/ref-verify.h
> index b7d2a4edfdb7..855de37719b5 100644
> --- a/fs/btrfs/ref-verify.h
> +++ b/fs/btrfs/ref-verify.h
> @@ -9,9 +9,8 @@
> #ifdef CONFIG_BTRFS_FS_REF_VERIFY
> int btrfs_build_ref_tree(struct btrfs_fs_info *fs_info);
> void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info);
> -int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr, u64 num_bytes,
> - u64 parent, u64 ref_root, u64 owner, u64 offset,
> - int action);
> +int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
> + struct btrfs_ref *generic_ref);
> void btrfs_free_ref_tree_range(struct btrfs_fs_info *fs_info, u64 start,
> u64 len);
>
> @@ -30,9 +29,8 @@ static inline void btrfs_free_ref_cache(struct btrfs_fs_info *fs_info)
> {
> }
>
> -static inline int btrfs_ref_tree_mod(struct btrfs_root *root, u64 bytenr,
> - u64 num_bytes, u64 parent, u64 ref_root,
> - u64 owner, u64 offset, int action)
> +static inline int btrfs_ref_tree_mod(struct btrfs_fs_info *fs_info,
> + struct btrfs_ref *generic_ref)
> {
> return 0;
> }
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 6/9] btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (4 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 5/9] btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod() Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 5:16 ` [PATCH v3 7/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref() Qu Wenruo
` (3 subsequent siblings)
9 siblings, 0 replies; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Since add_pinned_bytes() only needs to know if the extent is metadata
and if it's a chunk tree extent, btrfs_ref is a perfect match for it, as
we don't need various owner/level trick to determine extent type.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
---
fs/btrfs/extent-tree.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index fea552f92a2c..5419266d3e5d 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -756,14 +756,15 @@ static struct btrfs_space_info *__find_space_info(struct btrfs_fs_info *info,
return NULL;
}
-static void add_pinned_bytes(struct btrfs_fs_info *fs_info, s64 num_bytes,
- bool metadata, u64 root_objectid)
+static void add_pinned_bytes(struct btrfs_fs_info *fs_info,
+ struct btrfs_ref *ref)
{
struct btrfs_space_info *space_info;
+ s64 num_bytes = -ref->len;
u64 flags;
- if (metadata) {
- if (root_objectid == BTRFS_CHUNK_TREE_OBJECTID)
+ if (ref->type == BTRFS_REF_METADATA) {
+ if (ref->tree_ref.root == BTRFS_CHUNK_TREE_OBJECTID)
flags = BTRFS_BLOCK_GROUP_SYSTEM;
else
flags = BTRFS_BLOCK_GROUP_METADATA;
@@ -2070,11 +2071,8 @@ int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
btrfs_ref_tree_mod(fs_info, &generic_ref);
- if (ret == 0 && old_ref_mod < 0 && new_ref_mod >= 0) {
- bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
-
- add_pinned_bytes(fs_info, -num_bytes, metadata, root_objectid);
- }
+ if (ret == 0 && old_ref_mod < 0 && new_ref_mod >= 0)
+ add_pinned_bytes(fs_info, &generic_ref);
return ret;
}
@@ -7323,8 +7321,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
}
out:
if (pin)
- add_pinned_bytes(fs_info, buf->len, true,
- root->root_key.objectid);
+ add_pinned_bytes(fs_info, &generic_ref);
if (last_ref) {
/*
@@ -7375,11 +7372,8 @@ int btrfs_free_extent(struct btrfs_trans_handle *trans,
if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
btrfs_ref_tree_mod(fs_info, &generic_ref);
- if (ret == 0 && old_ref_mod >= 0 && new_ref_mod < 0) {
- bool metadata = owner < BTRFS_FIRST_FREE_OBJECTID;
-
- add_pinned_bytes(fs_info, num_bytes, metadata, root_objectid);
- }
+ if (ret == 0 && old_ref_mod >= 0 && new_ref_mod < 0)
+ add_pinned_bytes(fs_info, &generic_ref);
return ret;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH v3 7/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (5 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 6/9] btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes() Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 13:04 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 8/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent() Qu Wenruo
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
Now we don't need to play the dirty game of reusing @owner for tree block
level.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/ctree.h | 5 ++--
fs/btrfs/extent-tree.c | 57 ++++++++++++++++++++++++------------------
fs/btrfs/file.c | 17 +++++++++----
fs/btrfs/inode.c | 10 +++++---
fs/btrfs/ioctl.c | 15 ++++++-----
fs/btrfs/relocation.c | 42 ++++++++++++++++++++-----------
fs/btrfs/tree-log.c | 11 +++++---
7 files changed, 96 insertions(+), 61 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 9306925b6790..cd90f7cbc8f2 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -41,6 +41,7 @@ extern struct kmem_cache *btrfs_bit_radix_cachep;
extern struct kmem_cache *btrfs_path_cachep;
extern struct kmem_cache *btrfs_free_space_cachep;
struct btrfs_ordered_sum;
+struct btrfs_ref;
#define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
@@ -2755,9 +2756,7 @@ int btrfs_free_and_pin_reserved_extent(struct btrfs_fs_info *fs_info,
void btrfs_prepare_extent_commit(struct btrfs_fs_info *fs_info);
int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans);
int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- u64 bytenr, u64 num_bytes, u64 parent,
- u64 root_objectid, u64 owner, u64 offset);
+ struct btrfs_ref *generic_ref);
int btrfs_start_dirty_block_groups(struct btrfs_trans_handle *trans);
int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 5419266d3e5d..d08186ec6410 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2044,35 +2044,28 @@ int btrfs_discard_extent(struct btrfs_fs_info *fs_info, u64 bytenr,
/* Can return -ENOMEM */
int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- u64 bytenr, u64 num_bytes, u64 parent,
- u64 root_objectid, u64 owner, u64 offset)
+ struct btrfs_ref *generic_ref)
{
- struct btrfs_fs_info *fs_info = root->fs_info;
- struct btrfs_ref generic_ref = { 0 };
+ struct btrfs_fs_info *fs_info = trans->fs_info;
int old_ref_mod, new_ref_mod;
int ret;
- BUG_ON(owner < BTRFS_FIRST_FREE_OBJECTID &&
- root_objectid == BTRFS_TREE_LOG_OBJECTID);
+ ASSERT(generic_ref->type != BTRFS_REF_NOT_SET &&
+ generic_ref->action);
+ BUG_ON(generic_ref->type == BTRFS_REF_METADATA &&
+ generic_ref->tree_ref.root == BTRFS_TREE_LOG_OBJECTID);
- btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
- num_bytes, parent);
- generic_ref.real_root = root->root_key.objectid;
- if (owner < BTRFS_FIRST_FREE_OBJECTID) {
- btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
- ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
+ if (generic_ref->type == BTRFS_REF_METADATA)
+ ret = btrfs_add_delayed_tree_ref(trans, generic_ref,
NULL, &old_ref_mod, &new_ref_mod);
- } else {
- btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
- ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
+ else
+ ret = btrfs_add_delayed_data_ref(trans, generic_ref, 0,
&old_ref_mod, &new_ref_mod);
- }
- btrfs_ref_tree_mod(fs_info, &generic_ref);
+ btrfs_ref_tree_mod(fs_info, generic_ref);
if (ret == 0 && old_ref_mod < 0 && new_ref_mod >= 0)
- add_pinned_bytes(fs_info, &generic_ref);
+ add_pinned_bytes(fs_info, generic_ref);
return ret;
}
@@ -3246,7 +3239,10 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
u32 nritems;
struct btrfs_key key;
struct btrfs_file_extent_item *fi;
+ struct btrfs_ref generic_ref = { 0 };
+ bool for_reloc = btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC);
int i;
+ int action;
int level;
int ret = 0;
@@ -3264,6 +3260,10 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
parent = buf->start;
else
parent = 0;
+ if (inc)
+ action = BTRFS_ADD_DELAYED_REF;
+ else
+ action = BTRFS_DROP_DELAYED_REF;
for (i = 0; i < nritems; i++) {
if (level == 0) {
@@ -3281,10 +3281,14 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
num_bytes = btrfs_file_extent_disk_num_bytes(buf, fi);
key.offset -= btrfs_file_extent_offset(buf, fi);
+ btrfs_init_generic_ref(&generic_ref, action, bytenr,
+ num_bytes, parent);
+ generic_ref.real_root = root->root_key.objectid;
+ btrfs_init_data_ref(&generic_ref, ref_root, key.objectid,
+ key.offset);
+ generic_ref.skip_qgroup = for_reloc;
if (inc)
- ret = btrfs_inc_extent_ref(trans, root, bytenr,
- num_bytes, parent, ref_root,
- key.objectid, key.offset);
+ ret = btrfs_inc_extent_ref(trans, &generic_ref);
else
ret = btrfs_free_extent(trans, root, bytenr,
num_bytes, parent, ref_root,
@@ -3294,10 +3298,13 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
} else {
bytenr = btrfs_node_blockptr(buf, i);
num_bytes = fs_info->nodesize;
+ btrfs_init_generic_ref(&generic_ref, action, bytenr,
+ num_bytes, parent);
+ generic_ref.real_root = root->root_key.objectid;
+ btrfs_init_tree_ref(&generic_ref, level - 1, ref_root);
+ generic_ref.skip_qgroup = for_reloc;
if (inc)
- ret = btrfs_inc_extent_ref(trans, root, bytenr,
- num_bytes, parent, ref_root,
- level - 1, 0);
+ ret = btrfs_inc_extent_ref(trans, &generic_ref);
else
ret = btrfs_free_extent(trans, root, bytenr,
num_bytes, parent, ref_root,
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 34fe8a58b0e9..2aba5980f522 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -754,6 +754,7 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info = root->fs_info;
struct extent_buffer *leaf;
struct btrfs_file_extent_item *fi;
+ struct btrfs_ref ref = { 0 };
struct btrfs_key key;
struct btrfs_key new_key;
u64 ino = btrfs_ino(BTRFS_I(inode));
@@ -909,11 +910,14 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
btrfs_mark_buffer_dirty(leaf);
if (update_refs && disk_bytenr > 0) {
- ret = btrfs_inc_extent_ref(trans, root,
- disk_bytenr, num_bytes, 0,
+ btrfs_init_generic_ref(&ref,
+ BTRFS_ADD_DELAYED_REF,
+ disk_bytenr, num_bytes, 0);
+ btrfs_init_data_ref(&ref,
root->root_key.objectid,
new_key.objectid,
start - extent_offset);
+ ret = btrfs_inc_extent_ref(trans, &ref);
BUG_ON(ret); /* -ENOMEM */
}
key.offset = start;
@@ -1142,6 +1146,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
struct extent_buffer *leaf;
struct btrfs_path *path;
struct btrfs_file_extent_item *fi;
+ struct btrfs_ref ref = { 0 };
struct btrfs_key key;
struct btrfs_key new_key;
u64 bytenr;
@@ -1287,9 +1292,11 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
extent_end - split);
btrfs_mark_buffer_dirty(leaf);
- ret = btrfs_inc_extent_ref(trans, root, bytenr, num_bytes,
- 0, root->root_key.objectid,
- ino, orig_offset);
+ btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, bytenr,
+ num_bytes, 0);
+ btrfs_init_data_ref(&ref, root->root_key.objectid, ino,
+ orig_offset);
+ ret = btrfs_inc_extent_ref(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3f180b857e20..95535ba375f8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2531,6 +2531,7 @@ static noinline int relink_extent_backref(struct btrfs_path *path,
struct btrfs_file_extent_item *item;
struct btrfs_ordered_extent *ordered;
struct btrfs_trans_handle *trans;
+ struct btrfs_ref ref = { 0 };
struct btrfs_root *root;
struct btrfs_key key;
struct extent_buffer *leaf;
@@ -2701,10 +2702,11 @@ static noinline int relink_extent_backref(struct btrfs_path *path,
inode_add_bytes(inode, len);
btrfs_release_path(path);
- ret = btrfs_inc_extent_ref(trans, root, new->bytenr,
- new->disk_len, 0,
- backref->root_id, backref->inum,
- new->file_pos); /* start - extent_offset */
+ btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new->bytenr,
+ new->disk_len, 0);
+ btrfs_init_data_ref(&ref, backref->root_id, backref->inum,
+ new->file_pos); /* start - extent_offset */
+ ret = btrfs_inc_extent_ref(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out_free_path;
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 3f9d7be30bf4..f5d49a92f53c 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3755,13 +3755,16 @@ static int btrfs_clone(struct inode *src, struct inode *inode,
datal);
if (disko) {
+ struct btrfs_ref ref = { 0 };
inode_add_bytes(inode, datal);
- ret = btrfs_inc_extent_ref(trans,
- root,
- disko, diskl, 0,
- root->root_key.objectid,
- btrfs_ino(BTRFS_I(inode)),
- new_key.offset - datao);
+ btrfs_init_generic_ref(&ref,
+ BTRFS_ADD_DELAYED_REF, disko,
+ diskl, 0);
+ btrfs_init_data_ref(&ref,
+ root->root_key.objectid,
+ btrfs_ino(BTRFS_I(inode)),
+ new_key.offset - datao);
+ ret = btrfs_inc_extent_ref(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans,
ret);
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 70b85a593b7b..b82d532d8e5e 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1643,6 +1643,8 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
nritems = btrfs_header_nritems(leaf);
for (i = 0; i < nritems; i++) {
+ struct btrfs_ref ref = { 0 };
+
cond_resched();
btrfs_item_key_to_cpu(leaf, &key, i);
if (key.type != BTRFS_EXTENT_DATA_KEY)
@@ -1703,10 +1705,12 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
dirty = 1;
key.offset -= btrfs_file_extent_offset(leaf, fi);
- ret = btrfs_inc_extent_ref(trans, root, new_bytenr,
- num_bytes, parent,
- btrfs_header_owner(leaf),
- key.objectid, key.offset);
+ btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new_bytenr,
+ num_bytes, parent);
+ ref.real_root = root->root_key.objectid;
+ btrfs_init_data_ref(&ref, btrfs_header_owner(leaf),
+ key.objectid, key.offset);
+ ret = btrfs_inc_extent_ref(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
break;
@@ -1756,6 +1760,7 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
struct btrfs_fs_info *fs_info = dest->fs_info;
struct extent_buffer *eb;
struct extent_buffer *parent;
+ struct btrfs_ref ref = { 0 };
struct btrfs_key key;
u64 old_bytenr;
u64 new_bytenr;
@@ -1914,13 +1919,17 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
path->slots[level], old_ptr_gen);
btrfs_mark_buffer_dirty(path->nodes[level]);
- ret = btrfs_inc_extent_ref(trans, src, old_bytenr,
- blocksize, path->nodes[level]->start,
- src->root_key.objectid, level - 1, 0);
+ btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, old_bytenr,
+ blocksize, path->nodes[level]->start);
+ ref.skip_qgroup = true;
+ btrfs_init_tree_ref(&ref, level - 1, src->root_key.objectid);
+ ret = btrfs_inc_extent_ref(trans, &ref);
BUG_ON(ret);
- ret = btrfs_inc_extent_ref(trans, dest, new_bytenr,
- blocksize, 0, dest->root_key.objectid,
- level - 1, 0);
+ btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new_bytenr,
+ blocksize, 0);
+ ref.skip_qgroup = true;
+ btrfs_init_tree_ref(&ref, level - 1, dest->root_key.objectid);
+ ret = btrfs_inc_extent_ref(trans, &ref);
BUG_ON(ret);
ret = btrfs_free_extent(trans, src, new_bytenr, blocksize,
@@ -2719,6 +2728,7 @@ static int do_relocation(struct btrfs_trans_handle *trans,
rc->backref_cache.path[node->level] = node;
list_for_each_entry(edge, &node->upper, list[LOWER]) {
struct btrfs_key first_key;
+ struct btrfs_ref ref = { 0 };
cond_resched();
@@ -2816,11 +2826,13 @@ static int do_relocation(struct btrfs_trans_handle *trans,
trans->transid);
btrfs_mark_buffer_dirty(upper->eb);
- ret = btrfs_inc_extent_ref(trans, root,
- node->eb->start, blocksize,
- upper->eb->start,
- btrfs_header_owner(upper->eb),
- node->level, 0);
+ btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF,
+ node->eb->start, blocksize,
+ upper->eb->start);
+ ref.real_root = root->root_key.objectid;
+ btrfs_init_tree_ref(&ref, node->level,
+ btrfs_header_owner(upper->eb));
+ ret = btrfs_inc_extent_ref(trans, &ref);
BUG_ON(ret);
ret = btrfs_drop_subtree(trans, root, eb, upper->eb);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 1a69a45ae926..2e62f275ae5b 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -693,9 +693,11 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
goto out;
if (ins.objectid > 0) {
+ struct btrfs_ref ref = { 0 };
u64 csum_start;
u64 csum_end;
LIST_HEAD(ordered_sums);
+
/*
* is this extent already allocated in the extent
* allocation tree? If so, just add a reference
@@ -703,10 +705,13 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
ret = btrfs_lookup_data_extent(fs_info, ins.objectid,
ins.offset);
if (ret == 0) {
- ret = btrfs_inc_extent_ref(trans, root,
- ins.objectid, ins.offset,
- 0, root->root_key.objectid,
+ btrfs_init_generic_ref(&ref,
+ BTRFS_ADD_DELAYED_REF,
+ ins.objectid, ins.offset, 0);
+ btrfs_init_data_ref(&ref,
+ root->root_key.objectid,
key->objectid, offset);
+ ret = btrfs_inc_extent_ref(trans, &ref);
if (ret)
goto out;
} else {
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 7/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
2019-02-11 5:16 ` [PATCH v3 7/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref() Qu Wenruo
@ 2019-02-11 13:04 ` Nikolay Borisov
0 siblings, 0 replies; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 13:04 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
> Now we don't need to play the dirty game of reusing @owner for tree block
> level.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/ctree.h | 5 ++--
> fs/btrfs/extent-tree.c | 57 ++++++++++++++++++++++++------------------
> fs/btrfs/file.c | 17 +++++++++----
> fs/btrfs/inode.c | 10 +++++---
> fs/btrfs/ioctl.c | 15 ++++++-----
> fs/btrfs/relocation.c | 42 ++++++++++++++++++++-----------
> fs/btrfs/tree-log.c | 11 +++++---
> 7 files changed, 96 insertions(+), 61 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 9306925b6790..cd90f7cbc8f2 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -41,6 +41,7 @@ extern struct kmem_cache *btrfs_bit_radix_cachep;
> extern struct kmem_cache *btrfs_path_cachep;
> extern struct kmem_cache *btrfs_free_space_cachep;
> struct btrfs_ordered_sum;
> +struct btrfs_ref;
>
> #define BTRFS_MAGIC 0x4D5F53665248425FULL /* ascii _BHRfS_M, no null */
>
> @@ -2755,9 +2756,7 @@ int btrfs_free_and_pin_reserved_extent(struct btrfs_fs_info *fs_info,
> void btrfs_prepare_extent_commit(struct btrfs_fs_info *fs_info);
> int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans);
> int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> - struct btrfs_root *root,
> - u64 bytenr, u64 num_bytes, u64 parent,
> - u64 root_objectid, u64 owner, u64 offset);
> + struct btrfs_ref *generic_ref);
>
> int btrfs_start_dirty_block_groups(struct btrfs_trans_handle *trans);
> int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 5419266d3e5d..d08186ec6410 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2044,35 +2044,28 @@ int btrfs_discard_extent(struct btrfs_fs_info *fs_info, u64 bytenr,
>
> /* Can return -ENOMEM */
> int btrfs_inc_extent_ref(struct btrfs_trans_handle *trans,
> - struct btrfs_root *root,
> - u64 bytenr, u64 num_bytes, u64 parent,
> - u64 root_objectid, u64 owner, u64 offset)
> + struct btrfs_ref *generic_ref)
> {
> - struct btrfs_fs_info *fs_info = root->fs_info;
> - struct btrfs_ref generic_ref = { 0 };
> + struct btrfs_fs_info *fs_info = trans->fs_info;
> int old_ref_mod, new_ref_mod;
> int ret;
>
> - BUG_ON(owner < BTRFS_FIRST_FREE_OBJECTID &&
> - root_objectid == BTRFS_TREE_LOG_OBJECTID);
> + ASSERT(generic_ref->type != BTRFS_REF_NOT_SET &&
> + generic_ref->action);
> + BUG_ON(generic_ref->type == BTRFS_REF_METADATA &&
> + generic_ref->tree_ref.root == BTRFS_TREE_LOG_OBJECTID);
>
> - btrfs_init_generic_ref(&generic_ref, BTRFS_ADD_DELAYED_REF, bytenr,
> - num_bytes, parent);
> - generic_ref.real_root = root->root_key.objectid;
> - if (owner < BTRFS_FIRST_FREE_OBJECTID) {
> - btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
> - ret = btrfs_add_delayed_tree_ref(trans, &generic_ref,
> + if (generic_ref->type == BTRFS_REF_METADATA)
> + ret = btrfs_add_delayed_tree_ref(trans, generic_ref,
> NULL, &old_ref_mod, &new_ref_mod);
> - } else {
> - btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
> - ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
> + else
> + ret = btrfs_add_delayed_data_ref(trans, generic_ref, 0,
> &old_ref_mod, &new_ref_mod);
> - }
>
> - btrfs_ref_tree_mod(fs_info, &generic_ref);
> + btrfs_ref_tree_mod(fs_info, generic_ref);
>
> if (ret == 0 && old_ref_mod < 0 && new_ref_mod >= 0)
> - add_pinned_bytes(fs_info, &generic_ref);
> + add_pinned_bytes(fs_info, generic_ref);
>
> return ret;
> }
> @@ -3246,7 +3239,10 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
> u32 nritems;
> struct btrfs_key key;
> struct btrfs_file_extent_item *fi;
> + struct btrfs_ref generic_ref = { 0 };
> + bool for_reloc = btrfs_header_flag(buf, BTRFS_HEADER_FLAG_RELOC);
> int i;
> + int action;
> int level;
> int ret = 0;
>
> @@ -3264,6 +3260,10 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
> parent = buf->start;
> else
> parent = 0;
> + if (inc)
> + action = BTRFS_ADD_DELAYED_REF;
> + else
> + action = BTRFS_DROP_DELAYED_REF;
>
> for (i = 0; i < nritems; i++) {
> if (level == 0) {
> @@ -3281,10 +3281,14 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
>
> num_bytes = btrfs_file_extent_disk_num_bytes(buf, fi);
> key.offset -= btrfs_file_extent_offset(buf, fi);
> + btrfs_init_generic_ref(&generic_ref, action, bytenr,
> + num_bytes, parent);
> + generic_ref.real_root = root->root_key.objectid;
> + btrfs_init_data_ref(&generic_ref, ref_root, key.objectid,
> + key.offset);
> + generic_ref.skip_qgroup = for_reloc;
> if (inc)
> - ret = btrfs_inc_extent_ref(trans, root, bytenr,
> - num_bytes, parent, ref_root,
> - key.objectid, key.offset);
> + ret = btrfs_inc_extent_ref(trans, &generic_ref);
> else
> ret = btrfs_free_extent(trans, root, bytenr,
> num_bytes, parent, ref_root,
> @@ -3294,10 +3298,13 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
> } else {
> bytenr = btrfs_node_blockptr(buf, i);
> num_bytes = fs_info->nodesize;
> + btrfs_init_generic_ref(&generic_ref, action, bytenr,
> + num_bytes, parent);
> + generic_ref.real_root = root->root_key.objectid;
> + btrfs_init_tree_ref(&generic_ref, level - 1, ref_root);
> + generic_ref.skip_qgroup = for_reloc;
> if (inc)
> - ret = btrfs_inc_extent_ref(trans, root, bytenr,
> - num_bytes, parent, ref_root,
> - level - 1, 0);
> + ret = btrfs_inc_extent_ref(trans, &generic_ref);
> else
> ret = btrfs_free_extent(trans, root, bytenr,
> num_bytes, parent, ref_root,
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 34fe8a58b0e9..2aba5980f522 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -754,6 +754,7 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info = root->fs_info;
> struct extent_buffer *leaf;
> struct btrfs_file_extent_item *fi;
> + struct btrfs_ref ref = { 0 };
> struct btrfs_key key;
> struct btrfs_key new_key;
> u64 ino = btrfs_ino(BTRFS_I(inode));
> @@ -909,11 +910,14 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
> btrfs_mark_buffer_dirty(leaf);
>
> if (update_refs && disk_bytenr > 0) {
> - ret = btrfs_inc_extent_ref(trans, root,
> - disk_bytenr, num_bytes, 0,
> + btrfs_init_generic_ref(&ref,
> + BTRFS_ADD_DELAYED_REF,
> + disk_bytenr, num_bytes, 0);
> + btrfs_init_data_ref(&ref,
> root->root_key.objectid,
> new_key.objectid,
> start - extent_offset);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> BUG_ON(ret); /* -ENOMEM */
> }
> key.offset = start;
> @@ -1142,6 +1146,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
> struct extent_buffer *leaf;
> struct btrfs_path *path;
> struct btrfs_file_extent_item *fi;
> + struct btrfs_ref ref = { 0 };
> struct btrfs_key key;
> struct btrfs_key new_key;
> u64 bytenr;
> @@ -1287,9 +1292,11 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
> extent_end - split);
> btrfs_mark_buffer_dirty(leaf);
>
> - ret = btrfs_inc_extent_ref(trans, root, bytenr, num_bytes,
> - 0, root->root_key.objectid,
> - ino, orig_offset);
> + btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, bytenr,
> + num_bytes, 0);
> + btrfs_init_data_ref(&ref, root->root_key.objectid, ino,
> + orig_offset);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> goto out;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3f180b857e20..95535ba375f8 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2531,6 +2531,7 @@ static noinline int relink_extent_backref(struct btrfs_path *path,
> struct btrfs_file_extent_item *item;
> struct btrfs_ordered_extent *ordered;
> struct btrfs_trans_handle *trans;
> + struct btrfs_ref ref = { 0 };
> struct btrfs_root *root;
> struct btrfs_key key;
> struct extent_buffer *leaf;
> @@ -2701,10 +2702,11 @@ static noinline int relink_extent_backref(struct btrfs_path *path,
> inode_add_bytes(inode, len);
> btrfs_release_path(path);
>
> - ret = btrfs_inc_extent_ref(trans, root, new->bytenr,
> - new->disk_len, 0,
> - backref->root_id, backref->inum,
> - new->file_pos); /* start - extent_offset */
> + btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new->bytenr,
> + new->disk_len, 0);
> + btrfs_init_data_ref(&ref, backref->root_id, backref->inum,
> + new->file_pos); /* start - extent_offset */
> + ret = btrfs_inc_extent_ref(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> goto out_free_path;
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 3f9d7be30bf4..f5d49a92f53c 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3755,13 +3755,16 @@ static int btrfs_clone(struct inode *src, struct inode *inode,
> datal);
>
> if (disko) {
> + struct btrfs_ref ref = { 0 };
> inode_add_bytes(inode, datal);
> - ret = btrfs_inc_extent_ref(trans,
> - root,
> - disko, diskl, 0,
> - root->root_key.objectid,
> - btrfs_ino(BTRFS_I(inode)),
> - new_key.offset - datao);
> + btrfs_init_generic_ref(&ref,
> + BTRFS_ADD_DELAYED_REF, disko,
> + diskl, 0);
> + btrfs_init_data_ref(&ref,
> + root->root_key.objectid,
> + btrfs_ino(BTRFS_I(inode)),
> + new_key.offset - datao);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans,
> ret);
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 70b85a593b7b..b82d532d8e5e 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1643,6 +1643,8 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
>
> nritems = btrfs_header_nritems(leaf);
> for (i = 0; i < nritems; i++) {
> + struct btrfs_ref ref = { 0 };
> +
> cond_resched();
> btrfs_item_key_to_cpu(leaf, &key, i);
> if (key.type != BTRFS_EXTENT_DATA_KEY)
> @@ -1703,10 +1705,12 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
> dirty = 1;
>
> key.offset -= btrfs_file_extent_offset(leaf, fi);
> - ret = btrfs_inc_extent_ref(trans, root, new_bytenr,
> - num_bytes, parent,
> - btrfs_header_owner(leaf),
> - key.objectid, key.offset);
> + btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new_bytenr,
> + num_bytes, parent);
> + ref.real_root = root->root_key.objectid;
> + btrfs_init_data_ref(&ref, btrfs_header_owner(leaf),
> + key.objectid, key.offset);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> break;
> @@ -1756,6 +1760,7 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
> struct btrfs_fs_info *fs_info = dest->fs_info;
> struct extent_buffer *eb;
> struct extent_buffer *parent;
> + struct btrfs_ref ref = { 0 };
> struct btrfs_key key;
> u64 old_bytenr;
> u64 new_bytenr;
> @@ -1914,13 +1919,17 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
> path->slots[level], old_ptr_gen);
> btrfs_mark_buffer_dirty(path->nodes[level]);
>
> - ret = btrfs_inc_extent_ref(trans, src, old_bytenr,
> - blocksize, path->nodes[level]->start,
> - src->root_key.objectid, level - 1, 0);
> + btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, old_bytenr,
> + blocksize, path->nodes[level]->start);
> + ref.skip_qgroup = true;
> + btrfs_init_tree_ref(&ref, level - 1, src->root_key.objectid);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> BUG_ON(ret);
> - ret = btrfs_inc_extent_ref(trans, dest, new_bytenr,
> - blocksize, 0, dest->root_key.objectid,
> - level - 1, 0);
> + btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF, new_bytenr,
> + blocksize, 0);
> + ref.skip_qgroup = true;
> + btrfs_init_tree_ref(&ref, level - 1, dest->root_key.objectid);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> BUG_ON(ret);
>
> ret = btrfs_free_extent(trans, src, new_bytenr, blocksize,
> @@ -2719,6 +2728,7 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> rc->backref_cache.path[node->level] = node;
> list_for_each_entry(edge, &node->upper, list[LOWER]) {
> struct btrfs_key first_key;
> + struct btrfs_ref ref = { 0 };
>
> cond_resched();
>
> @@ -2816,11 +2826,13 @@ static int do_relocation(struct btrfs_trans_handle *trans,
> trans->transid);
> btrfs_mark_buffer_dirty(upper->eb);
>
> - ret = btrfs_inc_extent_ref(trans, root,
> - node->eb->start, blocksize,
> - upper->eb->start,
> - btrfs_header_owner(upper->eb),
> - node->level, 0);
> + btrfs_init_generic_ref(&ref, BTRFS_ADD_DELAYED_REF,
> + node->eb->start, blocksize,
> + upper->eb->start);
> + ref.real_root = root->root_key.objectid;
> + btrfs_init_tree_ref(&ref, node->level,
> + btrfs_header_owner(upper->eb));
> + ret = btrfs_inc_extent_ref(trans, &ref);
> BUG_ON(ret);
>
> ret = btrfs_drop_subtree(trans, root, eb, upper->eb);
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 1a69a45ae926..2e62f275ae5b 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -693,9 +693,11 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
> goto out;
>
> if (ins.objectid > 0) {
> + struct btrfs_ref ref = { 0 };
> u64 csum_start;
> u64 csum_end;
> LIST_HEAD(ordered_sums);
> +
> /*
> * is this extent already allocated in the extent
> * allocation tree? If so, just add a reference
> @@ -703,10 +705,13 @@ static noinline int replay_one_extent(struct btrfs_trans_handle *trans,
> ret = btrfs_lookup_data_extent(fs_info, ins.objectid,
> ins.offset);
> if (ret == 0) {
> - ret = btrfs_inc_extent_ref(trans, root,
> - ins.objectid, ins.offset,
> - 0, root->root_key.objectid,
> + btrfs_init_generic_ref(&ref,
> + BTRFS_ADD_DELAYED_REF,
> + ins.objectid, ins.offset, 0);
> + btrfs_init_data_ref(&ref,
> + root->root_key.objectid,
> key->objectid, offset);
> + ret = btrfs_inc_extent_ref(trans, &ref);
> if (ret)
> goto out;
> } else {
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 8/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (6 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 7/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref() Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-02-11 13:05 ` Nikolay Borisov
2019-02-11 5:16 ` [PATCH v3 9/9] btrfs: qgroup: Don't scan leaf if we're modifying reloc tree Qu Wenruo
2019-04-03 16:29 ` [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list David Sterba
9 siblings, 1 reply; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
Similar to btrfs_inc_extent_ref(), just use btrfs_ref to replace the
long parameter list and the confusing @owner parameter.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/ctree.h | 5 +---
fs/btrfs/extent-tree.c | 52 +++++++++++++++++++-----------------------
fs/btrfs/file.c | 22 ++++++++++--------
fs/btrfs/inode.c | 13 +++++++----
fs/btrfs/relocation.c | 25 ++++++++++++--------
5 files changed, 61 insertions(+), 56 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index cd90f7cbc8f2..84a20abd0811 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2744,10 +2744,7 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info,
u64 bytenr, u64 num_bytes, u64 flags,
int level, int is_data);
-int btrfs_free_extent(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
- u64 owner, u64 offset);
+int btrfs_free_extent(struct btrfs_trans_handle *trans, struct btrfs_ref *ref);
int btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
u64 start, u64 len, int delalloc);
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index d08186ec6410..e2f34a8f5e91 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3290,9 +3290,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
if (inc)
ret = btrfs_inc_extent_ref(trans, &generic_ref);
else
- ret = btrfs_free_extent(trans, root, bytenr,
- num_bytes, parent, ref_root,
- key.objectid, key.offset);
+ ret = btrfs_free_extent(trans, &generic_ref);
if (ret)
goto fail;
} else {
@@ -3306,9 +3304,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
if (inc)
ret = btrfs_inc_extent_ref(trans, &generic_ref);
else
- ret = btrfs_free_extent(trans, root, bytenr,
- num_bytes, parent, ref_root,
- level - 1, 0);
+ ret = btrfs_free_extent(trans, &generic_ref);
if (ret)
goto fail;
}
@@ -7340,47 +7336,43 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
}
/* Can return -ENOMEM */
-int btrfs_free_extent(struct btrfs_trans_handle *trans,
- struct btrfs_root *root,
- u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
- u64 owner, u64 offset)
+int btrfs_free_extent(struct btrfs_trans_handle *trans, struct btrfs_ref *ref)
{
- struct btrfs_fs_info *fs_info = root->fs_info;
- struct btrfs_ref generic_ref = { 0 };
+ struct btrfs_fs_info *fs_info = trans->fs_info;
int old_ref_mod, new_ref_mod;
int ret;
if (btrfs_is_testing(fs_info))
return 0;
- btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF, bytenr,
- num_bytes, parent);
- generic_ref.real_root = root->root_key.objectid;
/*
* tree log blocks never actually go into the extent allocation
* tree, just update pinning info and exit early.
*/
- if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
- WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
+ if ((ref->type == BTRFS_REF_METADATA &&
+ ref->tree_ref.root == BTRFS_TREE_LOG_OBJECTID) ||
+ (ref->type == BTRFS_REF_DATA &&
+ ref->data_ref.ref_root == BTRFS_TREE_LOG_OBJECTID)) {
/* unlocks the pinned mutex */
- btrfs_pin_extent(fs_info, bytenr, num_bytes, 1);
+ btrfs_pin_extent(fs_info, ref->bytenr, ref->len, 1);
old_ref_mod = new_ref_mod = 0;
ret = 0;
- } else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
- btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
- ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
+ } else if (ref->type == BTRFS_REF_METADATA) {
+ ret = btrfs_add_delayed_tree_ref(trans, ref, NULL,
&old_ref_mod, &new_ref_mod);
} else {
- btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
- ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
+ ret = btrfs_add_delayed_data_ref(trans, ref, 0,
&old_ref_mod, &new_ref_mod);
}
- if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
- btrfs_ref_tree_mod(fs_info, &generic_ref);
+ if (!((ref->type == BTRFS_REF_METADATA &&
+ ref->tree_ref.root == BTRFS_TREE_LOG_OBJECTID) ||
+ (ref->type == BTRFS_REF_DATA &&
+ ref->data_ref.ref_root == BTRFS_TREE_LOG_OBJECTID)))
+ btrfs_ref_tree_mod(fs_info, ref);
if (ret == 0 && old_ref_mod >= 0 && new_ref_mod < 0)
- add_pinned_bytes(fs_info, &generic_ref);
+ add_pinned_bytes(fs_info, ref);
return ret;
}
@@ -8957,6 +8949,7 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,
u64 parent;
struct btrfs_key key;
struct btrfs_key first_key;
+ struct btrfs_ref ref = { 0 };
struct extent_buffer *next;
int level = wc->level;
int reada = 0;
@@ -9102,9 +9095,10 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,
ret);
}
}
- ret = btrfs_free_extent(trans, root, bytenr, fs_info->nodesize,
- parent, root->root_key.objectid,
- level - 1, 0);
+ btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, bytenr,
+ fs_info->nodesize, parent);
+ btrfs_init_tree_ref(&ref, level - 1, root->root_key.objectid);
+ ret = btrfs_free_extent(trans, &ref);
if (ret)
goto out_unlock;
}
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 2aba5980f522..f032f123be53 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -997,11 +997,14 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
extent_end = ALIGN(extent_end,
fs_info->sectorsize);
} else if (update_refs && disk_bytenr > 0) {
- ret = btrfs_free_extent(trans, root,
- disk_bytenr, num_bytes, 0,
+ btrfs_init_generic_ref(&ref,
+ BTRFS_DROP_DELAYED_REF,
+ disk_bytenr, num_bytes, 0);
+ btrfs_init_data_ref(&ref,
root->root_key.objectid,
- key.objectid, key.offset -
- extent_offset);
+ key.objectid,
+ key.offset - extent_offset);
+ ret = btrfs_free_extent(trans, &ref);
BUG_ON(ret); /* -ENOMEM */
inode_sub_bytes(inode,
extent_end - key.offset);
@@ -1318,6 +1321,9 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
other_start = end;
other_end = 0;
+ btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, bytenr,
+ num_bytes, 0);
+ btrfs_init_data_ref(&ref, root->root_key.objectid, ino, orig_offset);
if (extent_mergeable(leaf, path->slots[0] + 1,
ino, bytenr, orig_offset,
&other_start, &other_end)) {
@@ -1328,9 +1334,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
extent_end = other_end;
del_slot = path->slots[0] + 1;
del_nr++;
- ret = btrfs_free_extent(trans, root, bytenr, num_bytes,
- 0, root->root_key.objectid,
- ino, orig_offset);
+ ret = btrfs_free_extent(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out;
@@ -1348,9 +1352,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
key.offset = other_start;
del_slot = path->slots[0];
del_nr++;
- ret = btrfs_free_extent(trans, root, bytenr, num_bytes,
- 0, root->root_key.objectid,
- ino, orig_offset);
+ ret = btrfs_free_extent(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
goto out;
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 95535ba375f8..bc01584791fd 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4720,12 +4720,17 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
if (found_extent &&
(test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
root == fs_info->tree_root)) {
+ struct btrfs_ref ref = { 0 };
+
btrfs_set_path_blocking(path);
bytes_deleted += extent_num_bytes;
- ret = btrfs_free_extent(trans, root, extent_start,
- extent_num_bytes, 0,
- btrfs_header_owner(leaf),
- ino, extent_offset);
+
+ btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF,
+ extent_start, extent_num_bytes, 0);
+ ref.real_root = root->root_key.objectid;
+ btrfs_init_data_ref(&ref, btrfs_header_owner(leaf),
+ ino, extent_offset);
+ ret = btrfs_free_extent(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
break;
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index b82d532d8e5e..3b93a4585b70 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1716,9 +1716,12 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
break;
}
- ret = btrfs_free_extent(trans, root, bytenr, num_bytes,
- parent, btrfs_header_owner(leaf),
- key.objectid, key.offset);
+ btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, bytenr,
+ num_bytes, parent);
+ ref.real_root = root->root_key.objectid;
+ btrfs_init_data_ref(&ref, btrfs_header_owner(leaf),
+ key.objectid, key.offset);
+ ret = btrfs_free_extent(trans, &ref);
if (ret) {
btrfs_abort_transaction(trans, ret);
break;
@@ -1932,14 +1935,18 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
ret = btrfs_inc_extent_ref(trans, &ref);
BUG_ON(ret);
- ret = btrfs_free_extent(trans, src, new_bytenr, blocksize,
- path->nodes[level]->start,
- src->root_key.objectid, level - 1, 0);
+ btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, new_bytenr,
+ blocksize, path->nodes[level]->start);
+ btrfs_init_tree_ref(&ref, level - 1, src->root_key.objectid);
+ ref.skip_qgroup = true;
+ ret = btrfs_free_extent(trans, &ref);
BUG_ON(ret);
- ret = btrfs_free_extent(trans, dest, old_bytenr, blocksize,
- 0, dest->root_key.objectid, level - 1,
- 0);
+ btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, old_bytenr,
+ blocksize, 0);
+ btrfs_init_tree_ref(&ref, level - 1, dest->root_key.objectid);
+ ref.skip_qgroup = true;
+ ret = btrfs_free_extent(trans, &ref);
BUG_ON(ret);
btrfs_unlock_up_safe(path, 0);
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 8/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
2019-02-11 5:16 ` [PATCH v3 8/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent() Qu Wenruo
@ 2019-02-11 13:05 ` Nikolay Borisov
0 siblings, 0 replies; 24+ messages in thread
From: Nikolay Borisov @ 2019-02-11 13:05 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 11.02.19 г. 7:16 ч., Qu Wenruo wrote:
> Similar to btrfs_inc_extent_ref(), just use btrfs_ref to replace the
> long parameter list and the confusing @owner parameter.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/ctree.h | 5 +---
> fs/btrfs/extent-tree.c | 52 +++++++++++++++++++-----------------------
> fs/btrfs/file.c | 22 ++++++++++--------
> fs/btrfs/inode.c | 13 +++++++----
> fs/btrfs/relocation.c | 25 ++++++++++++--------
> 5 files changed, 61 insertions(+), 56 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index cd90f7cbc8f2..84a20abd0811 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2744,10 +2744,7 @@ int btrfs_set_disk_extent_flags(struct btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info,
> u64 bytenr, u64 num_bytes, u64 flags,
> int level, int is_data);
> -int btrfs_free_extent(struct btrfs_trans_handle *trans,
> - struct btrfs_root *root,
> - u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
> - u64 owner, u64 offset);
> +int btrfs_free_extent(struct btrfs_trans_handle *trans, struct btrfs_ref *ref);
>
> int btrfs_free_reserved_extent(struct btrfs_fs_info *fs_info,
> u64 start, u64 len, int delalloc);
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index d08186ec6410..e2f34a8f5e91 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3290,9 +3290,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
> if (inc)
> ret = btrfs_inc_extent_ref(trans, &generic_ref);
> else
> - ret = btrfs_free_extent(trans, root, bytenr,
> - num_bytes, parent, ref_root,
> - key.objectid, key.offset);
> + ret = btrfs_free_extent(trans, &generic_ref);
> if (ret)
> goto fail;
> } else {
> @@ -3306,9 +3304,7 @@ static int __btrfs_mod_ref(struct btrfs_trans_handle *trans,
> if (inc)
> ret = btrfs_inc_extent_ref(trans, &generic_ref);
> else
> - ret = btrfs_free_extent(trans, root, bytenr,
> - num_bytes, parent, ref_root,
> - level - 1, 0);
> + ret = btrfs_free_extent(trans, &generic_ref);
> if (ret)
> goto fail;
> }
> @@ -7340,47 +7336,43 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
> }
>
> /* Can return -ENOMEM */
> -int btrfs_free_extent(struct btrfs_trans_handle *trans,
> - struct btrfs_root *root,
> - u64 bytenr, u64 num_bytes, u64 parent, u64 root_objectid,
> - u64 owner, u64 offset)
> +int btrfs_free_extent(struct btrfs_trans_handle *trans, struct btrfs_ref *ref)
> {
> - struct btrfs_fs_info *fs_info = root->fs_info;
> - struct btrfs_ref generic_ref = { 0 };
> + struct btrfs_fs_info *fs_info = trans->fs_info;
> int old_ref_mod, new_ref_mod;
> int ret;
>
> if (btrfs_is_testing(fs_info))
> return 0;
>
> - btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF, bytenr,
> - num_bytes, parent);
> - generic_ref.real_root = root->root_key.objectid;
> /*
> * tree log blocks never actually go into the extent allocation
> * tree, just update pinning info and exit early.
> */
> - if (root_objectid == BTRFS_TREE_LOG_OBJECTID) {
> - WARN_ON(owner >= BTRFS_FIRST_FREE_OBJECTID);
> + if ((ref->type == BTRFS_REF_METADATA &&
> + ref->tree_ref.root == BTRFS_TREE_LOG_OBJECTID) ||
> + (ref->type == BTRFS_REF_DATA &&
> + ref->data_ref.ref_root == BTRFS_TREE_LOG_OBJECTID)) {
> /* unlocks the pinned mutex */
> - btrfs_pin_extent(fs_info, bytenr, num_bytes, 1);
> + btrfs_pin_extent(fs_info, ref->bytenr, ref->len, 1);
> old_ref_mod = new_ref_mod = 0;
> ret = 0;
> - } else if (owner < BTRFS_FIRST_FREE_OBJECTID) {
> - btrfs_init_tree_ref(&generic_ref, (int)owner, root_objectid);
> - ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL,
> + } else if (ref->type == BTRFS_REF_METADATA) {
> + ret = btrfs_add_delayed_tree_ref(trans, ref, NULL,
> &old_ref_mod, &new_ref_mod);
> } else {
> - btrfs_init_data_ref(&generic_ref, root_objectid, owner, offset);
> - ret = btrfs_add_delayed_data_ref(trans, &generic_ref, 0,
> + ret = btrfs_add_delayed_data_ref(trans, ref, 0,
> &old_ref_mod, &new_ref_mod);
> }
>
> - if (root_objectid != BTRFS_TREE_LOG_OBJECTID)
> - btrfs_ref_tree_mod(fs_info, &generic_ref);
> + if (!((ref->type == BTRFS_REF_METADATA &&
> + ref->tree_ref.root == BTRFS_TREE_LOG_OBJECTID) ||
> + (ref->type == BTRFS_REF_DATA &&
> + ref->data_ref.ref_root == BTRFS_TREE_LOG_OBJECTID)))
> + btrfs_ref_tree_mod(fs_info, ref);
>
> if (ret == 0 && old_ref_mod >= 0 && new_ref_mod < 0)
> - add_pinned_bytes(fs_info, &generic_ref);
> + add_pinned_bytes(fs_info, ref);
>
> return ret;
> }
> @@ -8957,6 +8949,7 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,
> u64 parent;
> struct btrfs_key key;
> struct btrfs_key first_key;
> + struct btrfs_ref ref = { 0 };
> struct extent_buffer *next;
> int level = wc->level;
> int reada = 0;
> @@ -9102,9 +9095,10 @@ static noinline int do_walk_down(struct btrfs_trans_handle *trans,
> ret);
> }
> }
> - ret = btrfs_free_extent(trans, root, bytenr, fs_info->nodesize,
> - parent, root->root_key.objectid,
> - level - 1, 0);
> + btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, bytenr,
> + fs_info->nodesize, parent);
> + btrfs_init_tree_ref(&ref, level - 1, root->root_key.objectid);
> + ret = btrfs_free_extent(trans, &ref);
> if (ret)
> goto out_unlock;
> }
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 2aba5980f522..f032f123be53 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -997,11 +997,14 @@ int __btrfs_drop_extents(struct btrfs_trans_handle *trans,
> extent_end = ALIGN(extent_end,
> fs_info->sectorsize);
> } else if (update_refs && disk_bytenr > 0) {
> - ret = btrfs_free_extent(trans, root,
> - disk_bytenr, num_bytes, 0,
> + btrfs_init_generic_ref(&ref,
> + BTRFS_DROP_DELAYED_REF,
> + disk_bytenr, num_bytes, 0);
> + btrfs_init_data_ref(&ref,
> root->root_key.objectid,
> - key.objectid, key.offset -
> - extent_offset);
> + key.objectid,
> + key.offset - extent_offset);
> + ret = btrfs_free_extent(trans, &ref);
> BUG_ON(ret); /* -ENOMEM */
> inode_sub_bytes(inode,
> extent_end - key.offset);
> @@ -1318,6 +1321,9 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
>
> other_start = end;
> other_end = 0;
> + btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, bytenr,
> + num_bytes, 0);
> + btrfs_init_data_ref(&ref, root->root_key.objectid, ino, orig_offset);
> if (extent_mergeable(leaf, path->slots[0] + 1,
> ino, bytenr, orig_offset,
> &other_start, &other_end)) {
> @@ -1328,9 +1334,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
> extent_end = other_end;
> del_slot = path->slots[0] + 1;
> del_nr++;
> - ret = btrfs_free_extent(trans, root, bytenr, num_bytes,
> - 0, root->root_key.objectid,
> - ino, orig_offset);
> + ret = btrfs_free_extent(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> goto out;
> @@ -1348,9 +1352,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
> key.offset = other_start;
> del_slot = path->slots[0];
> del_nr++;
> - ret = btrfs_free_extent(trans, root, bytenr, num_bytes,
> - 0, root->root_key.objectid,
> - ino, orig_offset);
> + ret = btrfs_free_extent(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> goto out;
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 95535ba375f8..bc01584791fd 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4720,12 +4720,17 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
> if (found_extent &&
> (test_bit(BTRFS_ROOT_REF_COWS, &root->state) ||
> root == fs_info->tree_root)) {
> + struct btrfs_ref ref = { 0 };
> +
> btrfs_set_path_blocking(path);
> bytes_deleted += extent_num_bytes;
> - ret = btrfs_free_extent(trans, root, extent_start,
> - extent_num_bytes, 0,
> - btrfs_header_owner(leaf),
> - ino, extent_offset);
> +
> + btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF,
> + extent_start, extent_num_bytes, 0);
> + ref.real_root = root->root_key.objectid;
> + btrfs_init_data_ref(&ref, btrfs_header_owner(leaf),
> + ino, extent_offset);
> + ret = btrfs_free_extent(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> break;
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index b82d532d8e5e..3b93a4585b70 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1716,9 +1716,12 @@ int replace_file_extents(struct btrfs_trans_handle *trans,
> break;
> }
>
> - ret = btrfs_free_extent(trans, root, bytenr, num_bytes,
> - parent, btrfs_header_owner(leaf),
> - key.objectid, key.offset);
> + btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, bytenr,
> + num_bytes, parent);
> + ref.real_root = root->root_key.objectid;
> + btrfs_init_data_ref(&ref, btrfs_header_owner(leaf),
> + key.objectid, key.offset);
> + ret = btrfs_free_extent(trans, &ref);
> if (ret) {
> btrfs_abort_transaction(trans, ret);
> break;
> @@ -1932,14 +1935,18 @@ int replace_path(struct btrfs_trans_handle *trans, struct reloc_control *rc,
> ret = btrfs_inc_extent_ref(trans, &ref);
> BUG_ON(ret);
>
> - ret = btrfs_free_extent(trans, src, new_bytenr, blocksize,
> - path->nodes[level]->start,
> - src->root_key.objectid, level - 1, 0);
> + btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, new_bytenr,
> + blocksize, path->nodes[level]->start);
> + btrfs_init_tree_ref(&ref, level - 1, src->root_key.objectid);
> + ref.skip_qgroup = true;
> + ret = btrfs_free_extent(trans, &ref);
> BUG_ON(ret);
>
> - ret = btrfs_free_extent(trans, dest, old_bytenr, blocksize,
> - 0, dest->root_key.objectid, level - 1,
> - 0);
> + btrfs_init_generic_ref(&ref, BTRFS_DROP_DELAYED_REF, old_bytenr,
> + blocksize, 0);
> + btrfs_init_tree_ref(&ref, level - 1, dest->root_key.objectid);
> + ref.skip_qgroup = true;
> + ret = btrfs_free_extent(trans, &ref);
> BUG_ON(ret);
>
> btrfs_unlock_up_safe(path, 0);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 9/9] btrfs: qgroup: Don't scan leaf if we're modifying reloc tree
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (7 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 8/9] btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent() Qu Wenruo
@ 2019-02-11 5:16 ` Qu Wenruo
2019-04-03 16:29 ` [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list David Sterba
9 siblings, 0 replies; 24+ messages in thread
From: Qu Wenruo @ 2019-02-11 5:16 UTC (permalink / raw)
To: linux-btrfs
Since reloc tree doesn't contribute to qgroup numbers, just skip them.
This should catch the final cause of unnecessary data refs for qgroup +
metadata balance.
The 4G data 16 snapshots test (*) should explain it pretty well:
| delayed subtree | refactor delayed ref | this patch
---------------------------------------------------------------------
relocated | 22653 | 22673 | 22744
qgroup dirty | 122792 | 48360 | 70
time | 24.494 | 11.606 | 3.944
Finally, we're at the stage where qgroup + metadata balance cost no
obvious overhead.
*: Test environment
Test VM:
- vRAM 8G
- vCPU 8
- block dev vitrio-blk, 'unsafe' cache mode
- host block 850evo
Test workload
- Copy 4G data from /usr/ to one subvolume
- Create 16 snapshots of that subvolume, and modify 3 files in each
snapshot
- Enable quota, rescan
- Time "btrfs balance start -m"
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent-tree.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index e2f34a8f5e91..0dde0cbc1622 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -9188,11 +9188,13 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
else
ret = btrfs_dec_ref(trans, root, eb, 0);
BUG_ON(ret); /* -ENOMEM */
- ret = btrfs_qgroup_trace_leaf_items(trans, eb);
- if (ret) {
- btrfs_err_rl(fs_info,
- "error %d accounting leaf items. Quota is out of sync, rescan required.",
+ if (is_fstree(root->root_key.objectid)) {
+ ret = btrfs_qgroup_trace_leaf_items(trans, eb);
+ if (ret) {
+ btrfs_err_rl(fs_info,
+"error %d accounting leaf items. Quota is out of sync, rescan required.",
ret);
+ }
}
}
/* make block locked assertion in clean_tree_block happy */
--
2.20.1
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list
2019-02-11 5:16 [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list Qu Wenruo
` (8 preceding siblings ...)
2019-02-11 5:16 ` [PATCH v3 9/9] btrfs: qgroup: Don't scan leaf if we're modifying reloc tree Qu Wenruo
@ 2019-04-03 16:29 ` David Sterba
2019-04-04 1:12 ` Qu Wenruo
2019-04-04 6:44 ` Qu Wenruo
9 siblings, 2 replies; 24+ messages in thread
From: David Sterba @ 2019-04-03 16:29 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Mon, Feb 11, 2019 at 01:16:44PM +0800, Qu Wenruo wrote:
> Qu Wenruo (9):
> btrfs: delayed-ref: Introduce better documented delayed ref structures
> btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref
> btrfs: delayed-ref: Use btrfs_ref to refactor
> btrfs_add_delayed_tree_ref()
> btrfs: delayed-ref: Use btrfs_ref to refactor
> btrfs_add_delayed_data_ref()
> btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
> btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()
> btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
> btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
If you would like to get the above merged (ie. the parameter cleanup) to
5.2, please refresh and resend on top of current misc-next. I tested
this patchset back then and it did not pass fstests but I haven't
investigated. As this patchset does only a transformation, I'm
suspecting some typo so another look from you would be helpful. Thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list
2019-04-03 16:29 ` [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list David Sterba
@ 2019-04-04 1:12 ` Qu Wenruo
2019-04-04 6:44 ` Qu Wenruo
1 sibling, 0 replies; 24+ messages in thread
From: Qu Wenruo @ 2019-04-04 1:12 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 1182 bytes --]
On 2019/4/4 上午12:29, David Sterba wrote:
> On Mon, Feb 11, 2019 at 01:16:44PM +0800, Qu Wenruo wrote:
>> Qu Wenruo (9):
>> btrfs: delayed-ref: Introduce better documented delayed ref structures
>> btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref
>> btrfs: delayed-ref: Use btrfs_ref to refactor
>> btrfs_add_delayed_tree_ref()
>> btrfs: delayed-ref: Use btrfs_ref to refactor
>> btrfs_add_delayed_data_ref()
>> btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
>> btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()
>> btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
>> btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
>
> If you would like to get the above merged (ie. the parameter cleanup) to
> 5.2, please refresh and resend on top of current misc-next. I tested
> this patchset back then and it did not pass fstests but I haven't
> investigated. As this patchset does only a transformation, I'm
> suspecting some typo so another look from you would be helpful. Thanks.
I'll rebase and retest the patchset.
Thanks for the mentioning.
Qu
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list
2019-04-03 16:29 ` [PATCH v3 0/9] btrfs: Refactor delayed ref parameter list David Sterba
2019-04-04 1:12 ` Qu Wenruo
@ 2019-04-04 6:44 ` Qu Wenruo
1 sibling, 0 replies; 24+ messages in thread
From: Qu Wenruo @ 2019-04-04 6:44 UTC (permalink / raw)
To: dsterba, Qu Wenruo, linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 1482 bytes --]
On 2019/4/4 上午12:29, David Sterba wrote:
> On Mon, Feb 11, 2019 at 01:16:44PM +0800, Qu Wenruo wrote:
>> Qu Wenruo (9):
>> btrfs: delayed-ref: Introduce better documented delayed ref structures
>> btrfs: extent-tree: Open-code process_func in __btrfs_mod_ref
>> btrfs: delayed-ref: Use btrfs_ref to refactor
>> btrfs_add_delayed_tree_ref()
>> btrfs: delayed-ref: Use btrfs_ref to refactor
>> btrfs_add_delayed_data_ref()
>> btrfs: ref-verify: Use btrfs_ref to refactor btrfs_ref_tree_mod()
>> btrfs: extent-tree: Use btrfs_ref to refactor add_pinned_bytes()
>> btrfs: extent-tree: Use btrfs_ref to refactor btrfs_inc_extent_ref()
>> btrfs: extent-tree: Use btrfs_ref to refactor btrfs_free_extent()
>
> If you would like to get the above merged (ie. the parameter cleanup) to
> 5.2, please refresh and resend on top of current misc-next. I tested
> this patchset back then and it did not pass fstests but I haven't
> investigated. As this patchset does only a transformation, I'm
> suspecting some typo so another look from you would be helpful. Thanks.
I have rebased the patchset and re-test with full fstests run.
It passes with no new regression.
Although without the initial report, I'm not 100% sure what's causing
problem for your old test.
While the new patchset passes full fstests, I'm going to send out the
patch anyway, hoping the initial fail is from my previous inode mode
check bug.
Thanks,
Qu
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread