From: Markus Elfring <[email protected]>
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 <[email protected]>
---
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
On Mon, Jan 1, 2018 at 8:07 AM, SF Markus Elfring
<[email protected]> wrote:
> From: Markus Elfring <[email protected]>
> 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 <[email protected]>
> ---
> 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
>
>> 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
On Wed, Jan 3, 2018 at 12:45 AM, SF Markus Elfring
<[email protected]> 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
On Wed, 2018-01-03 at 11:28 -0800, Mahesh Bandewar (महेश बंडेवार)
wrote:
> On Wed, Jan 3, 2018 at 12:45 AM, SF Markus Elfring
> <[email protected]> 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.
> 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
On Thu, Jan 4, 2018 at 12:19 AM, SF Markus Elfring
<[email protected]> 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
>>> 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