On 4/6/26 15:43, Marc-André Lureau wrote:
Refactor the RamDiscardManager interface into two distinct components:
- RamDiscardSource: An interface that state providers (virtio-mem,
RamBlockAttributes) implement to provide discard state information
(granularity, populated/discarded ranges, replay callbacks).
- RamDiscardManager: A concrete QOM object that wraps a source, owns
the listener list, and handles listener registration/unregistration
and notifications.
This separation moves the listener management logic from individual
source implementations into the central RamDiscardManager, reducing
code duplication between virtio-mem and RamBlockAttributes.
The change prepares for future work where a RamDiscardManager could
aggregate multiple sources.
Note, the original virtio-mem code had conditions before discard:
if (vmem->size) {
rdl->notify_discard(rdl, rdl->section);
}
however, the new code calls discard unconditionally. This is considered
safe, since the populate/discard of sections are already asymmetrical
(unplug & unregister all listener section unconditionally).
Reviewed-by: Peter Xu <[email protected]>
Acked-by: David Hildenbrand <[email protected]>
Signed-off-by: Marc-André Lureau <[email protected]>
---
include/hw/virtio/virtio-mem.h | 3 -
include/system/memory.h | 197 +++++++++++++++++++++----------------
include/system/ramblock.h | 2 -
hw/virtio/virtio-mem.c | 163 ++++++------------------------
system/memory.c | 218 ++++++++++++++++++++++++++++++++++++-----
system/ram-block-attributes.c | 171 ++++++++++----------------------
6 files changed, 385 insertions(+), 369 deletions(-)
+int ram_discard_manager_notify_populate(RamDiscardManager *rdm,
+ uint64_t offset, uint64_t size)
+{
+ RamDiscardListener *rdl, *rdl2;
+ int ret = 0;
+
+ QLIST_FOREACH(rdl, &rdm->rdl_list, next) {
+ MemoryRegionSection tmp = *rdl->section;
+
+ if (!memory_region_section_intersect_range(&tmp, offset, size)) {
+ continue;
+ }
+ ret = rdl->notify_populate(rdl, &tmp);
+ if (ret) {
+ break;
+ }
+ }
+
+ if (ret) {
(Pre-existing, reduce scope?)
RamDiscardListener *rdl2;
+ /* Notify all already-notified listeners about discard. */
+ QLIST_FOREACH(rdl2, &rdm->rdl_list, next) {
+ MemoryRegionSection tmp = *rdl2->section;
+
+ if (rdl2 == rdl) {
+ break;
+ }
+ if (!memory_region_section_intersect_range(&tmp, offset, size)) {
+ continue;
+ }
+ rdl2->notify_discard(rdl2, &tmp);
+ }
+ }
+ return ret;
+}
void ram_discard_manager_register_listener(RamDiscardManager *rdm,
RamDiscardListener *rdl,
MemoryRegionSection *section)
{
- RamDiscardManagerClass *rdmc = RAM_DISCARD_MANAGER_GET_CLASS(rdm);
+ int ret;
+
+ g_assert(section->mr == rdm->mr);
+
+ rdl->section = memory_region_section_new_copy(section);
+ QLIST_INSERT_HEAD(&rdm->rdl_list, rdl, next);
- g_assert(rdmc->register_listener);
- rdmc->register_listener(rdm, rdl, section);
+ ret = ram_discard_source_replay_populated(rdm->rds, rdl->section,
+ rdm_populate_cb, rdl);
+ if (ret) {
+ error_report("%s: Replaying populated ranges failed: %s", __func__,
+ strerror(-ret));
Pre-existing, should ram_discard_manager_register_listener propagate
Error*? For example the current caller provides an errp
(vfio_ram_discard_register_listener). Can be improved on top of course.
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
+ }
}
