> -----Original Message-----
> From: Qemu-devel <qemu-devel-
> bounces+chen.zhang=intel....@nongnu.org> On Behalf Of Juan Quintela
> Sent: Tuesday, May 31, 2022 6:43 PM
> To: qemu-devel@nongnu.org
> Cc: Marcel Apfelbaum <marcel.apfelb...@gmail.com>; Philippe Mathieu-
> Daudé <f4...@amsat.org>; Yanan Wang <wangyana...@huawei.com>; Dr.
> David Alan Gilbert <dgilb...@redhat.com>; Juan Quintela
> <quint...@redhat.com>; Eduardo Habkost <edua...@habkost.net>; Peter
> Xu <pet...@redhat.com>; Leonardo Bras <leob...@redhat.com>
> Subject: [PATCH v7 01/13] multifd: Document the locking of
> MultiFD{Send/Recv}Params
>
> Reorder the structures so we can know if the fields are:
> - Read only
> - Their own locking (i.e. sems)
> - Protected by 'mutex'
> - Only for the multifd channel
>
> Signed-off-by: Juan Quintela <quint...@redhat.com>
> ---
> migration/multifd.h | 90 ++++++++++++++++++++++++++-------------------
> 1 file changed, 53 insertions(+), 37 deletions(-)
>
> diff --git a/migration/multifd.h b/migration/multifd.h index
> 4d8d89e5e5..345cfdb50c 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -65,7 +65,9 @@ typedef struct {
> } MultiFDPages_t;
>
> typedef struct {
> - /* this fields are not changed once the thread is created */
> + /* Fiields are only written at creating/deletion time */
S/Fiields/Fields
Thanks
Chen
> + /* No lock required for them, they are read only */
> +
> /* channel number */
> uint8_t id;
> /* channel thread name */
> @@ -74,39 +76,47 @@ typedef struct {
> QemuThread thread;
> /* communication channel */
> QIOChannel *c;
> - /* sem where to wait for more work */
> - QemuSemaphore sem;
> - /* this mutex protects the following parameters */
> - QemuMutex mutex;
> - /* is this channel thread running */
> - bool running;
> - /* should this thread finish */
> - bool quit;
> /* is the yank function registered */
> bool registered_yank;
> + /* packet allocated len */
> + uint32_t packet_len;
> + /* multifd flags for sending ram */
> + int write_flags;
> +
> + /* sem where to wait for more work */
> + QemuSemaphore sem;
> + /* syncs main thread and channels */
> + QemuSemaphore sem_sync;
> +
> + /* this mutex protects the following parameters */
> + QemuMutex mutex;
> + /* is this channel thread running */
> + bool running;
> + /* should this thread finish */
> + bool quit;
> + /* multifd flags for each packet */
> + uint32_t flags;
> + /* global number of generated multifd packets */
> + uint64_t packet_num;
> /* thread has work to do */
> int pending_job;
> - /* array of pages to sent */
> + /* array of pages to sent.
> + * The owner of 'pages' depends of 'pending_job' value:
> + * pending_job == 0 -> migration_thread can use it.
> + * pending_job != 0 -> multifd_channel can use it.
> + */
> MultiFDPages_t *pages;
> - /* packet allocated len */
> - uint32_t packet_len;
> +
> + /* thread local variables. No locking required */
> +
> /* pointer to the packet */
> MultiFDPacket_t *packet;
> - /* multifd flags for sending ram */
> - int write_flags;
> - /* multifd flags for each packet */
> - uint32_t flags;
> /* size of the next packet that contains pages */
> uint32_t next_packet_size;
> - /* global number of generated multifd packets */
> - uint64_t packet_num;
> - /* thread local variables */
> /* packets sent through this channel */
> uint64_t num_packets;
> /* non zero pages sent through this channel */
> uint64_t total_normal_pages;
> - /* syncs main thread and channels */
> - QemuSemaphore sem_sync;
> /* buffers to send */
> struct iovec *iov;
> /* number of iovs used */
> @@ -120,7 +130,9 @@ typedef struct {
> } MultiFDSendParams;
>
> typedef struct {
> - /* this fields are not changed once the thread is created */
> + /* Fiields are only written at creating/deletion time */
> + /* No lock required for them, they are read only */
> +
> /* channel number */
> uint8_t id;
> /* channel thread name */
> @@ -129,31 +141,35 @@ typedef struct {
> QemuThread thread;
> /* communication channel */
> QIOChannel *c;
> + /* packet allocated len */
> + uint32_t packet_len;
> +
> + /* syncs main thread and channels */
> + QemuSemaphore sem_sync;
> +
> /* this mutex protects the following parameters */
> QemuMutex mutex;
> /* is this channel thread running */
> bool running;
> /* should this thread finish */
> bool quit;
> + /* multifd flags for each packet */
> + uint32_t flags;
> + /* global number of generated multifd packets */
> + uint64_t packet_num;
> +
> + /* thread local variables. No locking required */
> +
> + /* pointer to the packet */
> + MultiFDPacket_t *packet;
> + /* size of the next packet that contains pages */
> + uint32_t next_packet_size;
> + /* packets sent through this channel */
> + uint64_t num_packets;
> /* ramblock host address */
> uint8_t *host;
> - /* packet allocated len */
> - uint32_t packet_len;
> - /* pointer to the packet */
> - MultiFDPacket_t *packet;
> - /* multifd flags for each packet */
> - uint32_t flags;
> - /* global number of generated multifd packets */
> - uint64_t packet_num;
> - /* thread local variables */
> - /* size of the next packet that contains pages */
> - uint32_t next_packet_size;
> - /* packets sent through this channel */
> - uint64_t num_packets;
> /* non zero pages recv through this channel */
> uint64_t total_normal_pages;
> - /* syncs main thread and channels */
> - QemuSemaphore sem_sync;
> /* buffers to recv */
> struct iovec *iov;
> /* Pages that are not zero */
> --
> 2.35.3
>
