Re: [PATCH v5] btrfs: Turn delayed_nodes_tree into an XArray

[Gabriel Niebler <gniebler@suse.com>]

Am 20.04.22 um 09:09 schrieb Nikolay Borisov:
> On 19.04.22 г. 18:57 ч., Gabriel Niebler wrote:
>> @@ -1870,29 +1863,33 @@ void btrfs_kill_delayed_inode_items(struct 
>> btrfs_inode *inode)
>>   void btrfs_kill_all_delayed_nodes(struct btrfs_root *root)
>>   {
>> -    u64 inode_id = 0;
>> +    unsigned long index = 0;
>> +    struct btrfs_delayed_node *delayed_node;
>>       struct btrfs_delayed_node *delayed_nodes[8];
> 
> Actually in order for the new code to be correct this array needs to be 
> zero-initialized upon every iteration of the while() loop. That's 
> because as it stands ATM delayed_nodes would retain the value from the 
> previous iteration. What this could lead to is on the last iteration we 
> potentially have 8 nodes in delayed_nodes which have been freed, so if 
> in the last iteration of the xa_for_each_start we copy only 3 nodes the 
> array would still have 8 nodes in total - 3 from the  last iteration and 
> 5 stale from the previous since they haven't been cleared out. So the 
> final for() loop which does the freeing would incur a double free on the 
> 5 already-freed entries.

Oh, you're right! I must not have thought about the *last* iteration, 
but yeah: that's a problem!

> To fix this you either need to clear delayed_nodes[i] in the final for 
> loop, so simply define it inside the while() like so:
> 
> struct btrfs_delayed_node *delayed_nodes[8] = {};

I will do that. Thanks for introducing me to this neat trick how to 
initialise an array to all NULLs, BTW. I admit I had to look it up, but 
now it'll save me from writing another loop...

<snip>

I'll also use the opportunity to fix the whitespace issues.

Sorry for the delay, BTW, don't know why I didn't see the message earlier.