* Re: [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-04 12:39 [PATCH] drivers: xen-selfballoon: consider slab pages Bob Liu
@ 2013-11-04 15:13 ` Konrad Rzeszutek Wilk
2013-11-04 15:13 ` Konrad Rzeszutek Wilk
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-11-04 15:13 UTC (permalink / raw)
To: Bob Liu; +Cc: xen-devel, linux-kernel, Bob Liu
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-04 12:39 [PATCH] drivers: xen-selfballoon: consider slab pages Bob Liu
2013-11-04 15:13 ` Konrad Rzeszutek Wilk
@ 2013-11-04 15:13 ` Konrad Rzeszutek Wilk
2013-11-04 17:22 ` [Xen-devel] " David Vrabel
2013-11-04 17:22 ` David Vrabel
3 siblings, 0 replies; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-11-04 15:13 UTC (permalink / raw)
To: Bob Liu; +Cc: xen-devel, Bob Liu, linux-kernel
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Xen-devel] [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-04 12:39 [PATCH] drivers: xen-selfballoon: consider slab pages Bob Liu
2013-11-04 15:13 ` Konrad Rzeszutek Wilk
2013-11-04 15:13 ` Konrad Rzeszutek Wilk
@ 2013-11-04 17:22 ` David Vrabel
2013-11-05 3:30 ` Bob Liu
2013-11-05 3:30 ` Bob Liu
2013-11-04 17:22 ` David Vrabel
3 siblings, 2 replies; 9+ messages in thread
From: David Vrabel @ 2013-11-04 17:22 UTC (permalink / raw)
To: Bob Liu; +Cc: konrad.wilk, xen-devel, Bob Liu, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Xen-devel] [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-04 17:22 ` [Xen-devel] " David Vrabel
@ 2013-11-05 3:30 ` Bob Liu
2013-11-05 10:33 ` David Vrabel
2013-11-05 10:33 ` [Xen-devel] " David Vrabel
2013-11-05 3:30 ` Bob Liu
1 sibling, 2 replies; 9+ messages in thread
From: Bob Liu @ 2013-11-05 3:30 UTC (permalink / raw)
To: David Vrabel; +Cc: Bob Liu, konrad.wilk, xen-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-05 3:30 ` Bob Liu
@ 2013-11-05 10:33 ` David Vrabel
2013-11-05 10:33 ` [Xen-devel] " David Vrabel
1 sibling, 0 replies; 9+ messages in thread
From: David Vrabel @ 2013-11-05 10:33 UTC (permalink / raw)
To: Bob Liu; +Cc: xen-devel, Bob Liu, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Xen-devel] [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-05 3:30 ` Bob Liu
2013-11-05 10:33 ` David Vrabel
@ 2013-11-05 10:33 ` David Vrabel
1 sibling, 0 replies; 9+ messages in thread
From: David Vrabel @ 2013-11-05 10:33 UTC (permalink / raw)
To: Bob Liu; +Cc: Bob Liu, konrad.wilk, xen-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-04 17:22 ` [Xen-devel] " David Vrabel
2013-11-05 3:30 ` Bob Liu
@ 2013-11-05 3:30 ` Bob Liu
1 sibling, 0 replies; 9+ messages in thread
From: Bob Liu @ 2013-11-05 3:30 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, Bob Liu, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drivers: xen-selfballoon: consider slab pages
2013-11-04 12:39 [PATCH] drivers: xen-selfballoon: consider slab pages Bob Liu
` (2 preceding siblings ...)
2013-11-04 17:22 ` [Xen-devel] " David Vrabel
@ 2013-11-04 17:22 ` David Vrabel
3 siblings, 0 replies; 9+ messages in thread
From: David Vrabel @ 2013-11-04 17:22 UTC (permalink / raw)
To: Bob Liu; +Cc: xen-devel, Bob Liu, linux-kernel
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
^ permalink raw reply [flat|nested] 9+ messages in thread