[PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[Boris Ostrovsky]

Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
memory failed") makes reserve_additional_memory() return BP_ECANCELED
when an error is encountered. This error, however, is ignored by the
caller (balloon_process()) since it is overwritten by subsequent call
to update_schedule(). This results in continuous attempts to add more
memory, all of which are likely to fail again.

We should stop trying to schedule next iteration of ballooning when
the current one has failed.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
v2: Move status test to update_schedule() 

 drivers/xen/balloon.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 1e0a317..3860d02 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -167,6 +167,9 @@ static struct page *balloon_next_page(struct page *page)
 
 static enum bp_state update_schedule(enum bp_state state)
 {
+	if (state == BP_ECANCELED)
+		return BP_ECANCELED;
+
 	if (state == BP_DONE) {
 		balloon_stats.schedule_delay = 1;
 		balloon_stats.retry_count = 1;
-- 
1.8.3.1

Re: [PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[Daniel Kiper]

On Tue, Oct 07, 2014 at 05:00:07PM -0400, Boris Ostrovsky wrote:
> Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
> memory failed") makes reserve_additional_memory() return BP_ECANCELED
> when an error is encountered. This error, however, is ignored by the
> caller (balloon_process()) since it is overwritten by subsequent call
> to update_schedule(). This results in continuous attempts to add more
> memory, all of which are likely to fail again.
>
> We should stop trying to schedule next iteration of ballooning when
> the current one has failed.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> v2: Move status test to update_schedule()
>
>  drivers/xen/balloon.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 1e0a317..3860d02 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -167,6 +167,9 @@ static struct page *balloon_next_page(struct page *page)
>
>  static enum bp_state update_schedule(enum bp_state state)
>  {
> +	if (state == BP_ECANCELED)
> +		return BP_ECANCELED;
> +
>  	if (state == BP_DONE) {
>  		balloon_stats.schedule_delay = 1;
>  		balloon_stats.retry_count = 1;

Thanks. Please next time send Linux Kernel patches to LKML too.
Cc-ing it just in case.

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[Daniel Kiper]

On Tue, Oct 07, 2014 at 05:00:07PM -0400, Boris Ostrovsky wrote:
> Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
> memory failed") makes reserve_additional_memory() return BP_ECANCELED
> when an error is encountered. This error, however, is ignored by the
> caller (balloon_process()) since it is overwritten by subsequent call
> to update_schedule(). This results in continuous attempts to add more
> memory, all of which are likely to fail again.
>
> We should stop trying to schedule next iteration of ballooning when
> the current one has failed.
>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
> v2: Move status test to update_schedule()
>
>  drivers/xen/balloon.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 1e0a317..3860d02 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -167,6 +167,9 @@ static struct page *balloon_next_page(struct page *page)
>
>  static enum bp_state update_schedule(enum bp_state state)
>  {
> +	if (state == BP_ECANCELED)
> +		return BP_ECANCELED;
> +
>  	if (state == BP_DONE) {
>  		balloon_stats.schedule_delay = 1;
>  		balloon_stats.retry_count = 1;

Thanks. Please next time send Linux Kernel patches to LKML too.
Cc-ing it just in case.

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[David Vrabel]

On 07/10/14 22:00, Boris Ostrovsky wrote:
> Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
> memory failed") makes reserve_additional_memory() return BP_ECANCELED
> when an error is encountered. This error, however, is ignored by the
> caller (balloon_process()) since it is overwritten by subsequent call
> to update_schedule(). This results in continuous attempts to add more
> memory, all of which are likely to fail again.
> 
> We should stop trying to schedule next iteration of ballooning when
> the current one has failed.

What triggers this bug?  I could never work out what it was.

Applied to stable/for-linus-3.18

David

Re: [PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[Boris Ostrovsky]

On 10/09/2014 01:17 PM, David Vrabel wrote:
> On 07/10/14 22:00, Boris Ostrovsky wrote:
>> Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
>> memory failed") makes reserve_additional_memory() return BP_ECANCELED
>> when an error is encountered. This error, however, is ignored by the
>> caller (balloon_process()) since it is overwritten by subsequent call
>> to update_schedule(). This results in continuous attempts to add more
>> memory, all of which are likely to fail again.
>>
>> We should stop trying to schedule next iteration of ballooning when
>> the current one has failed.
> What triggers this bug?  I could never work out what it was.

I don't know, I haven't been able to reproduce it neither. It's clearly 
something in AWS.

But once you hit it (which you can do by simulating add_memory() return 
an error) you will keep retrying, getting the same error back and 
flooding the log.

-boris

>
> Applied to stable/for-linus-3.18
>
> David

Re: [PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[Stefan Bader]

[-- Attachment #1.1: Type: text/plain, Size: 1050 bytes --]

On 09.10.2014 19:17, David Vrabel wrote:
> On 07/10/14 22:00, Boris Ostrovsky wrote:
>> Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
>> memory failed") makes reserve_additional_memory() return BP_ECANCELED
>> when an error is encountered. This error, however, is ignored by the
>> caller (balloon_process()) since it is overwritten by subsequent call
>> to update_schedule(). This results in continuous attempts to add more
>> memory, all of which are likely to fail again.
>>
>> We should stop trying to schedule next iteration of ballooning when
>> the current one has failed.
> 
> What triggers this bug?  I could never work out what it was.
> 
> Applied to stable/for-linus-3.18

I just checked in the kernel upstream repo and cannot find this but I believe
the initial pull was already made. Or was there a revert that I missed?

-Stefan

> 
> David
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v2] xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered

[David Vrabel]

On 22/10/14 15:42, Stefan Bader wrote:
> On 09.10.2014 19:17, David Vrabel wrote:
>> On 07/10/14 22:00, Boris Ostrovsky wrote:
>>> Commit 3dcf63677d4e ("xen/balloon: cancel ballooning if adding new
>>> memory failed") makes reserve_additional_memory() return BP_ECANCELED
>>> when an error is encountered. This error, however, is ignored by the
>>> caller (balloon_process()) since it is overwritten by subsequent call
>>> to update_schedule(). This results in continuous attempts to add more
>>> memory, all of which are likely to fail again.
>>>
>>> We should stop trying to schedule next iteration of ballooning when
>>> the current one has failed.
>>
>> What triggers this bug?  I could never work out what it was.
>>
>> Applied to stable/for-linus-3.18
> 
> I just checked in the kernel upstream repo and cannot find this but I believe
> the initial pull was already made. Or was there a revert that I missed?

I've not sent the pull request for it yet.

David