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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Oct 16, 2017 at 04:07:32PM +0100, Ian Jackson wrote:
> 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.

Yes, that's the case.

> 
> 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.
> 

This is true.

> 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