Messages in this thread |  | | Date | Tue, 5 Nov 2013 10:33:53 +0000 | From | David Vrabel <> | Subject | 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.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
|  |