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

[Jinjiang Tu <tujinjiang@huawei.com>]

在 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.

>
> my updated understanding is that there is no leak in this case- the
> nodes remain valid and reusable, and therefore do not require a
> separate fix.
>
> David could you correct me if I am mistaken?
>
> [1]
> https://lore.kernel.org/linux-mm/20251123132727.3262731-1-shardul.b@mpiricsoftware.com/
>
> [2]
> https://lore.kernel.org/linux-mm/57cbf887-d181-418b-a6c7-9f3eff5d632a@arm.com/
>
> [3]
> https://lore.kernel.org/all/Ys1r06szkVi3QEai@casper.infradead.org/
>
> Thanks,
> Shardul
>