xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Low mem virq incremental adjustments
@ 2012-03-07 16:15 Andres Lagar-Cavilla
  2012-03-07 17:02 ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Andres Lagar-Cavilla @ 2012-03-07 16:15 UTC (permalink / raw)
  To: xen-devel
  Cc: dan.magenheimer, ian.campbell, andres, tim, JBeulich, ian.jackson,
	adin

 .gitignore              |  1 +
 xen/common/page_alloc.c |  7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)


Consider tmem before firing the virq.

Add .gitignore rune.

Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>

diff -r f7ce6d26635c -r 5651945c7a74 .gitignore
--- a/.gitignore
+++ b/.gitignore
@@ -197,6 +197,7 @@ tools/misc/xen-hvmctx
 tools/misc/gtraceview
 tools/misc/gtracestat
 tools/misc/xenlockprof
+tools/misc/lowmemd
 tools/pygrub/build/*
 tools/python/build/*
 tools/python/xen/util/path.py
diff -r f7ce6d26635c -r 5651945c7a74 xen/common/page_alloc.c
--- 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;
+
+    if ( unlikely(avail_pages <= low_mem_virq_th) )
     {
         send_global_virq(VIRQ_ENOMEM);
 
@@ -391,7 +394,7 @@ static void check_low_mem_virq(void)
         return;
     }
 
-    if ( unlikely(total_avail_pages >= low_mem_virq_high) )
+    if ( unlikely(avail_pages >= low_mem_virq_high) )
     {
         /* Reset hysteresis. Bring threshold up one order.
          * If we are back where originally set, set high

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Low mem virq incremental adjustments
  2012-03-07 16:15 [PATCH] Low mem virq incremental adjustments Andres Lagar-Cavilla
@ 2012-03-07 17:02 ` Jan Beulich
  2012-03-07 18:12   ` Andres Lagar-Cavilla
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-03-07 17:02 UTC (permalink / raw)
  To: Andres Lagar-Cavilla, dan.magenheimer
  Cc: ian.campbell, andres, tim, xen-devel, ian.jackson, adin

 >>> 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?)

Jan

> +
> +    if ( unlikely(avail_pages <= low_mem_virq_th) )
>      {
>          send_global_virq(VIRQ_ENOMEM);
>  
> @@ -391,7 +394,7 @@ static void check_low_mem_virq(void)
>          return;
>      }
>  
> -    if ( unlikely(total_avail_pages >= low_mem_virq_high) )
> +    if ( unlikely(avail_pages >= low_mem_virq_high) )
>      {
>          /* Reset hysteresis. Bring threshold up one order.
>           * If we are back where originally set, set high

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Low mem virq incremental adjustments
  2012-03-07 17:02 ` Jan Beulich
@ 2012-03-07 18:12   ` Andres Lagar-Cavilla
  2012-03-08 21:59     ` Dan Magenheimer
  0 siblings, 1 reply; 6+ messages in thread
From: Andres Lagar-Cavilla @ 2012-03-07 18:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: dan.magenheimer, ian.campbell, andres, tim, xen-devel,
	ian.jackson, adin

>  >>> 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

>
> Jan
>
>> +
>> +    if ( unlikely(avail_pages <= low_mem_virq_th) )
>>      {
>>          send_global_virq(VIRQ_ENOMEM);
>>
>> @@ -391,7 +394,7 @@ static void check_low_mem_virq(void)
>>          return;
>>      }
>>
>> -    if ( unlikely(total_avail_pages >= low_mem_virq_high) )
>> +    if ( unlikely(avail_pages >= low_mem_virq_high) )
>>      {
>>          /* Reset hysteresis. Bring threshold up one order.
>>           * If we are back where originally set, set high
>
>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Low mem virq incremental adjustments
  2012-03-07 18:12   ` Andres Lagar-Cavilla
@ 2012-03-08 21:59     ` Dan Magenheimer
  2012-03-09  9:04       ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Magenheimer @ 2012-03-08 21:59 UTC (permalink / raw)
  To: andres, Jan Beulich
  Cc: ian.campbell, andres, tim, xen-devel, ian.jackson, adin

> 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.

Dan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Low mem virq incremental adjustments
  2012-03-08 21:59     ` Dan Magenheimer
@ 2012-03-09  9:04       ` Jan Beulich
  2012-03-09  9:56         ` Keir Fraser
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-03-09  9:04 UTC (permalink / raw)
  To: andres
  Cc: Dan Magenheimer, ian.campbell, andres, tim, xen-devel,
	ian.jackson, adin

>>> 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.

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Low mem virq incremental adjustments
  2012-03-09  9:04       ` Jan Beulich
@ 2012-03-09  9:56         ` Keir Fraser
  0 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2012-03-09  9:56 UTC (permalink / raw)
  To: Jan Beulich, andres
  Cc: Dan Magenheimer, ian.campbell, andres, tim, xen-devel,
	ian.jackson, adin

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-03-09  9:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-07 16:15 [PATCH] Low mem virq incremental adjustments Andres Lagar-Cavilla
2012-03-07 17:02 ` Jan Beulich
2012-03-07 18:12   ` Andres Lagar-Cavilla
2012-03-08 21:59     ` Dan Magenheimer
2012-03-09  9:04       ` Jan Beulich
2012-03-09  9:56         ` Keir Fraser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).