[PATCH] drivers/base/memory: set mem->altmap after successful device registration

[PATCH] drivers/base/memory: set mem->altmap after successful device registration

[Georgi Djakov]

If __add_memory_block() fails at xa_store() (under memory pressure
for example), device_unregister() is called, which eventually
triggers memory_block_release() with mem->altmap still set, causing
a WARN_ON(mem->altmap).

WARNING: CPU: 1 PID: 76 at drivers/base/memory.c:96 memory_block_release+0x28/0x30
Call trace:
 memory_block_release+0x28/0x30 (P)
 kobject_put+0xc8/0x1bc
 device_unregister+0x2c/0x68
 add_memory_block+0x1b4/0x214
 create_memory_block_devices+0x9c/0xfc
 create_altmaps_and_memory_blocks+0x130/0x1cc
 add_memory_resource+0x10c/0x21c
 add_memory_driver_managed+0xb8/0xe8
 virtio_mem_add_memory+0xa4/0x194 [virtio_mem]
 virtio_mem_sbm_plug_and_add_mb+0xc8/0x16c [virtio_mem]
 virtio_mem_run_wq+0xea8/0x1088 [virtio_mem]
 process_scheduled_works+0x18c/0x300
 worker_thread+0x1dc/0x2b8
 kthread+0x1ac/0x1f4
 ret_from_fork+0x10/0x20

Fix this by delaying the assignment of mem->altmap until after
__add_memory_block() has succeeded.

Fixes: 1a8c64e11043 ("mm/memory_hotplug: embed vmem_altmap details in memory block")
Signed-off-by: Georgi Djakov <georgi.djakov@oss.qualcomm.com>
---
 drivers/base/memory.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 11d57cfa8d72..78e9600e6d45 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -807,7 +807,6 @@ static int add_memory_block(unsigned long block_id, int nid, unsigned long state
 	mem->start_section_nr = block_id * sections_per_block;
 	mem->state = state;
 	mem->nid = nid;
-	mem->altmap = altmap;
 	INIT_LIST_HEAD(&mem->group_next);
 
 #ifndef CONFIG_NUMA
@@ -825,6 +824,8 @@ static int add_memory_block(unsigned long block_id, int nid, unsigned long state
 	if (ret)
 		return ret;
 
+	mem->altmap = altmap;
+
 	if (group) {
 		mem->group = group;
 		list_add(&mem->group_next, &group->memory_blocks);

Re: [PATCH] drivers/base/memory: set mem->altmap after successful device registration

[David Hildenbrand (Arm)]

On 5/13/26 15:09, Georgi Djakov wrote:
> If __add_memory_block() fails at xa_store() (under memory pressure
> for example), device_unregister() is called, which eventually
> triggers memory_block_release() with mem->altmap still set, causing
> a WARN_ON(mem->altmap).
> 
> WARNING: CPU: 1 PID: 76 at drivers/base/memory.c:96 memory_block_release+0x28/0x30
> Call trace:
>  memory_block_release+0x28/0x30 (P)
>  kobject_put+0xc8/0x1bc
>  device_unregister+0x2c/0x68
>  add_memory_block+0x1b4/0x214
>  create_memory_block_devices+0x9c/0xfc
>  create_altmaps_and_memory_blocks+0x130/0x1cc
>  add_memory_resource+0x10c/0x21c
>  add_memory_driver_managed+0xb8/0xe8
>  virtio_mem_add_memory+0xa4/0x194 [virtio_mem]
>  virtio_mem_sbm_plug_and_add_mb+0xc8/0x16c [virtio_mem]
>  virtio_mem_run_wq+0xea8/0x1088 [virtio_mem]
>  process_scheduled_works+0x18c/0x300
>  worker_thread+0x1dc/0x2b8
>  kthread+0x1ac/0x1f4
>  ret_from_fork+0x10/0x20
> 
> Fix this by delaying the assignment of mem->altmap until after
> __add_memory_block() has succeeded.

Something does not add up here. How do you get an altmap with virtio-mem?


-- 
Cheers,

David

Re: [PATCH] drivers/base/memory: set mem->altmap after successful device registration

[David Hildenbrand (Arm)]

On 5/13/26 15:14, David Hildenbrand (Arm) wrote:
> On 5/13/26 15:09, Georgi Djakov wrote:
>> If __add_memory_block() fails at xa_store() (under memory pressure
>> for example), device_unregister() is called, which eventually
>> triggers memory_block_release() with mem->altmap still set, causing
>> a WARN_ON(mem->altmap).
>>
>> WARNING: CPU: 1 PID: 76 at drivers/base/memory.c:96 memory_block_release+0x28/0x30
>> Call trace:
>>  memory_block_release+0x28/0x30 (P)
>>  kobject_put+0xc8/0x1bc
>>  device_unregister+0x2c/0x68
>>  add_memory_block+0x1b4/0x214
>>  create_memory_block_devices+0x9c/0xfc
>>  create_altmaps_and_memory_blocks+0x130/0x1cc
>>  add_memory_resource+0x10c/0x21c
>>  add_memory_driver_managed+0xb8/0xe8
>>  virtio_mem_add_memory+0xa4/0x194 [virtio_mem]
>>  virtio_mem_sbm_plug_and_add_mb+0xc8/0x16c [virtio_mem]
>>  virtio_mem_run_wq+0xea8/0x1088 [virtio_mem]
>>  process_scheduled_works+0x18c/0x300
>>  worker_thread+0x1dc/0x2b8
>>  kthread+0x1ac/0x1f4
>>  ret_from_fork+0x10/0x20
>>
>> Fix this by delaying the assignment of mem->altmap until after
>> __add_memory_block() has succeeded.
> 
> Something does not add up here. How do you get an altmap with virtio-mem?

That patch itself makes sense, but the callchain does not make sense.

virtio_mem does not set MHP_MEMMAP_ON_MEMORY, so it should never reach
create_altmaps_and_memory_blocks()?

-- 
Cheers,

David