[PATCH] bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

[PATCH] bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 1 Jan 2018 17:00:04 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/bonding/bond_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c669554d70bb..a96e0c9cc4bf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3910,7 +3910,6 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
 			  GFP_KERNEL);
 	if (!new_arr) {
 		ret = -ENOMEM;
-		pr_err("Failed to build slave-array.\n");
 		goto out;
 	}
 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
-- 
2.15.1

Re: [PATCH] bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

[Mahesh Bandewar (महेश बंडेवार)]

On Mon, Jan 1, 2018 at 8:07 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 1 Jan 2018 17:00:04 +0100
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
What is the issue with this message?

> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/net/bonding/bond_main.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index c669554d70bb..a96e0c9cc4bf 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3910,7 +3910,6 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
>                           GFP_KERNEL);
>         if (!new_arr) {
>                 ret = -ENOMEM;
> -               pr_err("Failed to build slave-array.\n");
>                 goto out;
>         }
>         if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> --
> 2.15.1
>

Re: bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

[SF Markus Elfring]

>> Omit an extra message for a memory allocation failure in this function.
>>
>> This issue was detected by using the Coccinelle software.
>>
> What is the issue with this message?

* Is it redundant?

* Would a Linux allocation failure report be already sufficient here?

Regards,
Markus

Re: bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

[Mahesh Bandewar (महेश बंडेवार)]

On Wed, Jan 3, 2018 at 12:45 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>>> Omit an extra message for a memory allocation failure in this function.
>>>
>>> This issue was detected by using the Coccinelle software.
>>>
>> What is the issue with this message?
>
> * Is it redundant?
>
> * Would a Linux allocation failure report be already sufficient here?
>
If you see 8 out of 9 call sites in this file ignore the return value.
This message in the log could give a clue when debugging. Unless it's
spamming it's not harmful, or is it?

> Regards,
> Markus

Re: bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()

[Eric Dumazet]

On Wed, 2018-01-03 at 11:28 -0800, Mahesh Bandewar (महेश बंडेवार)
wrote:
> On Wed, Jan 3, 2018 at 12:45 AM, SF Markus Elfring
> <elfring@users.sourceforge.net> wrote:
> > > > Omit an extra message for a memory allocation failure in this function.
> > > > 
> > > > This issue was detected by using the Coccinelle software.
> > > > 
> > > 
> > > What is the issue with this message?
> > 
> > * Is it redundant?
> > 
> > * Would a Linux allocation failure report be already sufficient here?
> > 
> 
> If you see 8 out of 9 call sites in this file ignore the return value.
> This message in the log could give a clue when debugging. Unless it's
> spamming it's not harmful, or is it?

A failed kzalloc() would already give a complete stack trace.

Really the pr_err() adds no value here.

Re: bonding: Completion of error handling around bond_update_slave_arr()

[SF Markus Elfring]

> If you see 8 out of 9 call sites in this file ignore the return value.

How do you think about to fix error detection and corresponding
exception handling then?

Regards,
Markus

Re: bonding: Completion of error handling around bond_update_slave_arr()

[Mahesh Bandewar (महेश बंडेवार)]

On Thu, Jan 4, 2018 at 12:19 AM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
>> If you see 8 out of 9 call sites in this file ignore the return value.
>
> How do you think about to fix error detection and corresponding
> exception handling then?
>
If I understand your question correctly - not having memory is not a
correctable error and hence there are consequences. In this case, the
slave_arr is not going to be rebuilt and this might mean loosing
packets (if the interface was dropped) or not using the interface to
send packets if  that was added (very unlikely case). From host's
perspective, however, this might be the last thing you want to worry
about when there is no memory left.

> Regards,
> Markus

Re: bonding: Completion of error handling around bond_update_slave_arr()

[SF Markus Elfring]

>>> If you see 8 out of 9 call sites in this file ignore the return value.
>>
>> How do you think about to fix error detection and corresponding
>> exception handling then?
>>
> If I understand your question correctly - not having memory is not a
> correctable error

I am unsure if it would be feasible to retry memory allocations for
this software module under other circumstances.


> and hence there are consequences.

Could one consequence be to let the error code “-ENOMEM” move through
the function call hierarchy?

Regards,
Markus