On Tue, Jul 23, 2024 at 05:50:24PM -0300, Fabiano Rosas wrote:
> The natural thing would be to put the hooks inside the data
> type. Something like this:
>
> struct MultiFDRecvData {
> MultiFDMethods *ops; <---
> void *opaque;
> size_t size;
> /* for preadv */
> off_t file_offset;
> };
>
> struct MultiFDSendData {
> MultiFDPayloadType type;
> MultiFDMethods *ops; <---
> union {
> MultiFDPages_t ram;
> } u;
> };
>
> multifd_ram_save_setup()
> {
> multifd_ram_send = multifd_send_data_alloc();
> multifd_register_ops(multifd_ram_send, &multifd_ram_ops);
> }
>
> void multifd_register_ops(MultiFDSendData *data, MultiFDMethods *ops)
> {
> if (data->type == RAM) {
> method = migrate_multifd_compression();
> assert(0 < method && method < MULTIFD_COMPRESSION__MAX);
> ops = multifd_ops[method];
> }
>
> data->ops = ops;
> }
>
> I'm just not sure whether we have some compiler cleverness optimizing
> these function pointer accesses due to multifd_send_state being static
> and multifd_send_state->ops being unchanged throughout the
> migration. But AFAICS, the proposal above is almost the same thing as we
> already have.
Right, this looks as pretty when you can put the ops inside, and iff the
perf goes as well. E.g., you'll need to register the Ops for each batch
too, besides the pointer jumps. You'll also need to check the hooks one by
one, even if we know most of them will be noop at least for VFIO.
IMHO it's a matter of whether the Ops is useful to VFIO / other devices
first in the forseeable future. My current gut feeling is they're mostly
not usable.. while supporting device state with an opaque buffer to send /
recv, plus a pretty static device state protocol seems relatively easy to
do.
--
Peter Xu
