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

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

On 12/18/25 13:18, Jinjiang Tu wrote:
> 
> 在 2025/12/18 19:51, David Hildenbrand (Red Hat) 写道:
>> On 12/18/25 12:45, Jinjiang Tu wrote:
>>> I encountered a memory leak issue caused by xas_create_range().
>>>
>>> collapse_file() calls xas_create_range() to pre-create all slots needed.
>>> If collapse_file() finally fails, these pre-created slots are empty 
>>> nodes
>>> and aren't destroyed.
>>>
>>> I can reproduce it with following steps.
>>> 1) create file /tmp/test_madvise_collapse and ftruncate to 4MB size, 
>>> and then mmap the file
>>> 2) memset for the first 2MB
>>> 3) madvise(MADV_COLLAPSE) for the second 2MB
>>> 4) unlink the file
>>>
>>> in 3), collapse_file() calls xas_create_range() to expand xarray 
>>> depth, and fails to collapse
>>> due to the whole 2M region is empty, the code is as following:
>>>
>>> collapse_file()
>>>     for (index = start; index < end;) {
>>>         xas_set(&xas, index);
>>>         folio = xas_load(&xas);
>>>
>>>         VM_BUG_ON(index != xas.xa_index);
>>>         if (is_shmem) {
>>>             if (!folio) {
>>>                 /*
>>>                  * Stop if extent has been truncated or
>>>                  * hole-punched, and is now completely
>>>                  * empty.
>>>                  */
>>>                 if (index == start) {
>>>                     if (!xas_next_entry(&xas, end - 1)) {
>>>                         result = SCAN_TRUNCATED;
>>>                         goto xa_locked;
>>>                     }
>>>                 }
>>>                 ...
>>>             }
>>>
>>>
>>> collapse_file() rollback path doesn't destroy the pre-created empty 
>>> nodes.
>>>
>>> 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 emtry, 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. As a result, these empty nodes are leaked.
>>>
>>> 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.
>>>
>>> IIUC, xas_create_range() is used to guarantee the xas_store(&xas, 
>>> new_folio); succeeds. Could we
>>> remove xas_create_range() call and just rollback when we fail to 
>>> xas_store?
>>
>> Hi,
>>
>> thanks for the report.
>>
>> Is that what [1] is fixing?
>>
>> [1] https://lore.kernel.org/linux-mm/20251204142625.1763372-1- 
>> shardul.b@mpiricsoftware.com/
>>
> No, this patch fixes memory leak caused by xas->xa_alloc allocated by xas_nomem() and the xa_node
> isn't installed into xarray.
> 
> In my case, the leaked xa_nodes have been installed into xarray by xas_create_range().

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.


[2] 
https://lore.kernel.org/linux-mm/20251123132727.3262731-1-shardul.b@mpiricsoftware.com/

-- 
Cheers

David