Re: [PATCH] Low mem virq incremental adjustments

[Keir Fraser <keir@xen.org>]

On 09/03/2012 09:04, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 08.03.12 at 22:59, Dan Magenheimer <dan.magenheimer@oracle.com> wrote:
>>>  From: Andres Lagar-Cavilla [mailto:andres@lagarcavilla.org]
>>> Sent: Wednesday, March 07, 2012 11:12 AM
>>> To: Jan Beulich
>>> Cc: Dan Magenheimer; ian.campbell@citrix.com; ian.jackson@citrix.com;
>> adin@gridcentric.ca;
>>> andres@gridcentric.ca; xen-devel; tim@xen.org
>>> Subject: Re: [PATCH] Low mem virq incremental adjustments
>>> 
>>>>>>> On 07.03.12 at 17:15, Andres Lagar-Cavilla <andres@lagarcavilla.org>
>>>> wrote:
>>>>> --- a/xen/common/page_alloc.c
>>>>> +++ b/xen/common/page_alloc.c
>>>>> @@ -377,7 +377,10 @@ static void __init setup_low_mem_virq(vo
>>>>> 
>>>>>  static void check_low_mem_virq(void)
>>>>>  {
>>>>> -    if ( unlikely(total_avail_pages <= low_mem_virq_th) )
>>>>> +    unsigned long avail_pages = total_avail_pages +
>>>>> +                (opt_tmem) ? tmem_freeable_pages(): 0;
>>>> 
>>>> Can tmem_freeable_pages() return anything other than zero when
>>>> opt_tmem is zero? (I.e. is the [improperly parenthesized!] conditional
>>>> expression necessary at all?)
>>> 
>>> I'm not sure. I'll let Dan take it from here, as he surely knows the right
>>> way. He acked it the way it is.
>>> Andres
>> 
>> Both would be correct (other than the parentheses).
>> 
>> I was also going to make the same comment about tmem_freeable_pages()
>> but decided the way Andres coded it is clearer because it doesn't
>> assume anything about tmem; if tmem is enabled, it uses an
>> abstract interface this code doesn't need to know anything about.
>> 
>> Anyway, either way is fine with me.
> 
> So I take it that you'll be submitting a fix at least for the parentheses
> issue.

Ugh, I checked it in removing the unnecessary parentheses, but didn't add
the required ones around the ternary operator. I'll fix that now.

 -- Keir

> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel