On Thu, Jul 11, 2024 at 06:12:44PM -0300, Fabiano Rosas wrote: > Peter Xu <pet...@redhat.com> writes: > > > On Thu, Jul 11, 2024 at 04:37:34PM -0300, Fabiano Rosas wrote: > > > > [...] > > > >> We also don't flush the iov at once, so f->buf seems redundant to > >> me. But of course, if we touch any of that we must ensure we're not > >> dropping any major optimization. > > > > Yes some tests over that would be more persuasive when it comes. > > > > Per my limited experience in the past few years: memcpy on chips nowadays > > is pretty cheap. You'll see very soon one more example of that when you > > start to look at the qatzip series: that series decided to do one more > > memcpy for all guest pages, to make it a larger chunk of buffer instead of > > submitting the compression tasks in 4k chunks (while I thought 4k wasn't > > too small itself). > > > > That may be more involved so may not be a great example (e.g. the > > compression algo can be special in this case where it just likes larger > > buffers), but it's not uncommon that I see people trade things with memcpy, > > especially small buffers. > > > > [...] > > > >> Any piece of code that fills an iov with data is prone to be able to > >> send that data through multifd. From this perspective, multifd is just a > >> way to give work to an iochannel. We don't *need* to use it, but it > >> might be simple enough to the point that the benefit of ditching > >> QEMUFile can be reached without too much rework. > >> > >> Say we provision multifd threads early and leave them waiting for any > >> part of the migration code to send some data. We could have n-1 threads > >> idle waiting for the bulk of the data and use a single thread for any > >> early traffic that does not need to be parallel. > >> > >> I'm not suggesting we do any of this right away or even that this is the > >> correct way to go, I'm just letting you know some of my ideas and why I > >> think ram + device state might not be the only data we put through > >> multifd. > > > > We can wait and see whether that can be of any use in the future, even if > > so, we still have chance to add more types into the union, I think. But > > again, I don't expect. > > > > My gut feeling: we shouldn't bother putting any (1) non-huge-chunk, or (2) > > non-IO, data onto multifd. Again, I would ask "why not the main channel", > > otherwise. > > > > [...] > > > >> Just to be clear, do you want a thread-pool to replace multifd? Or would > >> that be only used for concurrency on the producer side? > > > > Not replace multifd. It's just that I was imagining multifd threads only > > manage IO stuff, nothing else. > > > > I was indeed thinking whether we can reuse multifd threads, but then I > > found there's risk mangling these two concepts, as: when we do more than IO > > in multifd threads (e.g., talking to VFIO kernel fetching data which can > > block), we have risk of blocking IO even if we can push more so the NICs > > can be idle again. There's also the complexity where the job fetches data > > from VFIO kernel and want to enqueue again, it means an multifd task can > > enqueue to itself, and circular enqueue can be challenging: imagine 8 > > concurrent tasks (with a total of 8 multifd threads) trying to enqueue at > > the same time; they hunger themselves to death. Things like that. Then I > > figured the rest jobs are really fn(void*) type of things; they should > > deserve their own pool of threads. > > > > So the VFIO threads (used to be per-device) becomes migration worker > > threads, we need them for both src/dst: on dst there's still pending work > > to apply the continuous VFIO data back to the kernel driver, and that can't > > be done by multifd thread too due to similar same reason. Then those dest > > side worker threads can also do load() not only for VFIO but also other > > device states if we can add more. > > > > So to summary, we'll have: > > > > - 1 main thread (send / recv) > > - N multifd threads (IOs only) > > - M worker threads (jobs only) > > > > Of course, postcopy not involved.. How's that sound? > > Looks good. There's a better divide between producer and consumer this > way. I think it will help when designing new features. > > One observation is that we'll still have two different entities doing IO > (multifd threads and the migration thread), which I would prefer were > using a common code at a higher level than the iochannel.
At least for the main channel probably yes. I think Dan has had the idea of adding the buffering layer over iochannels, then replace qemufiles with that. Multifd channels looks ok so far to use as raw channels. > > One thing that I tried to look into for mapped-ram was whether we could > set up iouring in the migration code, but got entirely discouraged by > the migration thread doing IO at random points. And of course, you've > seen what we had to do with direct-io. That was in part due to having > the migration thread in parallel doing it's small writes at undetermined > points in time. On the locked_vm part: probably yes, we'd better try to avoid using page pinning if possible. It just looks like it becomes a more important scenario nowadays to put VMs into containers, it means then such feature may not be always usable there. For the rest: I really don't know much on iouring, but I remember it can be fast normally only in a poll model with interrupt-less context? Not sure whether it suites here for us, as I guess we should avoid consuming cpu resourcess with no good reason, and polling for perf falls into that category, I think. Even without it, kubevirt now already has issue on multifd eating cpus, and people observe multifd threads causing vcpu threads to be throttled, interrupting guest workloads; they're currently put in the same container. I also not sure how much it'll help comparing to when we have the multi-threading ready. I suspect not that much. -- Peter Xu
