>>> Tamas K Lengyel <tamas.leng...@zentific.com> 02/10/15 5:38 PM >>>
>On Tue, Feb 10, 2015 at 5:17 PM, Jan Beulich <jbeul...@suse.com> wrote:
>>>>> Tamas K Lengyel <tamas.leng...@zentific.com> 02/10/15 2:51 PM >>>
>> On Tue, Feb 10, 2015 at 1:52 PM, Jan Beulich <jbeul...@suse.com> wrote:
>>>>>> On 09.02.15 at 19:53, <tamas.leng...@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.
>
>I willwrap it into #ifndef NDEBUG as it is really only for debugging.
That'll make the code even uglier, and won't address the question I originally
asked.
>>>>> @@ -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.
>
>Well, we need to have it as uint64_t in the shared public struct
>definition so that the width of it is explicit. Unsigned long may be
>32-bit depending on the compiler so it is going to be casted
>somewhere.
That's understood.
> Whether it is preferred to be casted when assigned to a
>local variable or when passed to the function is just a style question
>- I'm fine with either.
As much as "gfn" function parameters are usually unsigned long, local
variables are too iirc. So please follow suit.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
