From: Alexey Kardashevskiy <aik@amd.com>
To: "Chenyi Qiang" <chenyi.qiang@intel.com>,
"David Hildenbrand" <david@redhat.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: 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>
Subject: Re: [PATCH v5 03/10] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard()
Date: Tue, 27 May 2025 16:56:36 +1000 [thread overview]
Message-ID: <001c8e69-4a12-416a-8c47-4112cd18ff06@amd.com> (raw)
In-Reply-To: <20250520102856.132417-4-chenyi.qiang@intel.com>
On 20/5/25 20:28, Chenyi Qiang wrote:
> 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.
>
> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
Reviewed-by: Alexey Kardashevskiy <aik@amd.com>
> ---
> 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.
>
> Changes in v3:
> - Newly added.
> ---
> hw/virtio/virtio-mem.c | 21 ++++++++++-----------
> include/system/memory.h | 36 +++++++++++++++++++-----------------
> migration/ram.c | 5 +++--
> system/memory.c | 12 ++++++------
> 4 files changed, 38 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 896948deb1..83b28551c4 100644
> --- a/include/system/memory.h
> +++ b/include/system/memory.h
> @@ -575,8 +575,8 @@ 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);
> +typedef int (*ReplayRamDiscardState)(MemoryRegionSection *section,
> + void *opaque);
>
> /*
> * RamDiscardManagerClass:
> @@ -650,36 +650,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:
> @@ -722,13 +724,13 @@ 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);
>
> -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);
>
> void ram_discard_manager_register_listener(RamDiscardManager *rdm,
> RamDiscardListener *rdl,
> diff --git a/migration/ram.c b/migration/ram.c
> index e12913b43e..c004f37060 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 b45b508dce..de45fbdd3f 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,
--
Alexey
next prev parent reply other threads:[~2025-05-27 6:58 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-20 10:28 [PATCH v5 00/10] Enable shared device assignment Chenyi Qiang
2025-05-20 10:28 ` [PATCH v5 01/10] memory: Export a helper to get intersection of a MemoryRegionSection with a given range Chenyi Qiang
2025-05-20 10:28 ` [PATCH v5 02/10] memory: Change memory_region_set_ram_discard_manager() to return the result Chenyi Qiang
2025-05-26 8:40 ` David Hildenbrand
2025-05-27 6:56 ` Alexey Kardashevskiy
2025-05-20 10:28 ` [PATCH v5 03/10] memory: Unify the definiton of ReplayRamPopulate() and ReplayRamDiscard() Chenyi Qiang
2025-05-26 8:42 ` David Hildenbrand
2025-05-26 9:35 ` Philippe Mathieu-Daudé
2025-05-26 10:21 ` Chenyi Qiang
2025-05-27 6:56 ` Alexey Kardashevskiy [this message]
2025-05-20 10:28 ` [PATCH v5 04/10] ram-block-attribute: Introduce RamBlockAttribute to manage RAMBlock with guest_memfd Chenyi Qiang
2025-05-26 9:01 ` David Hildenbrand
2025-05-26 9:28 ` Chenyi Qiang
2025-05-26 11:16 ` Alexey Kardashevskiy
2025-05-27 1:15 ` Chenyi Qiang
2025-05-27 1:20 ` Alexey Kardashevskiy
2025-05-27 3:14 ` Chenyi Qiang
2025-05-27 6:06 ` Alexey Kardashevskiy
2025-05-20 10:28 ` [PATCH v5 05/10] ram-block-attribute: Introduce a helper to notify shared/private state changes Chenyi Qiang
2025-05-26 9:02 ` David Hildenbrand
2025-05-27 7:35 ` Alexey Kardashevskiy
2025-05-27 9:06 ` Chenyi Qiang
2025-05-27 9:19 ` Alexey Kardashevskiy
2025-05-20 10:28 ` [PATCH v5 06/10] memory: Attach RamBlockAttribute to guest_memfd-backed RAMBlocks Chenyi Qiang
2025-05-26 9:06 ` David Hildenbrand
2025-05-26 9:46 ` Chenyi Qiang
2025-05-20 10:28 ` [PATCH v5 07/10] RAMBlock: Make guest_memfd require coordinate discard Chenyi Qiang
2025-05-26 9:08 ` David Hildenbrand
2025-05-27 5:47 ` Chenyi Qiang
2025-05-27 7:42 ` Alexey Kardashevskiy
2025-05-27 8:12 ` Chenyi Qiang
2025-05-27 11:20 ` David Hildenbrand
2025-05-28 1:57 ` Chenyi Qiang
2025-05-20 10:28 ` [PATCH v5 08/10] memory: Change NotifyRamDiscard() definition to return the result Chenyi Qiang
2025-05-26 9:31 ` Philippe Mathieu-Daudé
2025-05-26 10:36 ` Cédric Le Goater
2025-05-26 12:44 ` Cédric Le Goater
2025-05-27 5:29 ` Chenyi Qiang
2025-05-20 10:28 ` [PATCH v5 09/10] KVM: Introduce RamDiscardListener for attribute changes during memory conversions Chenyi Qiang
2025-05-26 9:22 ` David Hildenbrand
2025-05-27 8:01 ` Alexey Kardashevskiy
2025-05-20 10:28 ` [PATCH v5 10/10] ram-block-attribute: Add more error handling during state changes Chenyi Qiang
2025-05-26 9:17 ` David Hildenbrand
2025-05-26 10:19 ` Chenyi Qiang
2025-05-26 12:10 ` David Hildenbrand
2025-05-26 12:39 ` Chenyi Qiang
2025-05-27 9:11 ` Alexey Kardashevskiy
2025-05-27 10:18 ` Chenyi Qiang
2025-05-27 11:21 ` David Hildenbrand
2025-05-26 11:37 ` [PATCH v5 00/10] Enable shared device assignment Cédric Le Goater
2025-05-26 12:16 ` 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=001c8e69-4a12-416a-8c47-4112cd18ff06@amd.com \
--to=aik@amd.com \
--cc=baolu.lu@linux.intel.com \
--cc=chao.gao@intel.com \
--cc=chenyi.qiang@intel.com \
--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).