Re: [Xen-devel] [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to not pass a structure by value

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

Wei Liu writes ("Re: [PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() to 
not pass a structure by value"):
> On Mon, Oct 16, 2017 at 02:51:54PM +0100, Andrew Cooper wrote:
...
> > With Joshua's patch in place, the implementer of this callback is the
> > code generated by libxl_save_msgs_gen.pl, which is the aformentioned
> > extra process.  Passing by pointer or value has nothing to do with the
> > fact that the automatically generated code needs to know how to
> > serialise/deserialise the data to feed it back to the main process.
> 
> Right. I agree with you here after going back to the old thread.

ISTM that the callback being a struct rather than a pointer does make
the code in libxl_save_msgs_gen.pl simpler, since it can simply memcpy
the struct.

I certainly dislike your 1/2 patch with the current commit message.

Andrew Cooper writes ("[PATCH for-4.10 1/2] tools/libxc: Fix precopy_policy() 
to not pass a structure by value"):
> c/s 4d69b3495 "Introduce migration precopy policy" uses bogus reasoning to
> justify passing precopy_stats by value.
> 
> Under no circumstances can the precopy callback ever be executing in a
> separate address space.

This statement is true only if you think "the precopy callback" refers
to the stub generated by libxl_save_msgs_gen.  But a more natural
reading is that "the precopy callback" refers to the actual code which
implements whatever logic is required.

In a system using libxl, that code definitely _is_ executing in a
separate address space.  And passing the stats by value rather than
reference does make it marginally easier.

> Switch the callback to passing by pointer which is far more efficient, and
> drop the typedef (because none of the other callback have this oddity).

I would like you to expand on this efficiency argument.

Certainly, with libxl (which is the primary upstream-supported
toolstack) there is no discernable efficiency gain here.  The data
must be copied back and forth between processes.

If you are talking about out-of-tree consumers then you should say
so.  And you should also give a realistic explanation of the size of
the supposed performance benefit.

(FAOD: Nacked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>)

Sorry,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel