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

["David Hildenbrand (Red Hat)" <david@kernel.org>]

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?

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

-- 
Cheers

David