All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jennifer Herbert <Jennifer.Herbert@citrix.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	xen-devel@lists.xenproject.org, jtotto@uwaterloo.ca
Subject: Re: [PATCH v3 2/3] Introduce migration precopy policy
Date: Tue, 26 Sep 2017 12:38:04 +0100	[thread overview]
Message-ID: <59CA3C1C.9070708@citrix.com> (raw)
In-Reply-To: <61663abc-b885-7417-3c81-68b29ad780c9@citrix.com>

On 26/09/17 11:58, Andrew Cooper wrote:
> On 25/09/17 19:55, Jennifer Herbert wrote:
>> +/*
>> + * A precopy_policy callback may not be running in the same address
>> + * space as libxc an so precopy_stats is passed by value.
>> + */
> Please take a step back and thing about what is written here...
>
> As I've said repeatedly, the structure vs pointer argument here is
> entirely unrelated to the IPC boundary.
>
> I am also unhappy that, after multiple review requests saying "turn this
> into a pointer", its remained being passed by value.  It is not ok to
> hack things like this up simply because its the easier cause of action.

Passing tiny structures by value is not a hack - it is perfectly valid 
C.  Bare in mind that a pointer is not a zero
sized entity - it itself needs to be copied.  Passing by value here is 
quite possibly faster, and use less memory.   Furthermore, the multiple 
reviews concluded that this was related to IPC boundary, and that the 
tiny struct size was acceptable.

Maybe you could elaborate on why you do not think it is related to the 
IPC boundary.

Instead, we could pass each element of the structure, as an individual 
parameters.  This would seem a retrograde
step to me, but  would address your sensitivities.

-jenny

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

  reply	other threads:[~2017-09-26 11:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-25 18:55 [PATCH v3 0/3] Introduce migration precopy policy Jennifer Herbert
2017-09-25 18:55 ` [PATCH v3 1/3] Tidy libxc xc_domain_save Jennifer Herbert
2017-09-25 18:55 ` [PATCH v3 2/3] Introduce migration precopy policy Jennifer Herbert
2017-09-26  8:36   ` Wei Liu
2017-09-26 10:58   ` Andrew Cooper
2017-09-26 11:38     ` Jennifer Herbert [this message]
2017-09-26 11:02   ` Andrew Cooper
2017-09-26 11:49     ` Jennifer Herbert
2017-09-25 18:55 ` [PATCH v3 3/3] RFC: migration: defer precopy policy to libxl Jennifer Herbert
2017-09-26  8:48   ` Wei Liu
2017-09-26  9:42     ` Jennifer Herbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=59CA3C1C.9070708@citrix.com \
    --to=jennifer.herbert@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jtotto@uwaterloo.ca \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.