[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphor
From: |
Peter Xu |
Subject: |
Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore |
Date: |
Thu, 19 Oct 2023 10:35:02 -0400 |
Fabiano,
Sorry to look at this series late; I messed up my inbox after I reworked my
arrangement methodology of emails. ;)
On Thu, Oct 19, 2023 at 11:06:06AM +0200, Juan Quintela wrote:
> Fabiano Rosas <farosas@suse.de> wrote:
> > The channels_ready semaphore is a global variable not linked to any
> > single multifd channel. Waiting on it only means that "some" channel
> > has become ready to send data. Since we need to address the channels
> > by index (multifd_send_state->params[i]), that information adds
> > nothing of value.
>
> NAK.
>
> I disagree here O:-)
>
> the reason why that channel exist is for multifd_send_pages()
>
> And simplifying the function what it does is:
>
> sem_wait(channels_ready);
>
> for_each_channel()
> look if it is empty()
>
> But with the semaphore, we guarantee that when we go to the loop, there
> is a channel ready, so we know we donat busy wait searching for a
> channel that is free.
>
> Notice that I fully agree that the sem is not needed for locking.
> Locking is done with the mutex. It is just used to make sure that we
> don't busy loop on that loop.
>
> And we use a sem, because it is the easiest way to know how many
> channels are ready (even when we only care if there is one when we
> arrive to that code).
>
> We lost count of that counter, and we fixed that here:
>
> commit d2026ee117147893f8d80f060cede6d872ecbd7f
> Author: Juan Quintela <quintela@redhat.com>
> Date: Wed Apr 26 12:20:36 2023 +0200
>
> multifd: Fix the number of channels ready
>
> We don't wait in the sem when we are doing a sync_main. Make it
>
> And we were addressing the problem that some users where finding that we
> were busy waiting on that loop.
Juan,
I can understand why send_pages needs that sem, but not when sync main.
IOW, why multifd_send_sync_main() needs:
qemu_sem_wait(&multifd_send_state->channels_ready);
If it has:
qemu_sem_wait(&p->sem_sync);
How does a busy loop happen?
Thanks,
--
Peter Xu
- [RFC PATCH v2 0/6] migration/multifd: Locking changes, Fabiano Rosas, 2023/10/12
- [RFC PATCH v2 5/6] migration/multifd: Stop setting 'quit' outside of channels, Fabiano Rosas, 2023/10/12
- [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Fabiano Rosas, 2023/10/12
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Juan Quintela, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore,
Peter Xu <=
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Juan Quintela, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Peter Xu, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Juan Quintela, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Peter Xu, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Juan Quintela, 2023/10/20
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Fabiano Rosas, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Juan Quintela, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Fabiano Rosas, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Juan Quintela, 2023/10/19
- Re: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore, Peter Xu, 2023/10/19