Re: [PATCH v2 21/29] migration/multifd: Add pages to the receiving side

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Oct 31, 2023 at 08:18:06PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Mon, Oct 23, 2023 at 05:36:00PM -0300, Fabiano Rosas wrote:
>> >> Currently multifd does not need to have knowledge of pages on the
>> >> receiving side because all the information needed is within the
>> >> packets that come in the stream.
>> >> 
>> >> We're about to add support to fixed-ram migration, which cannot use
>> >> packets because it expects the ramblock section in the migration file
>> >> to contain only the guest pages data.
>> >> 
>> >> Add a pointer to MultiFDPages in the multifd_recv_state and use the
>> >> pages similarly to what we already do on the sending side. The pages
>> >> are used to transfer data between the ram migration code in the main
>> >> migration thread and the multifd receiving threads.
>> >> 
>> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> >
>> > If it'll be new code to maintain anyway, I think we don't necessarily
>> > always use multifd structs, right?
>> >
>> 
>> For the sending side, unrelated to this series, I'm experimenting with
>> defining a generic structure to be passed into multifd:
>> 
>> struct MultiFDData_t {
>>     void *opaque;
>>     size_t size;
>>     bool ready;
>>     void (*cleanup_fn)(void *);
>> };
>> 
>> The client code (ram.c) would use the opaque field to put whatever it
>> wants in it. Maybe we could have a similar concept on the receiving
>> side?
>> 
>> Here's a PoC I'm writing, if you're interested:
>> 
>> https://github.com/farosas/qemu/commits/multifd-packet-cleanups
>> 
>> (I'm delaying sending this to the list because we already have a
>> reasonable backlog of features and refactorings to merge.)
>
> I went through the idea, I agree it's reasonable to generalize multifd to
> drop the page constraints.

Ok, I'll propose it once we get a slowdown on the ml volume

> Actually I'm wondering maybe it should be
> better that we have a thread pool model for migration, then multifd can be
> based on that.
>
> Something like: job submissions, proper locks, notifications, quits,
> etc. with a bunch of API to manipulate the thread pool.

I agree in principle.

> And actually.. I just noticed we have. :) See util/thread-pool.c.  I didn't
> have closer look, but that looks like something good if we can work on top
> (e.g., I don't think we want the bottom halfs..), or refactor to satisfy
> all our needs from migration pov.  Not something I'm asking right away, but
> maybe we can at least keep an eye on.
>

I wonder if adapting multifd to use a QIOTask for the channels would
make sense as an intermediary step. Seems simpler and would force us to
format multifd in more generic terms.