[PATCH 0/3] RDS: Adjustments for two function implementations

[PATCH 0/3] RDS: Adjustments for two function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 22 May 2017 16:02:03 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Delete an error message for a failed memory allocation in rds_ib_add_one()
  Improve a size determination in rds_ib_add_one()
  Delete an error message for a failed memory allocation in rds_tcp_init_net()

 net/rds/ib.c  | 7 ++-----
 net/rds/tcp.c | 5 ++---
 2 files changed, 4 insertions(+), 8 deletions(-)

-- 
2.13.0

[PATCH 1/3] RDS: IB: Delete an error message for a failed memory allocation in rds_ib_add_one()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 22 May 2017 15:34:28 +0200

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

This issue was detected by using the Coccinelle software.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/rds/ib.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/rds/ib.c b/net/rds/ib.c
index 7a64c8db81ab..c5514d058171 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -166,8 +166,5 @@ static void rds_ib_add_one(struct ib_device *device)
-	if (!rds_ibdev->vector_load) {
-		pr_err("RDS/IB: %s failed to allocate vector memory\n",
-			__func__);
+	if (!rds_ibdev->vector_load)
 		goto put_dev;
-	}
 
 	rds_ibdev->dev = device;
 	rds_ibdev->pd = ib_alloc_pd(device, 0);
-- 
2.13.0

[PATCH 2/3] RDS: IB: Improve a size determination in rds_ib_add_one()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 22 May 2017 15:40:21 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/rds/ib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rds/ib.c b/net/rds/ib.c
index c5514d058171..ed37bf011704 100644
--- a/net/rds/ib.c
+++ b/net/rds/ib.c
@@ -131,7 +131,7 @@ static void rds_ib_add_one(struct ib_device *device)
 	if (device->node_type != RDMA_NODE_IB_CA)
 		return;
 
-	rds_ibdev = kzalloc_node(sizeof(struct rds_ib_device), GFP_KERNEL,
+	rds_ibdev = kzalloc_node(sizeof(*rds_ibdev), GFP_KERNEL,
 				 ibdev_to_node(device));
 	if (!rds_ibdev)
 		return;
-- 
2.13.0

[PATCH 3/3] RDS: TCP: Delete an error message for a failed memory allocation in rds_tcp_init_net()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 22 May 2017 15:45:31 +0200

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

This issue was detected by using the Coccinelle software.

Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/rds/tcp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/rds/tcp.c b/net/rds/tcp.c
index 431404dbdad1..b0348697dee5 100644
--- a/net/rds/tcp.c
+++ b/net/rds/tcp.c
@@ -431,10 +431,9 @@ static __net_init int rds_tcp_init_net(struct net *net)
 	} else {
 		tbl = kmemdup(rds_tcp_sysctl_table,
 			      sizeof(rds_tcp_sysctl_table), GFP_KERNEL);
-		if (!tbl) {
-			pr_warn("could not set allocate syctl table\n");
+		if (!tbl)
 			return -ENOMEM;
-		}
+
 		rtn->ctl_table = tbl;
 	}
 	tbl[RDS_TCP_SNDBUF].data = &rtn->sndbuf_size;
-- 
2.13.0

Re: [PATCH 3/3] RDS: TCP: Delete an error message for a failed memory allocation in rds_tcp_init_net()

[Sowmini Varadhan]

On (05/22/17 16:13), SF Markus Elfring wrote:
> 
> Omit an extra message for a memory allocation failure in this function.

The change itself is harmless,  but I'm curious about the "extra"
part: "extra" from what? If this happens, hopefully this will be logged
somewhere? Note that this type of (infrequent) logging noise is useful
in some cases, e.g., with 8ce675ff, when one is trying to do the
post-mortem of where things first went wrong.

--Sowmini

Re: RDS: TCP: Delete an error message for a failed memory allocation in rds_tcp_init_net()

[SF Markus Elfring]

>> Omit an extra message for a memory allocation failure in this function.
> 
> The change itself is harmless,  but I'm curious about the "extra"
> part: "extra" from what? If this happens, hopefully this will be logged
> somewhere? Note that this type of (infrequent) logging noise is useful
> in some cases, e.g., with 8ce675ff, when one is trying to do the
> post-mortem of where things first went wrong.

Do you find information from a Linux allocation failure report sufficient
for such an use case?

Regards,
Markus

Re: RDS: TCP: Delete an error message for a failed memory allocation in rds_tcp_init_net()

[Sowmini Varadhan]

> Do you find information from a Linux allocation failure report sufficient
> for such an use case?

yes, I suppose that would cover the needed cases. 
Your change looks good to me.

Acked-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>

Re: [PATCH 2/3] RDS: IB: Improve a size determination in rds_ib_add_one()

[Santosh Shilimkar]

On 5/22/2017 7:12 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 15:40:21 +0200
> 
> Replace the specification of a data structure by a pointer dereference
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

Re: [PATCH 1/3] RDS: IB: Delete an error message for a failed memory allocation in rds_ib_add_one()

[Santosh Shilimkar]

On 5/22/2017 7:11 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 22 May 2017 15:34:28 +0200
> 
> Omit an extra message for a memory allocation failure in this function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   net/rds/ib.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index 7a64c8db81ab..c5514d058171 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -166,8 +166,5 @@ static void rds_ib_add_one(struct ib_device *device)
> -	if (!rds_ibdev->vector_load) {
> -		pr_err("RDS/IB: %s failed to allocate vector memory\n",
> -			__func__);
> +	if (!rds_ibdev->vector_load)
>   		goto put_dev;
> -	}
>   
Well the ENOMEM is not carried here so the message was usefu
but its not critical so its fine to clean that up.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>