Re: [Xen-devel] [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 >>> --- >>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [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@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH] drivers: xen-selfballoon: consider slab pages
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 >> --- >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [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@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH] drivers: xen-selfballoon: consider slab pages
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@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
