[PATCH] xen/balloon: cancel ballooning if adding new memory failed

[PATCH] xen/balloon: cancel ballooning if adding new memory failed

[David Vrabel]

If the balloon driver is adding additional memory regions to the
balloon and add_memory() fails it will likely continuously fail so
cancel the balloon operation.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 drivers/xen/balloon.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 5c660c7..1e0a317 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -230,8 +230,8 @@ static enum bp_state reserve_additional_memory(long credit)
 	rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << PAGE_SHIFT);
 
 	if (rc) {
-		pr_info("%s: add_memory() failed: %i\n", __func__, rc);
-		return BP_EAGAIN;
+		pr_warn("Cannot add additional memory (%i)\n", rc);
+		return BP_ECANCELED;
 	}
 
 	balloon_hotplug -= credit;
-- 
1.7.10.4

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[Daniel Kiper]

On Mon, Sep 01, 2014 at 06:52:44PM +0100, David Vrabel wrote:
> If the balloon driver is adding additional memory regions to the
> balloon and add_memory() fails it will likely continuously fail so
> cancel the balloon operation.
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Make sense for me.

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

Daniel

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[David Vrabel]

On 01/09/14 23:07, Daniel Kiper wrote:
> On Mon, Sep 01, 2014 at 06:52:44PM +0100, David Vrabel wrote:
>> If the balloon driver is adding additional memory regions to the
>> balloon and add_memory() fails it will likely continuously fail so
>> cancel the balloon operation.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> Make sense for me.
> 
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Applied to to stable/for-linus-3.17-b

Thanks.

David

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[Ian Campbell]

On Mon, 2014-09-01 at 18:52 +0100, David Vrabel wrote:
> If the balloon driver is adding additional memory regions to the
> balloon and add_memory() fails it will likely continuously fail so
> cancel the balloon operation.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776448 and
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1304001 seem to
suggest this should be a candidate for stable backports?

Ian.

> ---
>  drivers/xen/balloon.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 5c660c7..1e0a317 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -230,8 +230,8 @@ static enum bp_state reserve_additional_memory(long credit)
>  	rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << PAGE_SHIFT);
>  
>  	if (rc) {
> -		pr_info("%s: add_memory() failed: %i\n", __func__, rc);
> -		return BP_EAGAIN;
> +		pr_warn("Cannot add additional memory (%i)\n", rc);
> +		return BP_ECANCELED;
>  	}
>  
>  	balloon_hotplug -= credit;

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[David Vrabel]

On 29/01/15 13:36, Ian Campbell wrote:
> On Mon, 2014-09-01 at 18:52 +0100, David Vrabel wrote:
>> If the balloon driver is adding additional memory regions to the
>> balloon and add_memory() fails it will likely continuously fail so
>> cancel the balloon operation.
>>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776448 and
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1304001 seem to
> suggest this should be a candidate for stable backports?

It's up to the distro kernel maintainer to request it if they think it
is important.  It's a harmless message with no side effects so I don't
think it qualifies.

David

>>  drivers/xen/balloon.c |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
>> index 5c660c7..1e0a317 100644
>> --- a/drivers/xen/balloon.c
>> +++ b/drivers/xen/balloon.c
>> @@ -230,8 +230,8 @@ static enum bp_state reserve_additional_memory(long credit)
>>  	rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << PAGE_SHIFT);
>>  
>>  	if (rc) {
>> -		pr_info("%s: add_memory() failed: %i\n", __func__, rc);
>> -		return BP_EAGAIN;
>> +		pr_warn("Cannot add additional memory (%i)\n", rc);
>> +		return BP_ECANCELED;
>>  	}
>>  
>>  	balloon_hotplug -= credit;
> 
> 

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[Daniel Kiper]

On Thu, Jan 29, 2015 at 06:01:36PM +0000, David Vrabel wrote:
> On 29/01/15 13:36, Ian Campbell wrote:
> > On Mon, 2014-09-01 at 18:52 +0100, David Vrabel wrote:
> >> If the balloon driver is adding additional memory regions to the
> >> balloon and add_memory() fails it will likely continuously fail so
> >> cancel the balloon operation.
> >>
> >> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> >
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776448 and
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1304001 seem to
> > suggest this should be a candidate for stable backports?
>
> It's up to the distro kernel maintainer to request it if they think it
> is important.  It's a harmless message with no side effects so I don't
> think it qualifies.

