Peter Xu <pet...@redhat.com> writes: > 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?
I'm not sure. I mainly thought about using it to save syscalls on the write side. But as I said, I didn't look further into it. > 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. Do you have a reference for that kubevirt issue I could look at? It maybe interesting to investigate further. Where's the throttling coming from? And doesn't less vcpu time imply less dirtying and therefore faster convergence?
