From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] Low mem virq incremental adjustments Date: Fri, 09 Mar 2012 09:56:08 +0000 Message-ID: References: <4F59D5B302000078000774F6@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4F59D5B302000078000774F6@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , andres@lagarcavilla.org Cc: Dan Magenheimer , ian.campbell@citrix.com, andres@gridcentric.ca, tim@xen.org, xen-devel , ian.jackson@citrix.com, adin@gridcentric.ca List-Id: xen-devel@lists.xenproject.org On 09/03/2012 09:04, "Jan Beulich" wrote: >>>> On 08.03.12 at 22:59, Dan Magenheimer 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 >>>> 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