From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <jbeulich@suse.com>
Subject: Re: [PATCH V4 01/13] xen/mem_event: Cleanup of
 mem_event structures
Date: Tue, 10 Feb 2015 16:17:14 +0000
Message-ID: <54DA2F0A02000078000C8143@mail.emea.novell.com>
References: <1423508018-22188-1-git-send-email-tamas.lengyel@zentific.com>
	<1423508018-22188-2-git-send-email-tamas.lengyel@zentific.com>
	<54DA0D0D020000780005E97F@mail.emea.novell.com>
	<CAErYnsj02yK-z3jCgM6N91uJprbh1dxKWK3d1xRzSdgcFF_eKA@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <CAErYnsj02yK-z3jCgM6N91uJprbh1dxKWK3d1xRzSdgcFF_eKA@mail.gmail.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: tamas.lengyel@zentific.com
Cc: tim@xen.org, kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, rcojocaru@bitdefender.com, stefano.stabellini@eu.citrix.com, eddie.dong@intel.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, steve@zentific.com, andres@lagarcavilla.org, jun.nakajima@intel.com, rshriram@cs.ubc.ca, keir@xen.org, dgdegra@tycho.nsa.gov, yanghy@cn.fujitsu.com
List-Id: xen-devel@lists.xenproject.org

>>> Tamas K Lengyel <tamas.lengyel@zentific.com> 02/10/15 2:51 PM >>>
On Tue, Feb 10, 2015 at 1:52 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 09.02.15 at 19:53, <tamas.lengyel@zentific.com> wrote:
>>> @@ -598,6 +600,12 @@ int mem_sharing_sharing_resume(struct domain *d)
>>>      {
>>>          struct vcpu *v;
>>>
>>> +        if ( rsp.version != MEM_EVENT_INTERFACE_VERSION )
>>> +        {
>>> +            gdprintk(XENLOG_WARNING, "mem_event interface version mismatch!\n");
>>
>> Why gdprintk()?
>
>Is that only for debug cases?

I'm intending to propose compiling out alll dprintk() and gdprintk() instance in
non-debug builds. Right now they're preferable when the message is so terse
that identifying its origin without file name and line number is difficult. Clearly
any non-debug messages shouldn't be of such poor quality.

>>> @@ -1310,18 +1322,19 @@ void p2m_mem_paging_resume(struct domain *d)
>>>          /* Fix p2m entry if the page was not dropped */
>>>          if ( !(rsp.flags & MEM_EVENT_FLAG_DROP_PAGE) )
>>>          {
>>> -            gfn_lock(p2m, rsp.gfn, 0);
>>> -            mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, 0, NULL);
>>> +            uint64_t gfn = rsp.u.mem_access.gfn;
>>> +            gfn_lock(p2m, gfn, 0);
>>
>> Blank line between declarations and statements. Also - why uint64_t
>> instead of just unsigned long?
>
>The type of mem_access.gfn is uint64_t so its that for consistency.

And the type most functions taking a gfn expect is unsigned long.

Jan