From: Chenyi Qiang <chenyi.qiang@intel.com>
To: "David Hildenbrand" <david@redhat.com>,
"Alexey Kardashevskiy" <aik@amd.com>,
"Peter Xu" <peterx@redhat.com>,
"Gupta Pankaj" <pankaj.gupta@amd.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Michael Roth" <michael.roth@amd.com>
Cc: "Chenyi Qiang" <chenyi.qiang@intel.com>,
qemu-devel@nongnu.org, kvm@vger.kernel.org,
"Williams Dan J" <dan.j.williams@intel.com>,
"Zhao Liu" <zhao1.liu@intel.com>,
"Baolu Lu" <baolu.lu@linux.intel.com>,
"Gao Chao" <chao.gao@intel.com>, "Xu Yilun" <yilun.xu@intel.com>,
"Li Xiaoyao" <xiaoyao.li@intel.com>,
"Cédric Le Goater" <clg@kaod.org>,
"Alex Williamson" <alex.williamson@redhat.com>
Subject: [PATCH v7 3/5] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard()
Date: Thu, 12 Jun 2025 16:27:44 +0800 [thread overview]
Message-ID: <20250612082747.51539-4-chenyi.qiang@intel.com> (raw)
In-Reply-To: <20250612082747.51539-1-chenyi.qiang@intel.com>
Update ReplayRamDiscard() function to return the result and unify the
ReplayRamPopulate() and ReplayRamDiscard() to ReplayRamDiscardState() at
the same time due to their identical definitions. This unification
simplifies related structures, such as VirtIOMEMReplayData, which makes
it cleaner.
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@amd.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
---
Changes in v7:
- Add Reviewed-by from Xiaoyao and Pankaj.
Changes in v6:
- Add Reviewed-by from David
- Add a documentation comment for the prototype change
Changes in v5:
- Rename ReplayRamStateChange to ReplayRamDiscardState (David)
- return data->fn(s, data->opaque) instead of 0 in
virtio_mem_rdm_replay_discarded_cb(). (Alexey)
Changes in v4:
- Modify the commit message. We won't use Replay() operation when
doing the attribute change like v3.
---
hw/virtio/virtio-mem.c | 21 +++++++-------
include/system/memory.h | 64 ++++++++++++++++++++++++++++++-----------
migration/ram.c | 5 ++--
system/memory.c | 12 ++++----
4 files changed, 66 insertions(+), 36 deletions(-)
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 2e491e8c44..c46f6f9c3e 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -1732,7 +1732,7 @@ static bool virtio_mem_rdm_is_populated(const RamDiscardManager *rdm,
}
struct VirtIOMEMReplayData {
- void *fn;
+ ReplayRamDiscardState fn;
void *opaque;
};
@@ -1740,12 +1740,12 @@ static int virtio_mem_rdm_replay_populated_cb(MemoryRegionSection *s, void *arg)
{
struct VirtIOMEMReplayData *data = arg;
- return ((ReplayRamPopulate)data->fn)(s, data->opaque);
+ return data->fn(s, data->opaque);
}
static int virtio_mem_rdm_replay_populated(const RamDiscardManager *rdm,
MemoryRegionSection *s,
- ReplayRamPopulate replay_fn,
+ ReplayRamDiscardState replay_fn,
void *opaque)
{
const VirtIOMEM *vmem = VIRTIO_MEM(rdm);
@@ -1764,14 +1764,13 @@ static int virtio_mem_rdm_replay_discarded_cb(MemoryRegionSection *s,
{
struct VirtIOMEMReplayData *data = arg;
- ((ReplayRamDiscard)data->fn)(s, data->opaque);
- return 0;
+ return data->fn(s, data->opaque);
}
-static void virtio_mem_rdm_replay_discarded(const RamDiscardManager *rdm,
- MemoryRegionSection *s,
- ReplayRamDiscard replay_fn,
- void *opaque)
+static int virtio_mem_rdm_replay_discarded(const RamDiscardManager *rdm,
+ MemoryRegionSection *s,
+ ReplayRamDiscardState replay_fn,
+ void *opaque)
{
const VirtIOMEM *vmem = VIRTIO_MEM(rdm);
struct VirtIOMEMReplayData data = {
@@ -1780,8 +1779,8 @@ static void virtio_mem_rdm_replay_discarded(const RamDiscardManager *rdm,
};
g_assert(s->mr == &vmem->memdev->mr);
- virtio_mem_for_each_unplugged_section(vmem, s, &data,
- virtio_mem_rdm_replay_discarded_cb);
+ return virtio_mem_for_each_unplugged_section(vmem, s, &data,
+ virtio_mem_rdm_replay_discarded_cb);
}
static void virtio_mem_rdm_register_listener(RamDiscardManager *rdm,
diff --git a/include/system/memory.h b/include/system/memory.h
index 60983d4977..eb2618e1b4 100644
--- a/include/system/memory.h
+++ b/include/system/memory.h
@@ -576,8 +576,20 @@ static inline void ram_discard_listener_init(RamDiscardListener *rdl,
rdl->double_discard_supported = double_discard_supported;
}
-typedef int (*ReplayRamPopulate)(MemoryRegionSection *section, void *opaque);
-typedef void (*ReplayRamDiscard)(MemoryRegionSection *section, void *opaque);
+/**
+ * ReplayRamDiscardState:
+ *
+ * The callback handler for #RamDiscardManagerClass.replay_populated/
+ * #RamDiscardManagerClass.replay_discarded to invoke on populated/discarded
+ * parts.
+ *
+ * @section: the #MemoryRegionSection of populated/discarded part
+ * @opaque: pointer to forward to the callback
+ *
+ * Returns 0 on success, or a negative error if failed.
+ */
+typedef int (*ReplayRamDiscardState)(MemoryRegionSection *section,
+ void *opaque);
/*
* RamDiscardManagerClass:
@@ -651,36 +663,38 @@ struct RamDiscardManagerClass {
/**
* @replay_populated:
*
- * Call the #ReplayRamPopulate callback for all populated parts within the
- * #MemoryRegionSection via the #RamDiscardManager.
+ * Call the #ReplayRamDiscardState callback for all populated parts within
+ * the #MemoryRegionSection via the #RamDiscardManager.
*
* In case any call fails, no further calls are made.
*
* @rdm: the #RamDiscardManager
* @section: the #MemoryRegionSection
- * @replay_fn: the #ReplayRamPopulate callback
+ * @replay_fn: the #ReplayRamDiscardState callback
* @opaque: pointer to forward to the callback
*
* Returns 0 on success, or a negative error if any notification failed.
*/
int (*replay_populated)(const RamDiscardManager *rdm,
MemoryRegionSection *section,
- ReplayRamPopulate replay_fn, void *opaque);
+ ReplayRamDiscardState replay_fn, void *opaque);
/**
* @replay_discarded:
*
- * Call the #ReplayRamDiscard callback for all discarded parts within the
- * #MemoryRegionSection via the #RamDiscardManager.
+ * Call the #ReplayRamDiscardState callback for all discarded parts within
+ * the #MemoryRegionSection via the #RamDiscardManager.
*
* @rdm: the #RamDiscardManager
* @section: the #MemoryRegionSection
- * @replay_fn: the #ReplayRamDiscard callback
+ * @replay_fn: the #ReplayRamDiscardState callback
* @opaque: pointer to forward to the callback
+ *
+ * Returns 0 on success, or a negative error if any notification failed.
*/
- void (*replay_discarded)(const RamDiscardManager *rdm,
- MemoryRegionSection *section,
- ReplayRamDiscard replay_fn, void *opaque);
+ int (*replay_discarded)(const RamDiscardManager *rdm,
+ MemoryRegionSection *section,
+ ReplayRamDiscardState replay_fn, void *opaque);
/**
* @register_listener:
@@ -721,15 +735,31 @@ uint64_t ram_discard_manager_get_min_granularity(const RamDiscardManager *rdm,
bool ram_discard_manager_is_populated(const RamDiscardManager *rdm,
const MemoryRegionSection *section);
+/**
+ * ram_discard_manager_replay_populated:
+ *
+ * A wrapper to call the #RamDiscardManagerClass.replay_populated callback
+ * of the #RamDiscardManager.
+ *
+ * Returns 0 on success, or a negative error if any notification failed.
+ */
int ram_discard_manager_replay_populated(const RamDiscardManager *rdm,
MemoryRegionSection *section,
- ReplayRamPopulate replay_fn,
+ ReplayRamDiscardState replay_fn,
void *opaque);
-void ram_discard_manager_replay_discarded(const RamDiscardManager *rdm,
- MemoryRegionSection *section,
- ReplayRamDiscard replay_fn,
- void *opaque);
+/**
+ * ram_discard_manager_replay_discarded:
+ *
+ * A wrapper to call the #RamDiscardManagerClass.replay_discarded callback
+ * of the #RamDiscardManager.
+ *
+ * Returns 0 on success, or a negative error if any notification failed.
+ */
+int ram_discard_manager_replay_discarded(const RamDiscardManager *rdm,
+ MemoryRegionSection *section,
+ ReplayRamDiscardState replay_fn,
+ void *opaque);
void ram_discard_manager_register_listener(RamDiscardManager *rdm,
RamDiscardListener *rdl,
diff --git a/migration/ram.c b/migration/ram.c
index d26dbd37c4..2d4af497b5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -848,8 +848,8 @@ static inline bool migration_bitmap_clear_dirty(RAMState *rs,
return ret;
}
-static void dirty_bitmap_clear_section(MemoryRegionSection *section,
- void *opaque)
+static int dirty_bitmap_clear_section(MemoryRegionSection *section,
+ void *opaque)
{
const hwaddr offset = section->offset_within_region;
const hwaddr size = int128_get64(section->size);
@@ -868,6 +868,7 @@ static void dirty_bitmap_clear_section(MemoryRegionSection *section,
}
*cleared_bits += bitmap_count_one_with_offset(rb->bmap, start, npages);
bitmap_clear(rb->bmap, start, npages);
+ return 0;
}
/*
diff --git a/system/memory.c b/system/memory.c
index d0c186e9f6..76b44b8220 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2138,7 +2138,7 @@ bool ram_discard_manager_is_populated(const RamDiscardManager *rdm,
int ram_discard_manager_replay_populated(const RamDiscardManager *rdm,
MemoryRegionSection *section,
- ReplayRamPopulate replay_fn,
+ ReplayRamDiscardState replay_fn,
void *opaque)
{
RamDiscardManagerClass *rdmc = RAM_DISCARD_MANAGER_GET_CLASS(rdm);
@@ -2147,15 +2147,15 @@ int ram_discard_manager_replay_populated(const RamDiscardManager *rdm,
return rdmc->replay_populated(rdm, section, replay_fn, opaque);
}
-void ram_discard_manager_replay_discarded(const RamDiscardManager *rdm,
- MemoryRegionSection *section,
- ReplayRamDiscard replay_fn,
- void *opaque)
+int ram_discard_manager_replay_discarded(const RamDiscardManager *rdm,
+ MemoryRegionSection *section,
+ ReplayRamDiscardState replay_fn,
+ void *opaque)
{
RamDiscardManagerClass *rdmc = RAM_DISCARD_MANAGER_GET_CLASS(rdm);
g_assert(rdmc->replay_discarded);
- rdmc->replay_discarded(rdm, section, replay_fn, opaque);
+ return rdmc->replay_discarded(rdm, section, replay_fn, opaque);
}
void ram_discard_manager_register_listener(RamDiscardManager *rdm,
--
2.43.5
next prev parent reply other threads:[~2025-06-12 8:29 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-12 8:27 [PATCH v7 0/5] Enable shared device assignment Chenyi Qiang
2025-06-12 8:27 ` [PATCH v7 1/5] memory: Export a helper to get intersection of a MemoryRegionSection with a given range Chenyi Qiang
2025-06-12 8:27 ` [PATCH v7 2/5] memory: Change memory_region_set_ram_discard_manager() to return the result Chenyi Qiang
2025-06-12 8:27 ` Chenyi Qiang [this message]
2025-06-19 3:06 ` [PATCH v7 3/5] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard() Chenyi Qiang
2025-06-19 14:29 ` Peter Xu
2025-06-20 4:04 ` Chenyi Qiang
2025-06-12 8:27 ` [PATCH v7 4/5] ram-block-attributes: Introduce RamBlockAttributes to manage RAMBlock with guest_memfd Chenyi Qiang
2025-06-20 2:53 ` Chenyi Qiang
2025-06-12 8:27 ` [PATCH v7 5/5] physmem: Support coordinated discarding of RAM " Chenyi Qiang
2025-06-18 21:58 ` [PATCH v7 0/5] Enable shared device assignment Peter Xu
2025-06-24 9:51 ` David Hildenbrand
2025-06-18 22:22 ` Peter Xu
2025-06-19 3:09 ` Chenyi Qiang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250612082747.51539-4-chenyi.qiang@intel.com \
--to=chenyi.qiang@intel.com \
--cc=aik@amd.com \
--cc=alex.williamson@redhat.com \
--cc=baolu.lu@linux.intel.com \
--cc=chao.gao@intel.com \
--cc=clg@kaod.org \
--cc=dan.j.williams@intel.com \
--cc=david@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=michael.roth@amd.com \
--cc=pankaj.gupta@amd.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=xiaoyao.li@intel.com \
--cc=yilun.xu@intel.com \
--cc=zhao1.liu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).