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 <faro...@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 <quint...@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