Re: [bug report] memory leak of xa_node in collapse_file() when rollbacks

[Jinjiang Tu <tujinjiang@huawei.com>]

在 2025/12/31 5:03, David Hildenbrand (Red Hat) 写道:
> On 12/27/25 02:24, Jinjiang Tu wrote:
>>
>> 在 2025/12/25 12:15, Shardul Bankar 写道:
>>> On Thu, 2025-12-18 at 21:11 +0800, Jinjiang Tu wrote:
>>>> 在 2025/12/18 20:49, David Hildenbrand (Red Hat) 写道:
>>>>>    Thanks for checking. I thought that was also discussed as part of
>>>>> the other fix.
>>>>>       See [2] where we have
>>>>>       "Note: This fixes the leak of pre-allocated nodes. A 
>>>>> separate fix
>>>>> will
>>>>>    be needed to clean up empty nodes that were inserted into the tree
>>>>> by
>>>>>    xas_create_range() but never populated."
>>>>>       Is that the issue you are describing? (sounds like it, but I 
>>>>> only
>>>>> skimmed over the details).
>>>>>       CCing Shardul.
>>>> Yes, the same issue. As I descirbed in the first email:
>>>> "
>>>> At first, I tried to destory the empty nodes when collapse_file()
>>>> goes to rollback path. However,
>>>> collapse_file() only holds xarray lock and may release the lock, so
>>>> we couldn't prevent concurrent
>>>> call of collapse_file(), so the deleted empty nodes may be needed by
>>>> other collapse_file() calls.
>>>> "
>>> Hi David, Jinjiang,
>>>
>>> As Jinjiang mentioned, this appears to address what I had originally
>>> referred to in the "Note:" in [1].
>>>
>>> Just to clarify the context of the "Note:", that was based on my
>>> assumption at the time that such empty nodes would be considered leaks.
>>> After Dev’s feedback in [2]:
>>> "No "fix" is needed in this case, the empty nodes are there in the tree
>>> and there is no leak."
>>>
>>> and looking at the older discussion in [3]:
>>> "There's nothing to free; if a node is allocated, then it's stored in
>>> the tree where it can later be found and reused. "
>>
>> However, if the empty nodes aren't reused, When the file is deleted,
>> shmem_evict_inode()->shmem_truncate_range() traverses all entries and
>> calls xas_store(xas, NULL) to delete, if the leaf xa_node that stores
>> deleted entry becomes empty, xas_store() will automatically delete the
>> empty node and delete it's parent is empty too, until parent node isn't
>> empty. shmem_evict_inode() won't traverse the empty nodes created by
>> xas_create_range() due to these nodes doesn't store any entries.
>
> So you're saying that nothing/nobody would clean up these xarray 
> entries and we'd be leaking them?
Yes.
>
> "struct xarray" documents "If all of the entries in the array are 
> NULL, @xa_head is a NULL pointer.". So we depend on all entries being 
> set to NULL in order to properly cleanup/free the xarray automatically.
>
Yes