On 2020/3/4 下午9:06, Nikolay Borisov wrote:
>
>
> On 2.03.20 г. 11:45 ч., Qu Wenruo wrote:
>> Bytenr and level are essential parameters for backref_node, thus it
>> makes sense to initial them at alloc time.
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>
> Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
>
>> ---
>> fs/btrfs/relocation.c | 25 ++++++++++++-------------
>> 1 file changed, 12 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index c76849409c81..67a4a61eb86a 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -308,10 +308,12 @@ static void backref_cache_cleanup(struct backref_cache *cache)
>> ASSERT(!cache->nr_edges);
>> }
>>
>> -static struct backref_node *alloc_backref_node(struct backref_cache *cache)
>> +static struct backref_node *alloc_backref_node(struct backref_cache *cache,
>> + u64 bytenr, int level)
>> {
>> struct backref_node *node;
>>
>> + ASSERT(level >= 0 && level < BTRFS_MAX_LEVEL);
>> node = kzalloc(sizeof(*node), GFP_NOFS);
>> if (node) {
>> INIT_LIST_HEAD(&node->list);
>> @@ -319,6 +321,9 @@ static struct backref_node *alloc_backref_node(struct backref_cache *cache)
>> INIT_LIST_HEAD(&node->lower);
>> RB_CLEAR_NODE(&node->rb_node);
>> cache->nr_nodes++;
>> +
>> + node->level = level;
>> + node->bytenr = bytenr;
>> }
>
> nit: One suggestion for this function, feel free to ignore:
>
> What if you do it:
> if (!node)
> return NULL
I didn't notice we can do that.
Looks like the style I prefer too.
I'll use that style in next version.
Thanks,
Qu
>
> and then have the initialization happen with just 1 level of nesting
> inside the function and not 2. That function is short and easy to
> understand but it might be a good time to refactor like that. If you
> think it's redundant then don't bother.
>
>> return node;
>> }
>
> <snip>
>