It pollutes system logs, makes unnecessary confusion and from time to time
noise on various lists. Fix is trivial, so, why not. However, as David said
this is up to stable kernel maintainer.

If yes you should also backport commit fd8b79511349efd1f0decea920f61b93acb34a75
(xen/balloon: Don't continue ballooning when BP_ECANCELED is encountered).
IIRC, both (trivial) patches are needed to fix this issue.

Daniel

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[Ian Campbell]

On Thu, 2015-01-29 at 18:01 +0000, David Vrabel wrote:
> On 29/01/15 13:36, Ian Campbell wrote:
> > On Mon, 2014-09-01 at 18:52 +0100, David Vrabel wrote:
> >> If the balloon driver is adding additional memory regions to the
> >> balloon and add_memory() fails it will likely continuously fail so
> >> cancel the balloon operation.
> >>
> >> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> > 
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776448 and
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1304001 seem to
> > suggest this should be a candidate for stable backports?
> 
> It's up to the distro kernel maintainer to request it if they think it
> is important.

Which is what I thought I was doing. Surely you don't think I should
just ask stable@ without conferring with the relevant maintainers first?

Ian.

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[David Vrabel]

On 30/01/15 11:34, Ian Campbell wrote:
> On Thu, 2015-01-29 at 18:01 +0000, David Vrabel wrote:
>> On 29/01/15 13:36, Ian Campbell wrote:
>>> On Mon, 2014-09-01 at 18:52 +0100, David Vrabel wrote:
>>>> If the balloon driver is adding additional memory regions to the
>>>> balloon and add_memory() fails it will likely continuously fail so
>>>> cancel the balloon operation.
>>>>
>>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>>
>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776448 and
>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1304001 seem to
>>> suggest this should be a candidate for stable backports?
>>
>> It's up to the distro kernel maintainer to request it if they think it
>> is important.
> 
> Which is what I thought I was doing. Surely you don't think I should
> just ask stable@ without conferring with the relevant maintainers first?

This fix doesn't even remotely qualify for stable.  As
stable_kernel_rules.txt says "In short, something critical."

You are of course free to send the request to stable@ yourself if you
disagree.

David

Re: [PATCH] xen/balloon: cancel ballooning if adding new memory failed

[Ian Campbell]

On Fri, 2015-01-30 at 11:53 +0000, David Vrabel wrote:
> On 30/01/15 11:34, Ian Campbell wrote:
> > On Thu, 2015-01-29 at 18:01 +0000, David Vrabel wrote:
> >> On 29/01/15 13:36, Ian Campbell wrote:
> >>> On Mon, 2014-09-01 at 18:52 +0100, David Vrabel wrote:
> >>>> If the balloon driver is adding additional memory regions to the
> >>>> balloon and add_memory() fails it will likely continuously fail so
> >>>> cancel the balloon operation.
> >>>>
> >>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> >>>
> >>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=776448 and
> >>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1304001 seem to
> >>> suggest this should be a candidate for stable backports?
> >>
> >> It's up to the distro kernel maintainer to request it if they think it
> >> is important.
> > 
> > Which is what I thought I was doing. Surely you don't think I should
> > just ask stable@ without conferring with the relevant maintainers first?
> 
> This fix doesn't even remotely qualify for stable.  As
> stable_kernel_rules.txt says "In short, something critical."

Two people can selectively quote from rules.txt: "It must fix a real bug
that bothers people" and from the bug "And they keep repeating forever,
quite frequently.", seems like a good enough reason to me.

> You are of course free to send the request to stable@ yourself if you
> disagree.

I've found your immediately dismissive attitude here quite off-putting
and unnecessary. Of course it is completely reasonable of me to
highlight a potential backport to the maintainers and the people CCd on
the original patch.

Ian.