Currently the goal_page in xen-selfballon doesn''t consider much about pages used in kernel space. A typical usage is slab pages, without consider slab pages the goal_page result may be too rough and lead extra memory pressure to guest os. Signed-off-by: Bob Liu <bob.liu@oracle.com> --- drivers/xen/xen-selfballoon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/xen/xen-selfballoon.c b/drivers/xen/xen-selfballoon.c index 21e18c1..4814759 100644 --- a/drivers/xen/xen-selfballoon.c +++ b/drivers/xen/xen-selfballoon.c @@ -191,6 +191,8 @@ static void selfballoon_process(struct work_struct *work) tgt_pages = cur_pages; /* default is no change */ goal_pages = vm_memory_committed() + totalreserve_pages + + global_page_state(NR_SLAB_RECLAIMABLE) + + global_page_state(NR_SLAB_UNRECLAIMABLE) + MB2PAGES(selfballoon_reserved_mb); #ifdef CONFIG_FRONTSWAP /* allow space for frontswap pages to be repatriated */ -- 1.7.10.4
Konrad Rzeszutek Wilk
2013-Nov-04 15:13 UTC
Re: [PATCH] drivers: xen-selfballoon: consider slab pages
On Mon, Nov 04, 2013 at 08:39:24PM +0800, Bob Liu wrote:> Currently the goal_page in xen-selfballon doesn''t consider much about pages used > in kernel space. > A typical usage is slab pages, without consider slab pages the goal_page result > may be too rough and lead extra memory pressure to guest os.Does "lead extra memory pressure" mean OOM?> > Signed-off-by: Bob Liu <bob.liu@oracle.com> > --- > drivers/xen/xen-selfballoon.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/xen/xen-selfballoon.c b/drivers/xen/xen-selfballoon.c > index 21e18c1..4814759 100644 > --- a/drivers/xen/xen-selfballoon.c > +++ b/drivers/xen/xen-selfballoon.c > @@ -191,6 +191,8 @@ static void selfballoon_process(struct work_struct *work) > tgt_pages = cur_pages; /* default is no change */ > goal_pages = vm_memory_committed() + > totalreserve_pages + > + global_page_state(NR_SLAB_RECLAIMABLE) + > + global_page_state(NR_SLAB_UNRECLAIMABLE) + > MB2PAGES(selfballoon_reserved_mb); > #ifdef CONFIG_FRONTSWAP > /* allow space for frontswap pages to be repatriated */ > -- > 1.7.10.4 >
David Vrabel
2013-Nov-04 17:22 UTC
Re: [PATCH] drivers: xen-selfballoon: consider slab pages
On 04/11/13 12:39, Bob Liu wrote:> Currently the goal_page in xen-selfballon doesn''t consider much about pages used > in kernel space. > A typical usage is slab pages, without consider slab pages the goal_page result > may be too rough and lead extra memory pressure to guest os.Can you provide some real world figures where the calculatation got it wrong? What was the resultant behavior? Swap death? OOM killer?> Signed-off-by: Bob Liu <bob.liu@oracle.com> > --- > drivers/xen/xen-selfballoon.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/xen/xen-selfballoon.c b/drivers/xen/xen-selfballoon.c > index 21e18c1..4814759 100644 > --- a/drivers/xen/xen-selfballoon.c > +++ b/drivers/xen/xen-selfballoon.c > @@ -191,6 +191,8 @@ static void selfballoon_process(struct work_struct *work) > tgt_pages = cur_pages; /* default is no change */ > goal_pages = vm_memory_committed() + > totalreserve_pages + > + global_page_state(NR_SLAB_RECLAIMABLE) +Does SLAB_RECLAIMABLE want to be included here? Unless I''m misunderstanding here, SLAB_RECLAIMABLE is effectively free.> + global_page_state(NR_SLAB_UNRECLAIMABLE) +This bit looks fine to me.> MB2PAGES(selfballoon_reserved_mb); > #ifdef CONFIG_FRONTSWAP > /* allow space for frontswap pages to be repatriated */David
On 11/05/2013 01:22 AM, David Vrabel wrote:> On 04/11/13 12:39, Bob Liu wrote: >> Currently the goal_page in xen-selfballon doesn''t consider much about pages used >> in kernel space. >> A typical usage is slab pages, without consider slab pages the goal_page result >> may be too rough and lead extra memory pressure to guest os. > > Can you provide some real world figures where the calculatation got it > wrong? What was the resultant behavior? Swap death? OOM killer? >Sorry, I didn''t run any testing I just think it''s unreasonable while reading the source code. vm_memory_committed() only calculate pages which mapped to process address space, but the kernel itself(like block, fs and network subsystem) may occupy some memory. And it''s possible that those subsystem may occupy a significant amount of memory in some situation. I''m afraid if we don''t consider those kernel memory while calculating goal_pages, guest memory will be set lower than guest really needs.>> Signed-off-by: Bob Liu <bob.liu@oracle.com> >> --- >> drivers/xen/xen-selfballoon.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/xen/xen-selfballoon.c b/drivers/xen/xen-selfballoon.c >> index 21e18c1..4814759 100644 >> --- a/drivers/xen/xen-selfballoon.c >> +++ b/drivers/xen/xen-selfballoon.c >> @@ -191,6 +191,8 @@ static void selfballoon_process(struct work_struct *work) >> tgt_pages = cur_pages; /* default is no change */ >> goal_pages = vm_memory_committed() + >> totalreserve_pages + >> + global_page_state(NR_SLAB_RECLAIMABLE) + > > Does SLAB_RECLAIMABLE want to be included here? Unless I''m > misunderstanding here, SLAB_RECLAIMABLE is effectively free. >SLAB_RECLAIMABLE isn''t effectively free, it means the slab page is in used but can be reclaimed(freed) during memory pressure.>> + global_page_state(NR_SLAB_UNRECLAIMABLE) + > > This bit looks fine to me. > >> MB2PAGES(selfballoon_reserved_mb); >> #ifdef CONFIG_FRONTSWAP >> /* allow space for frontswap pages to be repatriated */ > > David >Thanks for your review. -- Regards, -Bob
David Vrabel
2013-Nov-05 10:33 UTC
Re: [PATCH] drivers: xen-selfballoon: consider slab pages
On 05/11/13 03:30, Bob Liu wrote:> > On 11/05/2013 01:22 AM, David Vrabel wrote: >> On 04/11/13 12:39, Bob Liu wrote: >>> Currently the goal_page in xen-selfballon doesn''t consider much about pages used >>> in kernel space. >>> A typical usage is slab pages, without consider slab pages the goal_page result >>> may be too rough and lead extra memory pressure to guest os. >> >> Can you provide some real world figures where the calculatation got it >> wrong? What was the resultant behavior? Swap death? OOM killer? >> > > Sorry, I didn''t run any testing I just think it''s unreasonable while > reading the source code.I''m not keen on changes to heuristics without testing with real workloads and a demonstration that it is better in practice.> vm_memory_committed() only calculate pages which mapped to process > address space, but the kernel itself(like block, fs and network > subsystem) may occupy some memory. And it''s possible that those > subsystem may occupy a significant amount of memory in some situation. > > I''m afraid if we don''t consider those kernel memory while calculating > goal_pages, guest memory will be set lower than guest really needs. > >>> Signed-off-by: Bob Liu <bob.liu@oracle.com> >>> --- >>> drivers/xen/xen-selfballoon.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/xen/xen-selfballoon.c b/drivers/xen/xen-selfballoon.c >>> index 21e18c1..4814759 100644 >>> --- a/drivers/xen/xen-selfballoon.c >>> +++ b/drivers/xen/xen-selfballoon.c >>> @@ -191,6 +191,8 @@ static void selfballoon_process(struct work_struct *work) >>> tgt_pages = cur_pages; /* default is no change */ >>> goal_pages = vm_memory_committed() + >>> totalreserve_pages + >>> + global_page_state(NR_SLAB_RECLAIMABLE) + >> >> Does SLAB_RECLAIMABLE want to be included here? Unless I''m >> misunderstanding here, SLAB_RECLAIMABLE is effectively free. >> > > SLAB_RECLAIMABLE isn''t effectively free, it means the slab page is in > used but can be reclaimed(freed) during memory pressure.Similarly to the pages used for the page cache, I think the intention is for the selfballoon_reserved_mb value to include this.>>> + global_page_state(NR_SLAB_UNRECLAIMABLE) + >> >> This bit looks fine to me. >> >>> MB2PAGES(selfballoon_reserved_mb); >>> #ifdef CONFIG_FRONTSWAP >>> /* allow space for frontswap pages to be repatriated */David