[v7,4/6] numa: Introduce and use ram_block_notify_remap()

Commit Message

William Roche Feb. 1, 2025, 9:57 a.m. UTC

From: David Hildenbrand <[e-mail]>

Notify registered listeners about the remap at the end of
qemu_ram_remap() so e.g., a memory backend can re-apply its
settings correctly.

Signed-off-by: David Hildenbrand <[e-mail]>
Signed-off-by: William Roche <[e-mail]>
---
 hw/core/numa.c         | 11 +++++++++++
 include/exec/ramlist.h |  3 +++
 system/physmem.c       |  1 +
 3 files changed, 15 insertions(+)

Comments

Peter Xu Feb. 4, 2025, 5:17 p.m. UTC | #1

On Sat, Feb 01, 2025 at 09:57:24AM +0000, “William Roche wrote:
> From: David Hildenbrand <[e-mail]>
> 
> Notify registered listeners about the remap at the end of
> qemu_ram_remap() so e.g., a memory backend can re-apply its
> settings correctly.
> 
> Signed-off-by: David Hildenbrand <[e-mail]>
> Signed-off-by: William Roche <[e-mail]>

IIUC logically speaking we don't need a global remap notifier - here a
per-ramblock notifier looks more reasonable, like RAMBlock.resized().
It'll change the notify path from O(N**2) to O(N).  After all, backend1's
notifier won't care other ramblock's remap() events but only itself's.

It's not a huge deal as I expect we don't have a huge amount of ramblocks,
but looks like this series will miss the recent pull anyway..  so let me
comment as so on this one for consideration when respin.

We could also merge partial of the series to fix hugetlb poisoning first,
as this one looks like can be separately done too.

Thanks,

David Hildenbrand Feb. 4, 2025, 5:42 p.m. UTC | #2

On 04.02.25 18:17, Peter Xu wrote:
> On Sat, Feb 01, 2025 at 09:57:24AM +0000, “William Roche wrote:
>> From: David Hildenbrand <[e-mail]>
>>
>> Notify registered listeners about the remap at the end of
>> qemu_ram_remap() so e.g., a memory backend can re-apply its
>> settings correctly.
>>
>> Signed-off-by: David Hildenbrand <[e-mail]>
>> Signed-off-by: William Roche <[e-mail]>
> 
> IIUC logically speaking we don't need a global remap notifier - here a
> per-ramblock notifier looks more reasonable, like RAMBlock.resized().

Right. Note that qemu_ram_resize() also triggers global notifiers.

> It'll change the notify path from O(N**2) to O(N).  After all, backend1's
> notifier won't care other ramblock's remap() events but only itself's.
> 
> It's not a huge deal as I expect we don't have a huge amount of ramblocks,
> but looks like this series will miss the recent pull anyway..  so let me
> comment as so on this one for consideration when respin.

... and ram remap during reboot is not particularly the fast path we 
care about (or should be caring about).

> 
> We could also merge partial of the series to fix hugetlb poisoning first,
> as this one looks like can be separately done too.

hugetlb frequently uses preallocation, and the remaining patches in this 
series make sure preallocation after remapping happens.

Patch

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 218576f745..003bcd8a66 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -895,3 +895,14 @@  void ram_block_notify_resize(void *host, size_t old_size, size_t new_size)
         }
     }
 }
+
+void ram_block_notify_remap(void *host, size_t offset, size_t size)
+{
+    RAMBlockNotifier *notifier;
+
+    QLIST_FOREACH(notifier, &ram_list.ramblock_notifiers, next) {
+        if (notifier->ram_block_remapped) {
+            notifier->ram_block_remapped(notifier, host, offset, size);
+        }
+    }
+}
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index d9cfe530be..c1dc785a57 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -72,6 +72,8 @@  struct RAMBlockNotifier {
                               size_t max_size);
     void (*ram_block_resized)(RAMBlockNotifier *n, void *host, size_t old_size,
                               size_t new_size);
+    void (*ram_block_remapped)(RAMBlockNotifier *n, void *host, size_t offset,
+                               size_t size);
     QLIST_ENTRY(RAMBlockNotifier) next;
 };
 
@@ -80,6 +82,7 @@  void ram_block_notifier_remove(RAMBlockNotifier *n);
 void ram_block_notify_add(void *host, size_t size, size_t max_size);
 void ram_block_notify_remove(void *host, size_t size, size_t max_size);
 void ram_block_notify_resize(void *host, size_t old_size, size_t new_size);
+void ram_block_notify_remap(void *host, size_t offset, size_t size);
 
 GString *ram_block_format(void);
 
diff --git a/system/physmem.c b/system/physmem.c
index 686f569270..561b2c38c0 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2259,6 +2259,7 @@  void qemu_ram_remap(ram_addr_t addr)
                 }
                 memory_try_enable_merging(vaddr, page_size);
                 qemu_ram_setup_dump(vaddr, page_size);
+                ram_block_notify_remap(block->host, offset, page_size);
             }
 
             break;