Re: [PATCH v3 11/24] migration/multifd: Make multifd_send() thread safe

[Maciej S. Szmigiero]

On 5.12.2024 17:17, Peter Xu wrote:
> On Sun, Nov 17, 2024 at 08:20:06PM +0100, Maciej S. Szmigiero wrote:
> > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
> >
> > multifd_send() function is currently not thread safe, make it thread safe
> > by holding a lock during its execution.
> >
> > This way it will be possible to safely call it concurrently from multiple
> > threads.
> >
> > Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> One nitpick:
>
> > ---
> >   migration/multifd.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/migration/multifd.c b/migration/multifd.c
> > index 9578a985449b..4575495c8816 100644
> > --- a/migration/multifd.c
> > +++ b/migration/multifd.c
> > @@ -50,6 +50,10 @@ typedef struct {
> >   
> >   struct {
> >       MultiFDSendParams *params;
> > +
> > +    /* multifd_send() body is not thread safe, needs serialization */
> > +    QemuMutex multifd_send_mutex;
> > +
> >       /*
> >        * Global number of generated multifd packets.
> >        *
> > @@ -331,6 +335,7 @@ static void multifd_send_kick_main(MultiFDSendParams *p)
> >    */
> >   bool multifd_send(MultiFDSendData **send_data)
> >   {
> > +    QEMU_LOCK_GUARD(&multifd_send_state->multifd_send_mutex);
>
> Better move this after the varaible declarations to be clear..
>
> Perhaps even after multifd_send_should_exit() because reading that doesn't
> need a lock, just in case something wanna quit but keep stuck with the
> mutex.

Will do.

Thanks,
Maciej