On 5/4/26 14:30, Marc-André Lureau wrote: > Replace the source-level replay wrappers with a new > replay_by_populated_state() helper that iterates the section at > min-granularity, calls is_populated() for each chunk, and aggregates > consecutive chunks of the same state before invoking the callback. > > This moves the iteration logic from individual sources into the manager, > preparing for multi-source aggregation where the manager must combine > state from multiple sources anyway. > > The replay_populated/replay_discarded vtable entries in > RamDiscardSourceClass are no longer called but remain in the interface > for now; they will be removed in follow-up commits along with the > now-dead source implementations. > > Signed-off-by: Marc-André Lureau <[email protected]> > --- > system/ram-discard-manager.c | 85 > +++++++++++++++++++++++++++++++------------- > 1 file changed, 61 insertions(+), 24 deletions(-) > > diff --git a/system/ram-discard-manager.c b/system/ram-discard-manager.c > index 1c9ff7fda58..a907ddf3708 100644 > --- a/system/ram-discard-manager.c > +++ b/system/ram-discard-manager.c > @@ -27,26 +27,65 @@ static bool ram_discard_source_is_populated(const > RamDiscardSource *rds, > return rdsc->is_populated(rds, section); > } > > -static int ram_discard_source_replay_populated(const RamDiscardSource *rds, > - const MemoryRegionSection > *section, > - ReplayRamDiscardState > replay_fn, > - void *opaque) > +/* > + * Iterate the section at source granularity, aggregating consecutive chunks > + * with matching populated state, and call replay_fn for each run. > + */ > +static int replay_by_populated_state(const RamDiscardManager *rdm, > + const MemoryRegionSection *section, > + bool replay_populated, > + ReplayRamDiscardState replay_fn, > + void *opaque) > { > - RamDiscardSourceClass *rdsc = RAM_DISCARD_SOURCE_GET_CLASS(rds); > + uint64_t granularity, offset, size, end, pos, run_start = 0; > + bool in_run = false; > + int ret = 0; > > - g_assert(rdsc->replay_populated); > - return rdsc->replay_populated(rds, section, replay_fn, opaque); > -} > + granularity = ram_discard_source_get_min_granularity(rdm->rds, rdm->mr); > + offset = section->offset_within_region; > + size = int128_get64(section->size); > + end = offset + size; > + > + /* Align iteration to granularity boundaries */ > + pos = QEMU_ALIGN_DOWN(offset, granularity); > + > + for (; pos < end; pos += granularity) { > + MemoryRegionSection chunk = { > + .mr = section->mr, > + .offset_within_region = pos, > + .size = int128_make64(granularity), > + }; > + bool populated = ram_discard_source_is_populated(rdm->rds, &chunk); > + > + if (populated == replay_populated) { > + if (!in_run) { > + run_start = pos; > + in_run = true; > + } > + } else if (in_run) { > + MemoryRegionSection tmp = *section; > + > + if (memory_region_section_intersect_range(&tmp, run_start, > + pos - run_start)) { > + ret = replay_fn(&tmp, opaque); > + if (ret) { > + return ret; > + } > + } > + in_run = false; > + } > + } > > -static int ram_discard_source_replay_discarded(const RamDiscardSource *rds, > - const MemoryRegionSection > *section, > - ReplayRamDiscardState > replay_fn, > - void *opaque) > -{ > - RamDiscardSourceClass *rdsc = RAM_DISCARD_SOURCE_GET_CLASS(rds); > + if (in_run) { > + MemoryRegionSection tmp = *section; > > - g_assert(rdsc->replay_discarded); > - return rdsc->replay_discarded(rds, section, replay_fn, opaque); > + if (memory_region_section_intersect_range(&tmp, run_start, > + pos - run_start)) { > + ret = replay_fn(&tmp, opaque); > + } > + } > + > + return ret; > } > > RamDiscardManager *ram_discard_manager_new(MemoryRegion *mr, > @@ -78,8 +117,7 @@ int ram_discard_manager_replay_populated(const > RamDiscardManager *rdm, > ReplayRamDiscardState replay_fn, > void *opaque) > { > - return ram_discard_source_replay_populated(rdm->rds, section, > - replay_fn, opaque); > + return replay_by_populated_state(rdm, section, true, replay_fn, opaque); > } > > int ram_discard_manager_replay_discarded(const RamDiscardManager *rdm, > @@ -87,8 +125,7 @@ int ram_discard_manager_replay_discarded(const > RamDiscardManager *rdm, > ReplayRamDiscardState replay_fn, > void *opaque) > { > - return ram_discard_source_replay_discarded(rdm->rds, section, > - replay_fn, opaque); > + return replay_by_populated_state(rdm, section, false, replay_fn, opaque); > } > > static void ram_discard_manager_initfn(Object *obj) > @@ -182,8 +219,8 @@ void > ram_discard_manager_register_listener(RamDiscardManager *rdm, > rdl->section = memory_region_section_new_copy(section); > QLIST_INSERT_HEAD(&rdm->rdl_list, rdl, next); > > - ret = ram_discard_source_replay_populated(rdm->rds, rdl->section, > - rdm_populate_cb, rdl); > + ret = ram_discard_manager_replay_populated(rdm, rdl->section, > + rdm_populate_cb, rdl); > if (ret) { > error_report("%s: Replaying populated ranges failed: %s", __func__, > strerror(-ret)); > @@ -208,8 +245,8 @@ int > ram_discard_manager_replay_populated_to_listeners(RamDiscardManager *rdm) > int ret = 0; > > QLIST_FOREACH(rdl, &rdm->rdl_list, next) { > - ret = ram_discard_source_replay_populated(rdm->rds, rdl->section, > - rdm_populate_cb, rdl); > + ret = ram_discard_manager_replay_populated(rdm, rdl->section, > + rdm_populate_cb, rdl);
I'm probably missing something. You are changing the parameters passed to ram_discard_manager_replay_populated(), but ram_discard_manager_replay_populated() is not actually modified in this patch to take other parameters? -- Cheers, David
