Messages in this thread | | | Date | Tue, 05 Nov 2013 11:30:08 +0800 | From | Bob Liu <> | Subject | 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.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
| |