.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
>>> 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
Andres Lagar-Cavilla
2012-Mar-07 18:12 UTC
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> > 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 > > > >
> 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. > AndresBoth 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
>>> 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
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