On Sat, 2017-02-18 at 21:45 +0100, SF Markus Elfring wrote:
> From: Markus Elfring <[email protected]>
> Date: Sat, 18 Feb 2017 21:34:32 +0100
>
> Several update suggestions were taken into account
> from static source code analysis.
I've dropped this entire series. If you want me to consider it, you
need to respin it with the following changes:
1) Put all similar corrections in a single patch. If you are replacing
a sizeof() usage with a safer sizeof() usage, then I don't want one
patch per replacement, I want one patch for all replacements.
2) Use reasonable git commit messages. "Improve another" is never a
reasonable commit message because it implicitly refereces another git
commit message. Unless this is a fix for a previous commit, all commit
messages should be standalone.
3) Keep your git commit message subject lines within the recommended
size. You have many that are too long. As trivial as all of these
changes are, I'm not going to go fixup 29 commit messages to take them.
4) Make sure to preserve the reviewed-by tags on the resubmission.
> Markus Elfring (29):
> Use kcalloc() in mlx4_ib_alloc_pv_bufs()
> Improve another size determination in mlx4_ib_alloc_pv_bufs()
> Improve another size determination in mlx4_ib_alloc_demux_ctx()
> Improve another size determination in alloc_pv_object()
> Fix a typo in a comment line
> Delete three unnecessary return statements
> Split a condition check in handle_slaves_guid_change()
> Delete an unnecessary check before the function call "kfree" in
> free_pv_object()
> Move an assignment out of a check in forward_trap()
> Enclose 15 expressions for the sizeof operator by parentheses
> Use kmalloc_array() in three functions
> Enclose 17 expressions for the sizeof operator by parentheses
> Split a condition check in five functions
> Delete an unnecessary variable in __mlx4_ib_query_gid()
> Delete an unnecessary return statement in do_slave_init()
> Improve another size determination in do_slave_init()
> Improve another size determination in mlx4_ib_add()
> Delete an unnecessary variable initialisation in mlx4_ib_add()
> Delete an unnecessary variable assignment in mlx4_ib_add()
> Delete an error message for a failed memory allocation in
> mlx4_ib_add()
> Delete unnecessary braces in mlx4_ib_add()
> Use kmalloc_array() in alloc_proxy_bufs()
> Improve size determinations in create_qp_common()
> Delete unwanted spaces behind usages of the sizeof operator
> Add spaces for better code readability
> Enclose 14 expressions for the sizeof operator by parentheses
> Use kmalloc_array() in create_kernel_qp()
> Less function calls in create_kernel_qp() after error detection
> Use kmalloc_array() in create_srq_kernel()
>
> drivers/infiniband/hw/mlx4/mad.c | 72 ++++++++--------
> drivers/infiniband/hw/mlx4/main.c | 164 ++++++++++++++++++++------
> ----------
> drivers/infiniband/hw/mlx4/qp.c | 173 +++++++++++++++++++---------
> ----------
> drivers/infiniband/hw/mlx5/qp.c | 66 ++++++++++-----
> drivers/infiniband/hw/mlx5/srq.c | 5 +-
> 5 files changed, 261 insertions(+), 219 deletions(-)
>
--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
> I've dropped this entire series. If you want me to consider it, you
> need to respin it with the following changes:
How do you think about to integrate any of my update suggestions
which you do not find controversial (or questionable) at the moment?
> 1) Put all similar corrections in a single patch.
I preferred an other patch granularity so far.
Regards,
Markus
On Thu, 2017-04-20 at 23:02 +0200, SF Markus Elfring wrote:
> >
> > I've dropped this entire series. If you want me to consider it,
> > you
> > need to respin it with the following changes:
>
> How do you think about to integrate any of my update suggestions
> which you do not find controversial (or questionable) at the moment?
No. Far too many of your patches need commit messages fixes and the
like. If you want me to include them, then follow all of the
requirements I gave you.
>
> >
> > 1) Put all similar corrections in a single patch.
>
> I preferred an other patch granularity so far.
Not to be too blunt about it, but your preference is not the one that
matters in this situation. If you want me to consider the patches,
then you need to respin them following the directions I gave.
--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 10:21:01 +0200
* A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kcalloc".
This issue was detected by using the Coccinelle software.
* Replace the specification of a data structure by a pointer dereference
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <[email protected]>
Reviewed-by: Majd Dibbiny <[email protected]>
---
drivers/infiniband/hw/mlx4/mad.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index db564ccc0f92..61bd81baeb29 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1600,8 +1600,8 @@ static int mlx4_ib_alloc_pv_bufs(struct mlx4_ib_demux_pv_ctx *ctx,
return -EINVAL;
tun_qp = &ctx->qp[qp_type];
-
- tun_qp->ring = kzalloc(sizeof (struct mlx4_ib_buf) * MLX4_NUM_TUNNEL_BUFS,
+ tun_qp->ring = kcalloc(MLX4_NUM_TUNNEL_BUFS,
+ sizeof(*tun_qp->ring),
GFP_KERNEL);
if (!tun_qp->ring)
return -ENOMEM;
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 10:45:49 +0200
* Multiplications for the size determination of memory allocations
indicated that array data structures should be processed.
Thus use the corresponding function "kmalloc_array".
This issue was detected by using the Coccinelle software.
* Replace the specification of data types by pointer dereferences
to make the corresponding size determination a bit safer according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <[email protected]>
---
v2:
Changes were rebased on source files from Linux next-20170421.
These were recombined as requested by Doug Ledford.
drivers/infiniband/hw/mlx4/main.c | 14 +++++++++-----
drivers/infiniband/hw/mlx4/qp.c | 6 +++---
drivers/infiniband/hw/mlx5/qp.c | 21 +++++++++++++++------
drivers/infiniband/hw/mlx5/srq.c | 5 +++--
4 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index fba94df28cf1..30a70fa95fec 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -307,7 +307,9 @@ static int mlx4_ib_add_gid(struct ib_device *device,
ctx->refcount++;
}
if (!ret && hw_update) {
- gids = kmalloc(sizeof(*gids) * MLX4_MAX_PORT_GIDS, GFP_ATOMIC);
+ gids = kmalloc_array(MLX4_MAX_PORT_GIDS,
+ sizeof(*gids),
+ GFP_ATOMIC);
if (!gids) {
ret = -ENOMEM;
} else {
@@ -362,7 +364,9 @@ static int mlx4_ib_del_gid(struct ib_device *device,
if (!ret && hw_update) {
int i;
- gids = kmalloc(sizeof(*gids) * MLX4_MAX_PORT_GIDS, GFP_ATOMIC);
+ gids = kmalloc_array(MLX4_MAX_PORT_GIDS,
+ sizeof(*gids),
+ GFP_ATOMIC);
if (!gids) {
ret = -ENOMEM;
} else {
@@ -2831,9 +2835,9 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
goto err_counter;
ibdev->ib_uc_qpns_bitmap =
- kmalloc(BITS_TO_LONGS(ibdev->steer_qpn_count) *
- sizeof(long),
- GFP_KERNEL);
+ kmalloc_array(BITS_TO_LONGS(ibdev->steer_qpn_count),
+ sizeof(long),
+ GFP_KERNEL);
if (!ibdev->ib_uc_qpns_bitmap)
goto err_steer_qp_release;
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index c34eebc7db65..1d9d949c66d0 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -554,9 +554,9 @@ static int alloc_proxy_bufs(struct ib_device *dev, struct mlx4_ib_qp *qp)
{
int i;
- qp->sqp_proxy_rcv =
- kmalloc(sizeof (struct mlx4_ib_buf) * qp->rq.wqe_cnt,
- GFP_KERNEL);
+ qp->sqp_proxy_rcv = kmalloc_array(qp->rq.wqe_cnt,
+ sizeof(*qp->sqp_proxy_rcv),
+ GFP_KERNEL);
if (!qp->sqp_proxy_rcv)
return -ENOMEM;
for (i = 0; i < qp->rq.wqe_cnt; i++) {
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index ed6320186f89..1e98a8c9fab8 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -959,12 +959,21 @@ static int create_kernel_qp(struct mlx5_ib_dev *dev,
goto err_free;
}
- qp->sq.wrid = kmalloc(qp->sq.wqe_cnt * sizeof(*qp->sq.wrid), GFP_KERNEL);
- qp->sq.wr_data = kmalloc(qp->sq.wqe_cnt * sizeof(*qp->sq.wr_data), GFP_KERNEL);
- qp->rq.wrid = kmalloc(qp->rq.wqe_cnt * sizeof(*qp->rq.wrid), GFP_KERNEL);
- qp->sq.w_list = kmalloc(qp->sq.wqe_cnt * sizeof(*qp->sq.w_list), GFP_KERNEL);
- qp->sq.wqe_head = kmalloc(qp->sq.wqe_cnt * sizeof(*qp->sq.wqe_head), GFP_KERNEL);
-
+ qp->sq.wrid = kmalloc_array(qp->sq.wqe_cnt,
+ sizeof(*qp->sq.wrid),
+ GFP_KERNEL);
+ qp->sq.wr_data = kmalloc_array(qp->sq.wqe_cnt,
+ sizeof(*qp->sq.wr_data),
+ GFP_KERNEL);
+ qp->rq.wrid = kmalloc_array(qp->rq.wqe_cnt,
+ sizeof(*qp->rq.wrid),
+ GFP_KERNEL);
+ qp->sq.w_list = kmalloc_array(qp->sq.wqe_cnt,
+ sizeof(*qp->sq.w_list),
+ GFP_KERNEL);
+ qp->sq.wqe_head = kmalloc_array(qp->sq.wqe_cnt,
+ sizeof(*qp->sq.wqe_head),
+ GFP_KERNEL);
if (!qp->sq.wrid || !qp->sq.wr_data || !qp->rq.wrid ||
!qp->sq.w_list || !qp->sq.wqe_head) {
err = -ENOMEM;
diff --git a/drivers/infiniband/hw/mlx5/srq.c b/drivers/infiniband/hw/mlx5/srq.c
index 7cb145f9a6db..8ba1953177af 100644
--- a/drivers/infiniband/hw/mlx5/srq.c
+++ b/drivers/infiniband/hw/mlx5/srq.c
@@ -195,8 +195,9 @@ static int create_srq_kernel(struct mlx5_ib_dev *dev, struct mlx5_ib_srq *srq,
goto err_buf;
}
mlx5_fill_page_array(&srq->buf, in->pas);
-
- srq->wrid = kmalloc(srq->msrq.max * sizeof(u64), GFP_KERNEL);
+ srq->wrid = kmalloc_array(srq->msrq.max,
+ sizeof(*srq->wrid),
+ GFP_KERNEL);
if (!srq->wrid) {
err = -ENOMEM;
goto err_in;
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 19:56:54 +0200
Several update suggestions were taken into account
from static source code analysis.
Markus Elfring (17):
Use kcalloc() in mlx4_ib_alloc_pv_bufs()
Use kmalloc_array() in six functions
Improve size determinations in six functions
Fix a typo in a comment line
Delete four unnecessary return statements
Delete an unnecessary check before kfree() in free_pv_object()
Move an assignment out of a check in forward_trap()
Enclose 46 expressions for sizeof by parentheses
Split a condition check in six functions
Delete an unnecessary variable in __mlx4_ib_query_gid()
Delete an unnecessary variable initialisation in mlx4_ib_add()
Delete an unnecessary variable assignment in mlx4_ib_add()
Delete an error message for a failed memory allocation in mlx4_ib_add()
Delete unnecessary braces in mlx4_ib_add()
Delete unwanted spaces behind usages of the sizeof operator
Add spaces for better code readability
Less function calls in create_kernel_qp() after error detection
---
v2:
Changes were rebased on source files from Linux next-20170421.
Some of them were recombined as requested by Doug Ledford.
drivers/infiniband/hw/mlx4/mad.c | 72 ++++++++--------
drivers/infiniband/hw/mlx4/main.c | 164 ++++++++++++++++++++----------------
drivers/infiniband/hw/mlx4/qp.c | 173 +++++++++++++++++++-------------------
drivers/infiniband/hw/mlx5/qp.c | 66 ++++++++++-----
drivers/infiniband/hw/mlx5/srq.c | 5 +-
5 files changed, 261 insertions(+), 219 deletions(-)
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 12:28:35 +0200
Add a missing character in this description and adjust
the comment formatting.
Signed-off-by: Markus Elfring <[email protected]>
Reviewed-by: Majd Dibbiny <[email protected]>
---
drivers/infiniband/hw/mlx4/mad.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index ea4892b0f39e..d8c27e3ed69a 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1559,8 +1559,10 @@ static void mlx4_ib_multiplex_mad(struct mlx4_ib_demux_pv_ctx *ctx, struct ib_wc
}
}
- /* We are using standard ib_core services to send the mad, so generate a
- * stadard address handle by decoding the tunnelled mlx4_ah fields */
+ /*
+ * We are using standard ib_core services to send the mad, so generate
+ * a standard address handle by decoding the tunnelled mlx4_ah fields.
+ */
memcpy(&ah.av, &tunnel->hdr.av, sizeof (struct mlx4_av));
ah.ibah.device = ctx->ib_dev;
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 13:10:14 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: void function return statements are not generally useful
Thus remove such a statement in the affected functions.
Signed-off-by: Markus Elfring <[email protected]>
---
v2:
Changes were rebased on source files from Linux next-20170421.
These were recombined as requested by Doug Ledford.
drivers/infiniband/hw/mlx4/mad.c | 3 ---
drivers/infiniband/hw/mlx4/main.c | 1 -
2 files changed, 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index d8c27e3ed69a..cf33efce69d2 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1173,7 +1173,6 @@ static void handle_slaves_guid_change(struct mlx4_ib_dev *dev, u8 port_num,
out:
kfree(in_mad);
kfree(out_mad);
- return;
}
void handle_port_mgmt_change_event(struct work_struct *work)
@@ -2140,7 +2139,6 @@ void mlx4_ib_tunnels_update_work(struct work_struct *work)
mlx4_ib_tunnels_update(dmxw->dev, dmxw->slave, (int) dmxw->port,
dmxw->do_init);
kfree(dmxw);
- return;
}
static int mlx4_ib_alloc_demux_ctx(struct mlx4_ib_dev *dev,
@@ -2268,7 +2266,6 @@ static void mlx4_ib_master_tunnels(struct mlx4_ib_dev *dev, int do_init)
/* initialize or tear down tunnel QPs for the master */
for (i = 0; i < dev->dev->caps.num_ports; i++)
mlx4_ib_tunnels_update(dev, mlx4_master_func_num(dev->dev), i + 1, do_init);
- return;
}
int mlx4_ib_init_sriov(struct mlx4_ib_dev *dev)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 0ac670765466..232f556b1121 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -3109,7 +3109,6 @@ static void do_slave_init(struct mlx4_ib_dev *ibdev, int slave, int do_init)
}
out:
kfree(dm);
- return;
}
static void mlx4_ib_handle_catas_error(struct mlx4_ib_dev *ibdev)
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 11:33:43 +0200
Replace the specification of data types by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determinations a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring <[email protected]>
---
v2:
Changes were rebased on source files from Linux next-20170421.
These were recombined as requested by Doug Ledford.
drivers/infiniband/hw/mlx4/mad.c | 6 +++---
drivers/infiniband/hw/mlx4/main.c | 7 +++----
drivers/infiniband/hw/mlx4/qp.c | 4 ++--
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 61bd81baeb29..ea4892b0f39e 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1607,7 +1607,7 @@ static int mlx4_ib_alloc_pv_bufs(struct mlx4_ib_demux_pv_ctx *ctx,
return -ENOMEM;
tun_qp->tx_ring = kcalloc(MLX4_NUM_TUNNEL_BUFS,
- sizeof (struct mlx4_ib_tun_tx_buf),
+ sizeof(*tun_qp->tx_ring),
GFP_KERNEL);
if (!tun_qp->tx_ring) {
kfree(tun_qp->ring);
@@ -1948,7 +1948,7 @@ static int alloc_pv_object(struct mlx4_ib_dev *dev, int slave, int port,
struct mlx4_ib_demux_pv_ctx *ctx;
*ret_ctx = NULL;
- ctx = kzalloc(sizeof (struct mlx4_ib_demux_pv_ctx), GFP_KERNEL);
+ ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
if (!ctx)
return -ENOMEM;
@@ -2150,7 +2150,7 @@ static int mlx4_ib_alloc_demux_ctx(struct mlx4_ib_dev *dev,
int i;
ctx->tun = kcalloc(dev->dev->caps.sqp_demux,
- sizeof (struct mlx4_ib_demux_pv_ctx *), GFP_KERNEL);
+ sizeof(*ctx->tun), GFP_KERNEL);
if (!ctx->tun)
return -ENOMEM;
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 30a70fa95fec..0ac670765466 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2804,9 +2804,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
}
if (mlx4_is_bonded(dev))
for (i = 1; i < ibdev->num_ports ; ++i) {
- new_counter_index =
- kmalloc(sizeof(struct counter_index),
- GFP_KERNEL);
+ new_counter_index = kmalloc(sizeof(*new_counter_index),
+ GFP_KERNEL);
if (!new_counter_index)
goto err_counter;
new_counter_index->index = counter_index;
@@ -3085,7 +3084,7 @@ static void do_slave_init(struct mlx4_ib_dev *ibdev, int slave, int do_init)
return;
for (i = 0; i < ports; i++) {
- dm[i] = kmalloc(sizeof (struct mlx4_ib_demux_work), GFP_ATOMIC);
+ dm[i] = kmalloc(sizeof(*dm[i]), GFP_ATOMIC);
if (!dm[i]) {
while (--i >= 0)
kfree(dm[i]);
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 1d9d949c66d0..9269cd503443 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -691,14 +691,14 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
if (qp_type == MLX4_IB_QPT_SMI || qp_type == MLX4_IB_QPT_GSI ||
(qp_type & (MLX4_IB_QPT_PROXY_SMI | MLX4_IB_QPT_PROXY_SMI_OWNER |
MLX4_IB_QPT_PROXY_GSI | MLX4_IB_QPT_TUN_SMI_OWNER))) {
- sqp = kzalloc(sizeof (struct mlx4_ib_sqp), gfp);
+ sqp = kzalloc(sizeof(*sqp), gfp);
if (!sqp)
return -ENOMEM;
qp = &sqp->qp;
qp->pri.vid = 0xFFFF;
qp->alt.vid = 0xFFFF;
} else {
- qp = kzalloc(sizeof (struct mlx4_ib_qp), gfp);
+ qp = kzalloc(sizeof(*qp), gfp);
if (!qp)
return -ENOMEM;
qp->pri.vid = 0xFFFF;
--
2.12.2
On Fri, 2017-04-21 at 20:17 +0200, SF Markus Elfring wrote:
> Several update suggestions were taken into account
> from static source code analysis.
Hello Markus,
Patches should either be useful to users of the Linux kernel, e.g. by adding
new functionality or by fixing a bug, or to kernel developers, e.g. by making
their workflow easier. I don't think this patch series falls in either
category so please drop these patches.
Thanks,
Bart.
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 13:50:38 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: kfree(NULL) is safe and this check is probably not required
Thus fix the affected source code place.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx4/mad.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index cf33efce69d2..3caf468ca133 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1962,10 +1962,8 @@ static int alloc_pv_object(struct mlx4_ib_dev *dev, int slave, int port,
static void free_pv_object(struct mlx4_ib_dev *dev, int slave, int port)
{
- if (dev->sriov.demux[port - 1].tun[slave]) {
- kfree(dev->sriov.demux[port - 1].tun[slave]);
- dev->sriov.demux[port - 1].tun[slave] = NULL;
- }
+ kfree(dev->sriov.demux[port - 1].tun[slave]);
+ dev->sriov.demux[port - 1].tun[slave] = NULL;
}
static int create_pv_resources(struct ib_device *ibdev, int slave, int port,
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 14:00:08 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix the affected source code place.
Signed-off-by: Markus Elfring <[email protected]>
Reviewed-by: Majd Dibbiny <[email protected]>
---
drivers/infiniband/hw/mlx4/mad.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 3caf468ca133..0a5b46791335 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -401,7 +401,8 @@ static void forward_trap(struct mlx4_ib_dev *dev, u8 port_num, const struct ib_m
*/
spin_lock_irqsave(&dev->sm_lock, flags);
memcpy(send_buf->mad, mad, sizeof *mad);
- if ((send_buf->ah = dev->sm_ah[port_num - 1]))
+ send_buf->ah = dev->sm_ah[port_num - 1];
+ if (send_buf->ah)
ret = ib_post_send_mad(send_buf, NULL);
else
ret = -EINVAL;
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 15:27:55 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script "checkpatch.pl" pointed information out like the following.
WARNING: sizeof … should be sizeof(…)
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <[email protected]>
---
v2:
Changes were rebased on source files from Linux next-20170421.
These were recombined as requested by Doug Ledford.
drivers/infiniband/hw/mlx4/mad.c | 30 +++++++++++++++---------------
drivers/infiniband/hw/mlx4/main.c | 35 +++++++++++++++++------------------
drivers/infiniband/hw/mlx4/qp.c | 31 ++++++++++++++++---------------
3 files changed, 48 insertions(+), 48 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 0a5b46791335..41461aeb7c5e 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -195,7 +195,7 @@ static void update_sm_ah(struct mlx4_ib_dev *dev, u8 port_num, u16 lid, u8 sl)
if (!dev->send_agent[port_num - 1][0])
return;
- memset(&ah_attr, 0, sizeof ah_attr);
+ memset(&ah_attr, 0, sizeof(ah_attr));
ah_attr.dlid = lid;
ah_attr.sl = sl;
ah_attr.port_num = port_num;
@@ -400,7 +400,7 @@ static void forward_trap(struct mlx4_ib_dev *dev, u8 port_num, const struct ib_m
* it's OK for our devices).
*/
spin_lock_irqsave(&dev->sm_lock, flags);
- memcpy(send_buf->mad, mad, sizeof *mad);
+ memcpy(send_buf->mad, mad, sizeof(*mad));
send_buf->ah = dev->sm_ah[port_num - 1];
if (send_buf->ah)
ret = ib_post_send_mad(send_buf, NULL);
@@ -555,7 +555,7 @@ int mlx4_ib_send_to_slave(struct mlx4_ib_dev *dev, int slave, u8 port,
/* create ah. Just need an empty one with the port num for the post send.
* The driver will set the force loopback bit in post_send */
- memset(&attr, 0, sizeof attr);
+ memset(&attr, 0, sizeof(attr));
attr.port_num = port;
if (is_eth) {
union ib_gid sgid;
@@ -590,8 +590,8 @@ int mlx4_ib_send_to_slave(struct mlx4_ib_dev *dev, int slave, u8 port,
/* copy over to tunnel buffer */
if (grh)
- memcpy(&tun_mad->grh, grh, sizeof *grh);
- memcpy(&tun_mad->mad, mad, sizeof *mad);
+ memcpy(&tun_mad->grh, grh, sizeof(*grh));
+ memcpy(&tun_mad->mad, mad, sizeof(*mad));
/* adjust tunnel data */
tun_mad->hdr.pkey_index = cpu_to_be16(tun_pkey_ix);
@@ -961,7 +961,7 @@ static int iboe_process_mad(struct ib_device *ibdev, int mad_flags, u8 port_num,
}
mutex_unlock(&dev->counters_table[port_num - 1].mutex);
if (stats_avail) {
- memset(out_mad->data, 0, sizeof out_mad->data);
+ memset(out_mad->data, 0, sizeof(out_mad->data));
switch (counter_stats.counter_mode & 0xf) {
case 0:
edit_counter(&counter_stats,
@@ -1136,8 +1136,8 @@ static void handle_slaves_guid_change(struct mlx4_ib_dev *dev, u8 port_num,
if (!mlx4_is_mfunc(dev->dev) || !mlx4_is_master(dev->dev))
return;
- in_mad = kmalloc(sizeof *in_mad, GFP_KERNEL);
- out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);
+ in_mad = kmalloc(sizeof(*in_mad), GFP_KERNEL);
+ out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
if (!in_mad || !out_mad)
goto out;
@@ -1146,8 +1146,8 @@ static void handle_slaves_guid_change(struct mlx4_ib_dev *dev, u8 port_num,
for (i = 0; i < 4; i++) {
if (change_bitmap && (!((change_bitmap >> (8 * i)) & 0xff)))
continue;
- memset(in_mad, 0, sizeof *in_mad);
- memset(out_mad, 0, sizeof *out_mad);
+ memset(in_mad, 0, sizeof(*in_mad));
+ memset(out_mad, 0, sizeof(*out_mad));
in_mad->base_version = 1;
in_mad->mgmt_class = IB_MGMT_CLASS_SUBN_LID_ROUTED;
@@ -1417,7 +1417,7 @@ int mlx4_ib_send_to_wire(struct mlx4_ib_dev *dev, int slave, u8 port,
sizeof (struct mlx4_mad_snd_buf),
DMA_TO_DEVICE);
- memcpy(&sqp_mad->payload, mad, sizeof *mad);
+ memcpy(&sqp_mad->payload, mad, sizeof(*mad));
ib_dma_sync_single_for_device(&dev->ib_dev,
sqp->tx_ring[wire_tx_ix].buf.map,
@@ -1800,7 +1800,7 @@ static int create_pv_sqp(struct mlx4_ib_demux_pv_ctx *ctx,
tun_qp = &ctx->qp[qp_type];
- memset(&qp_init_attr, 0, sizeof qp_init_attr);
+ memset(&qp_init_attr, 0, sizeof(qp_init_attr));
qp_init_attr.init_attr.send_cq = ctx->cq;
qp_init_attr.init_attr.recv_cq = ctx->cq;
qp_init_attr.init_attr.sq_sig_type = IB_SIGNAL_ALL_WR;
@@ -1833,7 +1833,7 @@ static int create_pv_sqp(struct mlx4_ib_demux_pv_ctx *ctx,
return ret;
}
- memset(&attr, 0, sizeof attr);
+ memset(&attr, 0, sizeof(attr));
attr.qp_state = IB_QPS_INIT;
ret = 0;
if (create_tun)
@@ -2180,7 +2180,7 @@ static int mlx4_ib_alloc_demux_ctx(struct mlx4_ib_dev *dev,
goto err_mcg;
}
- snprintf(name, sizeof name, "mlx4_ibt%d", port);
+ snprintf(name, sizeof(name), "mlx4_ibt%d", port);
ctx->wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
if (!ctx->wq) {
pr_err("Failed to create tunnelling WQ for port %d\n", port);
@@ -2188,7 +2188,7 @@ static int mlx4_ib_alloc_demux_ctx(struct mlx4_ib_dev *dev,
goto err_wq;
}
- snprintf(name, sizeof name, "mlx4_ibud%d", port);
+ snprintf(name, sizeof(name), "mlx4_ibud%d", port);
ctx->ud_wq = alloc_ordered_workqueue(name, WQ_MEM_RECLAIM);
if (!ctx->ud_wq) {
pr_err("Failed to create up/down WQ for port %d\n", port);
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 232f556b1121..8f0e37c4f381 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -460,8 +460,8 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
resp.response_length = offsetof(typeof(resp), response_length) +
sizeof(resp.response_length);
- in_mad = kzalloc(sizeof *in_mad, GFP_KERNEL);
- out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);
+ in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
err = -ENOMEM;
if (!in_mad || !out_mad)
goto out;
@@ -474,8 +474,7 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
if (err)
goto out;
- memset(props, 0, sizeof *props);
-
+ memset(props, 0, sizeof(*props));
have_ib_ports = num_ib_ports(dev->dev);
props->fw_ver = dev->dev->caps.fw_ver;
@@ -598,8 +597,8 @@ static int ib_link_query_port(struct ib_device *ibdev, u8 port,
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
int err = -ENOMEM;
- in_mad = kzalloc(sizeof *in_mad, GFP_KERNEL);
- out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);
+ in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
if (!in_mad || !out_mad)
goto out;
@@ -774,8 +773,8 @@ int __mlx4_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
int clear = 0;
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
- in_mad = kzalloc(sizeof *in_mad, GFP_KERNEL);
- out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);
+ in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
if (!in_mad || !out_mad)
goto out;
@@ -911,8 +910,8 @@ int __mlx4_ib_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
int err = -ENOMEM;
- in_mad = kzalloc(sizeof *in_mad, GFP_KERNEL);
- out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);
+ in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
if (!in_mad || !out_mad)
goto out;
@@ -1283,7 +1282,7 @@ static struct ib_pd *mlx4_ib_alloc_pd(struct ib_device *ibdev,
struct mlx4_ib_pd *pd;
int err;
- pd = kmalloc(sizeof *pd, GFP_KERNEL);
+ pd = kmalloc(sizeof(*pd), GFP_KERNEL);
if (!pd)
return ERR_PTR(-ENOMEM);
@@ -1322,7 +1321,7 @@ static struct ib_xrcd *mlx4_ib_alloc_xrcd(struct ib_device *ibdev,
if (!(to_mdev(ibdev)->dev->caps.flags & MLX4_DEV_CAP_FLAG_XRC))
return ERR_PTR(-ENOSYS);
- xrcd = kmalloc(sizeof *xrcd, GFP_KERNEL);
+ xrcd = kmalloc(sizeof(*xrcd), GFP_KERNEL);
if (!xrcd)
return ERR_PTR(-ENOMEM);
@@ -1370,7 +1369,7 @@ static int add_gid_entry(struct ib_qp *ibqp, union ib_gid *gid)
struct mlx4_ib_dev *mdev = to_mdev(ibqp->device);
struct mlx4_ib_gid_entry *ge;
- ge = kzalloc(sizeof *ge, GFP_KERNEL);
+ ge = kzalloc(sizeof(*ge), GFP_KERNEL);
if (!ge)
return -ENOMEM;
@@ -2092,8 +2091,8 @@ static int init_node_data(struct mlx4_ib_dev *dev)
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
int err = -ENOMEM;
- in_mad = kzalloc(sizeof *in_mad, GFP_KERNEL);
- out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);
+ in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
if (!in_mad || !out_mad)
goto out;
@@ -2603,7 +2602,7 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
if (num_ports == 0)
return NULL;
- ibdev = (struct mlx4_ib_dev *) ib_alloc_device(sizeof *ibdev);
+ ibdev = (struct mlx4_ib_dev *) ib_alloc_device(sizeof(*ibdev));
if (!ibdev) {
dev_err(&dev->persist->pdev->dev,
"Device struct alloc failed\n");
@@ -3302,12 +3301,12 @@ static void mlx4_ib_event(struct mlx4_dev *dev, void *ibdev_ptr,
break;
case MLX4_DEV_EVENT_PORT_MGMT_CHANGE:
- ew = kmalloc(sizeof *ew, GFP_ATOMIC);
+ ew = kmalloc(sizeof(*ew), GFP_ATOMIC);
if (!ew)
break;
INIT_WORK(&ew->work, handle_port_mgmt_change_event);
- memcpy(&ew->ib_eqe, eqe, sizeof *eqe);
+ memcpy(&ew->ib_eqe, eqe, sizeof(*eqe));
ew->ib_dev = ibdev;
/* need to queue only for port owner, which uses GEN_EQE */
if (mlx4_is_master(dev))
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 9269cd503443..c2a0f75d7d07 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -247,9 +247,10 @@ static void post_nop_wqe(struct mlx4_ib_qp *qp, int n, int size)
s = sizeof(struct mlx4_wqe_ctrl_seg);
if (qp->ibqp.qp_type == IB_QPT_UD) {
- struct mlx4_wqe_datagram_seg *dgram = wqe + sizeof *ctrl;
+ struct mlx4_wqe_datagram_seg *dgram = wqe + sizeof(*ctrl);
struct mlx4_av *av = (struct mlx4_av *)dgram->av;
- memset(dgram, 0, sizeof *dgram);
+
+ memset(dgram, 0, sizeof(*dgram));
av->port_pd = cpu_to_be32((qp->port << 24) | to_mpd(qp->ibqp.pd)->pdn);
s += sizeof(struct mlx4_wqe_datagram_seg);
}
@@ -257,7 +258,8 @@ static void post_nop_wqe(struct mlx4_ib_qp *qp, int n, int size)
/* Pad the remainder of the WQE with an inline data segment. */
if (size > s) {
inl = wqe + s;
- inl->byte_count = cpu_to_be32(1 << 31 | (size - s - sizeof *inl));
+ inl->byte_count = cpu_to_be32(1 << 31
+ | (size - s - sizeof(*inl)));
}
ctrl->srcrb_flags = 0;
ctrl->qpn_vlan.fence_size = size / 16;
@@ -726,7 +728,7 @@ static int create_qp_common(struct mlx4_ib_dev *dev, struct ib_pd *pd,
if (pd->uobject) {
struct mlx4_ib_create_qp ucmd;
- if (ib_copy_from_udata(&ucmd, udata, sizeof ucmd)) {
+ if (ib_copy_from_udata(&ucmd, udata, sizeof(ucmd))) {
err = -EFAULT;
goto err;
}
@@ -1179,7 +1181,7 @@ static struct ib_qp *_mlx4_ib_create_qp(struct ib_pd *pd,
case IB_QPT_RC:
case IB_QPT_UC:
case IB_QPT_RAW_PACKET:
- qp = kzalloc(sizeof *qp, gfp);
+ qp = kzalloc(sizeof(*qp), gfp);
if (!qp)
return ERR_PTR(-ENOMEM);
qp->pri.vid = 0xFFFF;
@@ -1634,7 +1636,7 @@ static int __mlx4_ib_modify_qp(struct ib_qp *ibqp,
IB_LINK_LAYER_ETHERNET)
return -ENOTSUPP;
- context = kzalloc(sizeof *context, GFP_KERNEL);
+ context = kzalloc(sizeof(*context), GFP_KERNEL);
if (!context)
return -ENOMEM;
@@ -2298,7 +2300,7 @@ static int build_sriov_qp0_header(struct mlx4_ib_sqp *sqp,
struct mlx4_ib_dev *mdev = to_mdev(sqp->qp.ibqp.device);
struct ib_device *ib_dev = &mdev->ib_dev;
struct mlx4_wqe_mlx_seg *mlx = wqe;
- struct mlx4_wqe_inline_seg *inl = wqe + sizeof *mlx;
+ struct mlx4_wqe_inline_seg *inl = wqe + sizeof(*mlx);
struct mlx4_ib_ah *ah = to_mah(wr->ah);
u16 pkey;
u32 qkey;
@@ -2447,7 +2449,7 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_ud_wr *wr,
struct mlx4_ib_dev *ibdev = to_mdev(ib_dev);
struct mlx4_wqe_mlx_seg *mlx = wqe;
struct mlx4_wqe_ctrl_seg *ctrl = wqe;
- struct mlx4_wqe_inline_seg *inl = wqe + sizeof *mlx;
+ struct mlx4_wqe_inline_seg *inl = wqe + sizeof(*mlx);
struct mlx4_ib_ah *ah = to_mah(wr->ah);
union ib_gid sgid;
u16 pkey;
@@ -2820,7 +2822,7 @@ static void build_tunnel_header(struct ib_ud_wr *wr, void *wqe, unsigned *mlx_se
int spc;
int i;
- memcpy(&hdr.av, &ah->av, sizeof hdr.av);
+ memcpy(&hdr.av, &ah->av, sizeof(hdr.av));
hdr.remote_qpn = cpu_to_be32(wr->remote_qpn);
hdr.pkey_index = cpu_to_be16(wr->pkey_index);
hdr.qkey = cpu_to_be32(wr->remote_qkey);
@@ -2899,7 +2901,7 @@ static int build_lso_seg(struct mlx4_wqe_lso_seg *wqe, struct ib_ud_wr *wr,
struct mlx4_ib_qp *qp, unsigned *lso_seg_len,
__be32 *lso_hdr_sz, __be32 *blh)
{
- unsigned halign = ALIGN(sizeof *wqe + wr->hlen, 16);
+ unsigned int halign = ALIGN(sizeof(*wqe) + wr->hlen, 16);
if (unlikely(halign > MLX4_IB_CACHE_LINE_SIZE))
*blh = cpu_to_be32(1 << 6);
@@ -3017,9 +3019,8 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
qp->sq_signal_bits;
ctrl->imm = send_ieth(wr);
-
- wqe += sizeof *ctrl;
- size = sizeof *ctrl / 16;
+ wqe += sizeof(*ctrl);
+ size = sizeof(*ctrl) / 16;
switch (qp->mlx4_ib_qp_type) {
case MLX4_IB_QPT_RC:
@@ -3400,7 +3401,7 @@ static void to_ib_ah_attr(struct mlx4_ib_dev *ibdev, struct ib_ah_attr *ib_ah_at
struct mlx4_dev *dev = ibdev->dev;
int is_eth;
- memset(ib_ah_attr, 0, sizeof *ib_ah_attr);
+ memset(ib_ah_attr, 0, sizeof(*ib_ah_attr));
ib_ah_attr->port_num = path->sched_queue & 0x40 ? 2 : 1;
if (ib_ah_attr->port_num == 0 || ib_ah_attr->port_num > dev->caps.num_ports)
@@ -3426,7 +3427,7 @@ static void to_ib_ah_attr(struct mlx4_ib_dev *ibdev, struct ib_ah_attr *ib_ah_at
ib_ah_attr->grh.flow_label =
be32_to_cpu(path->tclass_flowlabel) & 0xfffff;
memcpy(ib_ah_attr->grh.dgid.raw,
- path->rgid, sizeof ib_ah_attr->grh.dgid.raw);
+ path->rgid, sizeof(ib_ah_attr->grh.dgid.raw));
}
}
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 18:13:16 +0200
The kfree() function was called in up to two cases during error handling
even if the passed variable contained a null pointer.
* Split a condition check for memory allocation failures.
* Adjust jump targets according to the Linux coding style convention.
* Delete initialisations for variables at the beginning
which became unnecessary with this refactoring.
Signed-off-by: Markus Elfring <[email protected]>
---
v2:
Changes were rebased on source files from Linux next-20170421.
These were recombined as requested by Doug Ledford.
drivers/infiniband/hw/mlx4/mad.c | 14 +++---
drivers/infiniband/hw/mlx4/main.c | 90 +++++++++++++++++++++++++--------------
2 files changed, 68 insertions(+), 36 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 41461aeb7c5e..b26817f0669f 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -1129,17 +1129,20 @@ static void propagate_pkey_ev(struct mlx4_ib_dev *dev, int port_num,
static void handle_slaves_guid_change(struct mlx4_ib_dev *dev, u8 port_num,
u32 guid_tbl_blk_num, u32 change_bitmap)
{
- struct ib_smp *in_mad = NULL;
- struct ib_smp *out_mad = NULL;
+ struct ib_smp *in_mad;
+ struct ib_smp *out_mad;
u16 i;
if (!mlx4_is_mfunc(dev->dev) || !mlx4_is_master(dev->dev))
return;
in_mad = kmalloc(sizeof(*in_mad), GFP_KERNEL);
+ if (!in_mad)
+ return;
+
out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
- if (!in_mad || !out_mad)
- goto out;
+ if (!out_mad)
+ goto free_in_mad;
guid_tbl_blk_num *= 4;
@@ -1172,8 +1175,9 @@ static void handle_slaves_guid_change(struct mlx4_ib_dev *dev, u8 port_num,
}
out:
- kfree(in_mad);
kfree(out_mad);
+free_in_mad:
+ kfree(in_mad);
}
void handle_port_mgmt_change_event(struct work_struct *work)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 8f0e37c4f381..e18e46a68809 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -435,8 +435,8 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
struct ib_udata *uhw)
{
struct mlx4_ib_dev *dev = to_mdev(ibdev);
- struct ib_smp *in_mad = NULL;
- struct ib_smp *out_mad = NULL;
+ struct ib_smp *in_mad;
+ struct ib_smp *out_mad;
int err;
int have_ib_ports;
struct mlx4_uverbs_ex_query_device cmd;
@@ -461,10 +461,14 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
resp.response_length = offsetof(typeof(resp), response_length) +
sizeof(resp.response_length);
in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ if (!in_mad)
+ return -ENOMEM;
+
out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
- err = -ENOMEM;
- if (!in_mad || !out_mad)
- goto out;
+ if (!out_mad) {
+ err = -ENOMEM;
+ goto free_in_mad;
+ }
init_query_mad(in_mad);
in_mad->attr_id = IB_SMP_ATTR_NODE_INFO;
@@ -573,9 +577,9 @@ static int mlx4_ib_query_device(struct ib_device *ibdev,
goto out;
}
out:
- kfree(in_mad);
kfree(out_mad);
-
+free_in_mad:
+ kfree(in_mad);
return err;
}
@@ -591,16 +595,21 @@ mlx4_ib_port_link_layer(struct ib_device *device, u8 port_num)
static int ib_link_query_port(struct ib_device *ibdev, u8 port,
struct ib_port_attr *props, int netw_view)
{
- struct ib_smp *in_mad = NULL;
- struct ib_smp *out_mad = NULL;
+ struct ib_smp *in_mad;
+ struct ib_smp *out_mad;
int ext_active_speed;
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
- int err = -ENOMEM;
+ int err;
in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ if (!in_mad)
+ return -ENOMEM;
+
out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
- if (!in_mad || !out_mad)
- goto out;
+ if (!out_mad) {
+ err = -ENOMEM;
+ goto free_in_mad;
+ }
init_query_mad(in_mad);
in_mad->attr_id = IB_SMP_ATTR_PORT_INFO;
@@ -673,8 +682,9 @@ static int ib_link_query_port(struct ib_device *ibdev, u8 port,
props->active_speed = IB_SPEED_SDR;
out:
- kfree(in_mad);
kfree(out_mad);
+free_in_mad:
+ kfree(in_mad);
return err;
}
@@ -766,17 +776,22 @@ static int mlx4_ib_query_port(struct ib_device *ibdev, u8 port,
int __mlx4_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
union ib_gid *gid, int netw_view)
{
- struct ib_smp *in_mad = NULL;
- struct ib_smp *out_mad = NULL;
- int err = -ENOMEM;
+ struct ib_smp *in_mad;
+ struct ib_smp *out_mad;
+ int err;
struct mlx4_ib_dev *dev = to_mdev(ibdev);
int clear = 0;
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ if (!in_mad)
+ return -ENOMEM;
+
out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
- if (!in_mad || !out_mad)
- goto out;
+ if (!out_mad) {
+ err = -ENOMEM;
+ goto free_in_mad;
+ }
init_query_mad(in_mad);
in_mad->attr_id = IB_SMP_ATTR_PORT_INFO;
@@ -814,8 +829,9 @@ int __mlx4_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
out:
if (clear)
memset(gid->raw + 8, 0, 8);
- kfree(in_mad);
kfree(out_mad);
+free_in_mad:
+ kfree(in_mad);
return err;
}
@@ -905,15 +921,20 @@ static void mlx4_init_sl2vl_tbl(struct mlx4_ib_dev *mdev)
int __mlx4_ib_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
u16 *pkey, int netw_view)
{
- struct ib_smp *in_mad = NULL;
- struct ib_smp *out_mad = NULL;
+ struct ib_smp *in_mad;
+ struct ib_smp *out_mad;
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
- int err = -ENOMEM;
+ int err;
in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ if (!in_mad)
+ return -ENOMEM;
+
out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
- if (!in_mad || !out_mad)
- goto out;
+ if (!out_mad) {
+ err = -ENOMEM;
+ goto free_in_mad;
+ }
init_query_mad(in_mad);
in_mad->attr_id = IB_SMP_ATTR_PKEY_TABLE;
@@ -930,8 +951,9 @@ int __mlx4_ib_query_pkey(struct ib_device *ibdev, u8 port, u16 index,
*pkey = be16_to_cpu(((__be16 *) out_mad->data)[index % 32]);
out:
- kfree(in_mad);
kfree(out_mad);
+free_in_mad:
+ kfree(in_mad);
return err;
}
@@ -2086,15 +2108,20 @@ static int mlx4_ib_mcg_detach(struct ib_qp *ibqp, union ib_gid *gid, u16 lid)
static int init_node_data(struct mlx4_ib_dev *dev)
{
- struct ib_smp *in_mad = NULL;
- struct ib_smp *out_mad = NULL;
+ struct ib_smp *in_mad;
+ struct ib_smp *out_mad;
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
- int err = -ENOMEM;
+ int err;
in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
+ if (!in_mad)
+ return -ENOMEM;
+
out_mad = kmalloc(sizeof(*out_mad), GFP_KERNEL);
- if (!in_mad || !out_mad)
- goto out;
+ if (!out_mad) {
+ err = -ENOMEM;
+ goto free_in_mad;
+ }
init_query_mad(in_mad);
in_mad->attr_id = IB_SMP_ATTR_NODE_DESC;
@@ -2117,8 +2144,9 @@ static int init_node_data(struct mlx4_ib_dev *dev)
memcpy(&dev->ib_dev.node_guid, out_mad->data + 12, 8);
out:
- kfree(in_mad);
kfree(out_mad);
+free_in_mad:
+ kfree(in_mad);
return err;
}
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 18:22:42 +0200
* Call the function "memset" directly in an if branch without using
an intermediate variable.
* Delete the local variable "clear" which became unnecessary with
this refactoring.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx4/main.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index e18e46a68809..a54e2f04929b 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -780,7 +780,6 @@ int __mlx4_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
struct ib_smp *out_mad;
int err;
struct mlx4_ib_dev *dev = to_mdev(ibdev);
- int clear = 0;
int mad_ifc_flags = MLX4_MAD_IFC_IGNORE_KEYS;
in_mad = kzalloc(sizeof(*in_mad), GFP_KERNEL);
@@ -809,8 +808,8 @@ int __mlx4_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
if (mlx4_is_mfunc(dev->dev) && !netw_view) {
if (index) {
/* For any index > 0, return the null guid */
+ memset(gid->raw + 8, 0, 8);
err = 0;
- clear = 1;
goto out;
}
}
@@ -827,8 +826,6 @@ int __mlx4_ib_query_gid(struct ib_device *ibdev, u8 port, int index,
memcpy(gid->raw + 8, out_mad->data + (index % 8) * 8, 8);
out:
- if (clear)
- memset(gid->raw + 8, 0, 8);
kfree(out_mad);
free_in_mad:
kfree(in_mad);
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 18:44:15 +0200
The local variable "new_counter_index" will be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx4/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index a54e2f04929b..d08b9b154c97 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2615,7 +2615,7 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
int num_req_counters;
int allocated;
u32 counter_index;
- struct counter_index *new_counter_index = NULL;
+ struct counter_index *new_counter_index;
pr_info_once("%s", mlx4_ib_version);
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 18:52:03 +0200
Delete an assignment for the local variable "num_ports" at the beginning
because it was initialised with the same value.
Signed-off-by: Markus Elfring <[email protected]>
Reviewed-by: Majd Dibbiny <[email protected]>
---
drivers/infiniband/hw/mlx4/main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index d08b9b154c97..4eb2466530dc 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2618,8 +2618,6 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
struct counter_index *new_counter_index;
pr_info_once("%s", mlx4_ib_version);
-
- num_ports = 0;
mlx4_foreach_ib_transport_port(i, dev)
num_ports++;
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 18:55:35 +0200
Omit an extra message for a memory allocation failure in this function.
Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx4/main.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 4eb2466530dc..67368dea9b6a 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2626,11 +2626,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
return NULL;
ibdev = (struct mlx4_ib_dev *) ib_alloc_device(sizeof(*ibdev));
- if (!ibdev) {
- dev_err(&dev->persist->pdev->dev,
- "Device struct alloc failed\n");
+ if (!ibdev)
return NULL;
- }
iboe = &ibdev->iboe;
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 18:58:43 +0200
Do not use curly brackets at one source code place
where a single statement should be sufficient.
Signed-off-by: Markus Elfring <[email protected]>
Reviewed-by: Majd Dibbiny <[email protected]>
---
drivers/infiniband/hw/mlx4/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 67368dea9b6a..7dd5468cd4b8 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2901,9 +2901,8 @@ static void *mlx4_ib_add(struct mlx4_dev *dev)
}
if (dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_ROCE_V1_V2) {
err = mlx4_config_roce_v2_port(dev, ROCE_V2_UDP_DPORT);
- if (err) {
+ if (err)
goto err_notif;
- }
}
}
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 19:11:59 +0200
* Replace the source code "sizeof (" by "sizeof("
according to the Linux coding style convention.
* Adjust indentation at a few places.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx4/qp.c | 128 ++++++++++++++++++++--------------------
1 file changed, 63 insertions(+), 65 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index c2a0f75d7d07..068abfdc9d01 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -342,39 +342,39 @@ static int send_wqe_overhead(enum mlx4_ib_qp_type type, u32 flags)
*/
switch (type) {
case MLX4_IB_QPT_UD:
- return sizeof (struct mlx4_wqe_ctrl_seg) +
- sizeof (struct mlx4_wqe_datagram_seg) +
+ return sizeof(struct mlx4_wqe_ctrl_seg) +
+ sizeof(struct mlx4_wqe_datagram_seg) +
((flags & MLX4_IB_QP_LSO) ? MLX4_IB_LSO_HEADER_SPARE : 0);
case MLX4_IB_QPT_PROXY_SMI_OWNER:
case MLX4_IB_QPT_PROXY_SMI:
case MLX4_IB_QPT_PROXY_GSI:
- return sizeof (struct mlx4_wqe_ctrl_seg) +
- sizeof (struct mlx4_wqe_datagram_seg) + 64;
+ return sizeof(struct mlx4_wqe_ctrl_seg) +
+ sizeof(struct mlx4_wqe_datagram_seg) + 64;
case MLX4_IB_QPT_TUN_SMI_OWNER:
case MLX4_IB_QPT_TUN_GSI:
- return sizeof (struct mlx4_wqe_ctrl_seg) +
- sizeof (struct mlx4_wqe_datagram_seg);
+ return sizeof(struct mlx4_wqe_ctrl_seg) +
+ sizeof(struct mlx4_wqe_datagram_seg);
case MLX4_IB_QPT_UC:
- return sizeof (struct mlx4_wqe_ctrl_seg) +
- sizeof (struct mlx4_wqe_raddr_seg);
+ return sizeof(struct mlx4_wqe_ctrl_seg) +
+ sizeof(struct mlx4_wqe_raddr_seg);
case MLX4_IB_QPT_RC:
- return sizeof (struct mlx4_wqe_ctrl_seg) +
- sizeof (struct mlx4_wqe_masked_atomic_seg) +
- sizeof (struct mlx4_wqe_raddr_seg);
+ return sizeof(struct mlx4_wqe_ctrl_seg) +
+ sizeof(struct mlx4_wqe_masked_atomic_seg) +
+ sizeof(struct mlx4_wqe_raddr_seg);
case MLX4_IB_QPT_SMI:
case MLX4_IB_QPT_GSI:
- return sizeof (struct mlx4_wqe_ctrl_seg) +
+ return sizeof(struct mlx4_wqe_ctrl_seg) +
ALIGN(MLX4_IB_UD_HEADER_SIZE +
DIV_ROUND_UP(MLX4_IB_UD_HEADER_SIZE,
MLX4_INLINE_ALIGN) *
- sizeof (struct mlx4_wqe_inline_seg),
- sizeof (struct mlx4_wqe_data_seg)) +
+ sizeof(struct mlx4_wqe_inline_seg),
+ sizeof(struct mlx4_wqe_data_seg)) +
ALIGN(4 +
- sizeof (struct mlx4_wqe_inline_seg),
- sizeof (struct mlx4_wqe_data_seg));
+ sizeof(struct mlx4_wqe_inline_seg),
+ sizeof(struct mlx4_wqe_data_seg));
default:
- return sizeof (struct mlx4_wqe_ctrl_seg);
+ return sizeof(struct mlx4_wqe_ctrl_seg);
}
}
@@ -398,7 +398,8 @@ static int set_rq_size(struct mlx4_ib_dev *dev, struct ib_qp_cap *cap,
qp->rq.wqe_cnt = roundup_pow_of_two(max(1U, cap->max_recv_wr));
qp->rq.max_gs = roundup_pow_of_two(max(1U, cap->max_recv_sge));
- qp->rq.wqe_shift = ilog2(qp->rq.max_gs * sizeof (struct mlx4_wqe_data_seg));
+ qp->rq.wqe_shift = ilog2(qp->rq.max_gs
+ * sizeof(struct mlx4_wqe_data_seg));
}
/* leave userspace return values as they were, so as not to break ABI */
@@ -426,7 +427,7 @@ static int set_kernel_sq_size(struct mlx4_ib_dev *dev, struct ib_qp_cap *cap,
if (cap->max_send_wr > (dev->dev->caps.max_wqes - MLX4_IB_SQ_MAX_SPARE) ||
cap->max_send_sge > min(dev->dev->caps.max_sq_sg, dev->dev->caps.max_rq_sg) ||
cap->max_inline_data + send_wqe_overhead(type, qp->flags) +
- sizeof (struct mlx4_wqe_inline_seg) > dev->dev->caps.max_sq_desc_sz)
+ sizeof(struct mlx4_wqe_inline_seg) > dev->dev->caps.max_sq_desc_sz)
return -EINVAL;
/*
@@ -438,8 +439,8 @@ static int set_kernel_sq_size(struct mlx4_ib_dev *dev, struct ib_qp_cap *cap,
cap->max_send_sge + 2 > dev->dev->caps.max_sq_sg)
return -EINVAL;
- s = max(cap->max_send_sge * sizeof (struct mlx4_wqe_data_seg),
- cap->max_inline_data + sizeof (struct mlx4_wqe_inline_seg)) +
+ s = max(cap->max_send_sge * sizeof(struct mlx4_wqe_data_seg),
+ cap->max_inline_data + sizeof(struct mlx4_wqe_inline_seg)) +
send_wqe_overhead(type, qp->flags);
if (s > dev->dev->caps.max_sq_desc_sz)
@@ -509,7 +510,7 @@ static int set_kernel_sq_size(struct mlx4_ib_dev *dev, struct ib_qp_cap *cap,
qp->sq.max_gs = (min(dev->dev->caps.max_sq_desc_sz,
(qp->sq_max_wqes_per_wr << qp->sq.wqe_shift)) -
send_wqe_overhead(type, qp->flags)) /
- sizeof (struct mlx4_wqe_data_seg);
+ sizeof(struct mlx4_wqe_data_seg);
qp->buf_size = (qp->rq.wqe_cnt << qp->rq.wqe_shift) +
(qp->sq.wqe_cnt << qp->sq.wqe_shift);
@@ -563,13 +564,13 @@ static int alloc_proxy_bufs(struct ib_device *dev, struct mlx4_ib_qp *qp)
return -ENOMEM;
for (i = 0; i < qp->rq.wqe_cnt; i++) {
qp->sqp_proxy_rcv[i].addr =
- kmalloc(sizeof (struct mlx4_ib_proxy_sqp_hdr),
+ kmalloc(sizeof(struct mlx4_ib_proxy_sqp_hdr),
GFP_KERNEL);
if (!qp->sqp_proxy_rcv[i].addr)
goto err;
qp->sqp_proxy_rcv[i].map =
ib_dma_map_single(dev, qp->sqp_proxy_rcv[i].addr,
- sizeof (struct mlx4_ib_proxy_sqp_hdr),
+ sizeof(struct mlx4_ib_proxy_sqp_hdr),
DMA_FROM_DEVICE);
if (ib_dma_mapping_error(dev, qp->sqp_proxy_rcv[i].map)) {
kfree(qp->sqp_proxy_rcv[i].addr);
@@ -582,7 +583,7 @@ static int alloc_proxy_bufs(struct ib_device *dev, struct mlx4_ib_qp *qp)
while (i > 0) {
--i;
ib_dma_unmap_single(dev, qp->sqp_proxy_rcv[i].map,
- sizeof (struct mlx4_ib_proxy_sqp_hdr),
+ sizeof(struct mlx4_ib_proxy_sqp_hdr),
DMA_FROM_DEVICE);
kfree(qp->sqp_proxy_rcv[i].addr);
}
@@ -597,7 +598,7 @@ static void free_proxy_bufs(struct ib_device *dev, struct mlx4_ib_qp *qp)
for (i = 0; i < qp->rq.wqe_cnt; i++) {
ib_dma_unmap_single(dev, qp->sqp_proxy_rcv[i].map,
- sizeof (struct mlx4_ib_proxy_sqp_hdr),
+ sizeof(struct mlx4_ib_proxy_sqp_hdr),
DMA_FROM_DEVICE);
kfree(qp->sqp_proxy_rcv[i].addr);
}
@@ -2320,7 +2321,7 @@ static int build_sriov_qp0_header(struct mlx4_ib_sqp *sqp,
/* for proxy-qp0 sends, need to add in size of tunnel header */
/* for tunnel-qp0 sends, tunnel header is already in s/g list */
if (sqp->qp.mlx4_ib_qp_type == MLX4_IB_QPT_PROXY_SMI_OWNER)
- send_size += sizeof (struct mlx4_ib_tunnel_header);
+ send_size += sizeof(struct mlx4_ib_tunnel_header);
ib_ud_header_init(send_size, 1, 0, 0, 0, 0, 0, 0, &sqp->ud_header);
@@ -2402,7 +2403,7 @@ static int build_sriov_qp0_header(struct mlx4_ib_sqp *sqp,
}
*mlx_seg_len =
- ALIGN(i * sizeof (struct mlx4_wqe_inline_seg) + header_size, 16);
+ ALIGN(i * sizeof(struct mlx4_wqe_inline_seg) + header_size, 16);
return 0;
}
@@ -2692,7 +2693,7 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_ud_wr *wr,
}
*mlx_seg_len =
- ALIGN(i * sizeof (struct mlx4_wqe_inline_seg) + header_size, 16);
+ ALIGN(i * sizeof(struct mlx4_wqe_inline_seg) + header_size, 16);
return 0;
}
@@ -2783,7 +2784,7 @@ static void set_masked_atomic_seg(struct mlx4_wqe_masked_atomic_seg *aseg,
static void set_datagram_seg(struct mlx4_wqe_datagram_seg *dseg,
struct ib_ud_wr *wr)
{
- memcpy(dseg->av, &to_mah(wr->ah)->av, sizeof (struct mlx4_av));
+ memcpy(dseg->av, &to_mah(wr->ah)->av, sizeof(struct mlx4_av));
dseg->dqpn = cpu_to_be32(wr->remote_qpn);
dseg->qkey = cpu_to_be32(wr->remote_qkey);
dseg->vlan = to_mah(wr->ah)->av.eth.vlan;
@@ -2805,7 +2806,7 @@ static void set_tunnel_datagram_seg(struct mlx4_ib_dev *dev,
sqp_av.sl_tclass_flowlabel = av->ib.sl_tclass_flowlabel &
cpu_to_be32(0xf0000000);
- memcpy(dseg->av, &sqp_av, sizeof (struct mlx4_av));
+ memcpy(dseg->av, &sqp_av, sizeof(struct mlx4_av));
if (qpt == MLX4_IB_QPT_PROXY_GSI)
dseg->dqpn = cpu_to_be32(dev->dev->caps.qp1_tunnel[port - 1]);
else
@@ -2831,10 +2832,10 @@ static void build_tunnel_header(struct ib_ud_wr *wr, void *wqe, unsigned *mlx_se
spc = MLX4_INLINE_ALIGN -
((unsigned long) (inl + 1) & (MLX4_INLINE_ALIGN - 1));
- if (sizeof (hdr) <= spc) {
- memcpy(inl + 1, &hdr, sizeof (hdr));
+ if (sizeof(hdr) <= spc) {
+ memcpy(inl + 1, &hdr, sizeof(hdr));
wmb();
- inl->byte_count = cpu_to_be32(1 << 31 | sizeof (hdr));
+ inl->byte_count = cpu_to_be32(1 << 31 | sizeof(hdr));
i = 1;
} else {
memcpy(inl + 1, &hdr, spc);
@@ -2842,14 +2843,14 @@ static void build_tunnel_header(struct ib_ud_wr *wr, void *wqe, unsigned *mlx_se
inl->byte_count = cpu_to_be32(1 << 31 | spc);
inl = (void *) (inl + 1) + spc;
- memcpy(inl + 1, (void *) &hdr + spc, sizeof (hdr) - spc);
+ memcpy(inl + 1, (void *) &hdr + spc, sizeof(hdr) - spc);
wmb();
- inl->byte_count = cpu_to_be32(1 << 31 | (sizeof (hdr) - spc));
+ inl->byte_count = cpu_to_be32(1 << 31 | (sizeof(hdr) - spc));
i = 2;
}
*mlx_seg_len =
- ALIGN(i * sizeof (struct mlx4_wqe_inline_seg) + sizeof (hdr), 16);
+ ALIGN(i * sizeof(struct mlx4_wqe_inline_seg) + sizeof(hdr), 16);
}
static void set_mlx_icrc_seg(void *dseg)
@@ -3031,27 +3032,23 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
case IB_WR_MASKED_ATOMIC_FETCH_AND_ADD:
set_raddr_seg(wqe, atomic_wr(wr)->remote_addr,
atomic_wr(wr)->rkey);
- wqe += sizeof (struct mlx4_wqe_raddr_seg);
-
+ wqe += sizeof(struct mlx4_wqe_raddr_seg);
set_atomic_seg(wqe, atomic_wr(wr));
- wqe += sizeof (struct mlx4_wqe_atomic_seg);
-
- size += (sizeof (struct mlx4_wqe_raddr_seg) +
- sizeof (struct mlx4_wqe_atomic_seg)) / 16;
-
+ wqe += sizeof(struct mlx4_wqe_atomic_seg);
+ size += (sizeof(struct mlx4_wqe_raddr_seg) +
+ sizeof(struct mlx4_wqe_atomic_seg))
+ / 16;
break;
case IB_WR_MASKED_ATOMIC_CMP_AND_SWP:
set_raddr_seg(wqe, atomic_wr(wr)->remote_addr,
atomic_wr(wr)->rkey);
- wqe += sizeof (struct mlx4_wqe_raddr_seg);
-
+ wqe += sizeof(struct mlx4_wqe_raddr_seg);
set_masked_atomic_seg(wqe, atomic_wr(wr));
- wqe += sizeof (struct mlx4_wqe_masked_atomic_seg);
-
- size += (sizeof (struct mlx4_wqe_raddr_seg) +
- sizeof (struct mlx4_wqe_masked_atomic_seg)) / 16;
-
+ wqe += sizeof(struct mlx4_wqe_masked_atomic_seg);
+ size += (sizeof(struct mlx4_wqe_raddr_seg) +
+ sizeof(struct mlx4_wqe_masked_atomic_seg))
+ / 16;
break;
case IB_WR_RDMA_READ:
@@ -3059,16 +3056,17 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
case IB_WR_RDMA_WRITE_WITH_IMM:
set_raddr_seg(wqe, rdma_wr(wr)->remote_addr,
rdma_wr(wr)->rkey);
- wqe += sizeof (struct mlx4_wqe_raddr_seg);
- size += sizeof (struct mlx4_wqe_raddr_seg) / 16;
+ wqe += sizeof(struct mlx4_wqe_raddr_seg);
+ size += sizeof(struct mlx4_wqe_raddr_seg) / 16;
break;
case IB_WR_LOCAL_INV:
ctrl->srcrb_flags |=
cpu_to_be32(MLX4_WQE_CTRL_STRONG_ORDER);
set_local_inv_seg(wqe, wr->ex.invalidate_rkey);
- wqe += sizeof (struct mlx4_wqe_local_inval_seg);
- size += sizeof (struct mlx4_wqe_local_inval_seg) / 16;
+ wqe += sizeof(struct mlx4_wqe_local_inval_seg);
+ size += sizeof(struct mlx4_wqe_local_inval_seg)
+ / 16;
break;
case IB_WR_REG_MR:
@@ -3101,13 +3099,13 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
set_datagram_seg(wqe, ud_wr(wr));
/* set the forced-loopback bit in the data seg av */
*(__be32 *) wqe |= cpu_to_be32(0x80000000);
- wqe += sizeof (struct mlx4_wqe_datagram_seg);
- size += sizeof (struct mlx4_wqe_datagram_seg) / 16;
+ wqe += sizeof(struct mlx4_wqe_datagram_seg);
+ size += sizeof(struct mlx4_wqe_datagram_seg) / 16;
break;
case MLX4_IB_QPT_UD:
set_datagram_seg(wqe, ud_wr(wr));
- wqe += sizeof (struct mlx4_wqe_datagram_seg);
- size += sizeof (struct mlx4_wqe_datagram_seg) / 16;
+ wqe += sizeof(struct mlx4_wqe_datagram_seg);
+ size += sizeof(struct mlx4_wqe_datagram_seg) / 16;
if (wr->opcode == IB_WR_LSO) {
err = build_lso_seg(wqe, ud_wr(wr), qp, &seglen,
@@ -3148,8 +3146,8 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
set_tunnel_datagram_seg(to_mdev(ibqp->device), wqe,
ud_wr(wr),
qp->mlx4_ib_qp_type);
- wqe += sizeof (struct mlx4_wqe_datagram_seg);
- size += sizeof (struct mlx4_wqe_datagram_seg) / 16;
+ wqe += sizeof(struct mlx4_wqe_datagram_seg);
+ size += sizeof(struct mlx4_wqe_datagram_seg) / 16;
build_tunnel_header(ud_wr(wr), wqe, &seglen);
wqe += seglen;
size += seglen / 16;
@@ -3180,7 +3178,7 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
dseg = wqe;
dseg += wr->num_sge - 1;
- size += wr->num_sge * (sizeof (struct mlx4_wqe_data_seg) / 16);
+ size += wr->num_sge * (sizeof(struct mlx4_wqe_data_seg) / 16);
/* Add one more inline data segment for ICRC for MLX sends */
if (unlikely(qp->mlx4_ib_qp_type == MLX4_IB_QPT_SMI ||
@@ -3188,7 +3186,7 @@ int mlx4_ib_post_send(struct ib_qp *ibqp, struct ib_send_wr *wr,
qp->mlx4_ib_qp_type &
(MLX4_IB_QPT_PROXY_SMI_OWNER | MLX4_IB_QPT_TUN_SMI_OWNER))) {
set_mlx_icrc_seg(dseg + 1);
- size += sizeof (struct mlx4_wqe_data_seg) / 16;
+ size += sizeof(struct mlx4_wqe_data_seg) / 16;
}
for (i = wr->num_sge - 1; i >= 0; --i, --dseg)
@@ -3313,10 +3311,10 @@ int mlx4_ib_post_recv(struct ib_qp *ibqp, struct ib_recv_wr *wr,
MLX4_IB_QPT_PROXY_SMI | MLX4_IB_QPT_PROXY_GSI)) {
ib_dma_sync_single_for_device(ibqp->device,
qp->sqp_proxy_rcv[ind].map,
- sizeof (struct mlx4_ib_proxy_sqp_hdr),
+ sizeof(struct mlx4_ib_proxy_sqp_hdr),
DMA_FROM_DEVICE);
scat->byte_count =
- cpu_to_be32(sizeof (struct mlx4_ib_proxy_sqp_hdr));
+ cpu_to_be32(sizeof(struct mlx4_ib_proxy_sqp_hdr));
/* use dma lkey from upper layer entry */
scat->lkey = cpu_to_be32(wr->sg_list->lkey);
scat->addr = cpu_to_be64(qp->sqp_proxy_rcv[ind].map);
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 19:14:11 +0200
Use space characters at some source code places according to
the Linux coding style convention.
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx4/qp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
index 068abfdc9d01..15240163c399 100644
--- a/drivers/infiniband/hw/mlx4/qp.c
+++ b/drivers/infiniband/hw/mlx4/qp.c
@@ -1065,7 +1065,7 @@ static void destroy_qp_common(struct mlx4_ib_dev *dev, struct mlx4_ib_qp *qp,
list_del(&qp->cq_recv_list);
if (!is_user) {
__mlx4_ib_cq_clean(recv_cq, qp->mqp.qpn,
- qp->ibqp.srq ? to_msrq(qp->ibqp.srq): NULL);
+ qp->ibqp.srq ? to_msrq(qp->ibqp.srq) : NULL);
if (send_cq != recv_cq)
__mlx4_ib_cq_clean(send_cq, qp->mqp.qpn, NULL);
}
@@ -2599,7 +2599,7 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_ud_wr *wr,
u16 ether_type;
u16 pcp = (be32_to_cpu(ah->av.ib.sl_tclass_flowlabel) >> 29) << 13;
- ether_type = (!is_udp) ? ETH_P_IBOE:
+ ether_type = (!is_udp) ? ETH_P_IBOE :
(ip_version == 4 ? ETH_P_IP : ETH_P_IPV6);
mlx->sched_prio = cpu_to_be16(pcp);
--
2.12.2
From: Markus Elfring <[email protected]>
Date: Fri, 21 Apr 2017 19:19:20 +0200
The kfree() function was called in up to five cases
by the create_kernel_qp() function during error handling
even if the passed data structure member contained a null pointer.
* Adjust jump targets according to the Linux coding style convention.
* Split a condition check for memory allocation failures so that
each pointer from these function calls will be checked immediately.
See also background information:
Topic "CWE-754: Improper check for unusual or exceptional conditions"
Link: https://cwe.mitre.org/data/definitions/754.html
Signed-off-by: Markus Elfring <[email protected]>
---
drivers/infiniband/hw/mlx5/qp.c | 43 ++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/qp.c b/drivers/infiniband/hw/mlx5/qp.c
index 1e98a8c9fab8..c7bfa8ffaf0d 100644
--- a/drivers/infiniband/hw/mlx5/qp.c
+++ b/drivers/infiniband/hw/mlx5/qp.c
@@ -934,7 +934,7 @@ static int create_kernel_qp(struct mlx5_ib_dev *dev,
*in = mlx5_vzalloc(*inlen);
if (!*in) {
err = -ENOMEM;
- goto err_buf;
+ goto free_buffer;
}
qpc = MLX5_ADDR_OF(create_qp_in, *in, qpc);
@@ -956,45 +956,56 @@ static int create_kernel_qp(struct mlx5_ib_dev *dev,
err = mlx5_db_alloc(dev->mdev, &qp->db);
if (err) {
mlx5_ib_dbg(dev, "err %d\n", err);
- goto err_free;
+ goto vfree_in;
}
qp->sq.wrid = kmalloc_array(qp->sq.wqe_cnt,
sizeof(*qp->sq.wrid),
GFP_KERNEL);
+ if (!qp->sq.wrid)
+ goto free_db;
+
qp->sq.wr_data = kmalloc_array(qp->sq.wqe_cnt,
sizeof(*qp->sq.wr_data),
GFP_KERNEL);
+ if (!qp->sq.wr_data)
+ goto free_sq_wrid;
+
qp->rq.wrid = kmalloc_array(qp->rq.wqe_cnt,
sizeof(*qp->rq.wrid),
GFP_KERNEL);
+ if (!qp->rq.wrid)
+ goto free_sq_wr_data;
+
qp->sq.w_list = kmalloc_array(qp->sq.wqe_cnt,
sizeof(*qp->sq.w_list),
GFP_KERNEL);
+ if (!qp->sq.w_list)
+ goto free_rq_wrid;
+
qp->sq.wqe_head = kmalloc_array(qp->sq.wqe_cnt,
sizeof(*qp->sq.wqe_head),
GFP_KERNEL);
- if (!qp->sq.wrid || !qp->sq.wr_data || !qp->rq.wrid ||
- !qp->sq.w_list || !qp->sq.wqe_head) {
- err = -ENOMEM;
- goto err_wrid;
- }
+ if (!qp->sq.wqe_head)
+ goto free_sq_w_list;
+
qp->create_type = MLX5_QP_KERNEL;
return 0;
-
-err_wrid:
- kfree(qp->sq.wqe_head);
+free_sq_w_list:
kfree(qp->sq.w_list);
- kfree(qp->sq.wrid);
- kfree(qp->sq.wr_data);
+free_rq_wrid:
kfree(qp->rq.wrid);
+free_sq_wr_data:
+ kfree(qp->sq.wr_data);
+free_sq_wrid:
+ kfree(qp->sq.wrid);
+free_db:
mlx5_db_free(dev->mdev, &qp->db);
-
-err_free:
+ err = -ENOMEM;
+vfree_in:
kvfree(*in);
-
-err_buf:
+free_buffer:
mlx5_buf_free(dev->mdev, &qp->buf);
return err;
}
--
2.12.2
> Patches should either be useful to users of the Linux kernel, e.g. by adding
> new functionality or by fixing a bug, or to kernel developers, e.g. by making
> their workflow easier.
Such an expectation is generally fine.
> I don't think this patch series falls in either category
I find that my update suggestion touches some aspects for the desired
source code quality, doesn't it?
> so please drop these patches.
I hope that corresponding software improvements will be clarified further.
Regards,
Markus
On Fri, 2017-04-21 at 20:54 +0200, SF Markus Elfring wrote:
> Use space characters at some source code places according to
> the Linux coding style convention.
[]
> diff --git a/drivers/infiniband/hw/mlx4/qp.c b/drivers/infiniband/hw/mlx4/qp.c
[]
> @@ -2599,7 +2599,7 @@ static int build_mlx_header(struct mlx4_ib_sqp *sqp, struct ib_ud_wr *wr,
> u16 ether_type;
> u16 pcp = (be32_to_cpu(ah->av.ib.sl_tclass_flowlabel) >> 29) << 13;
>
> - ether_type = (!is_udp) ? ETH_P_IBOE:
> + ether_type = (!is_udp) ? ETH_P_IBOE :
> (ip_version == 4 ? ETH_P_IP : ETH_P_IPV6);
Please refrain from tool generated mechanical changes.
Do try to make the code human readable and correct when
you are also making actual object code differences.
If this is to be modified at all then also please use
consistent ?: condition testing parentheses or perhaps
don't use parentheses at all.
Maybe even align the ETH_P_ uses like:
ether_type = !is_udp ? ETH_P_IBOE :
ip_version == 4 ? ETH_P_IP :
ETH_P_IPV6;
On Fri, 2017-04-21 at 21:21 +0200, SF Markus Elfring wrote:
> > I don't think this patch series falls in either category
>
> I find that my update suggestion touches some aspects for the desired
> source code quality, doesn't it?
No.
Bart.
> Date: Fri, 21 Apr 2017 19:56:54 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (17):
> Use kcalloc() in mlx4_ib_alloc_pv_bufs()
> Use kmalloc_array() in six functions
> Improve size determinations in six functions
> Fix a typo in a comment line
> Delete four unnecessary return statements
> Delete an unnecessary check before kfree() in free_pv_object()
> Move an assignment out of a check in forward_trap()
> Enclose 46 expressions for sizeof by parentheses
> Split a condition check in six functions
> Delete an unnecessary variable in __mlx4_ib_query_gid()
> Delete an unnecessary variable initialisation in mlx4_ib_add()
> Delete an unnecessary variable assignment in mlx4_ib_add()
> Delete an error message for a failed memory allocation in mlx4_ib_add()
> Delete unnecessary braces in mlx4_ib_add()
> Delete unwanted spaces behind usages of the sizeof operator
> Add spaces for better code readability
> Less function calls in create_kernel_qp() after error detection
> ---
>
> v2:
> Changes were rebased on source files from Linux next-20170421.
> Some of them were recombined as requested by Doug Ledford.
>
> drivers/infiniband/hw/mlx4/mad.c | 72 ++++++++--------
> drivers/infiniband/hw/mlx4/main.c | 164 ++++++++++++++++++++----------------
> drivers/infiniband/hw/mlx4/qp.c | 173 +++++++++++++++++++-------------------
> drivers/infiniband/hw/mlx5/qp.c | 66 ++++++++++-----
> drivers/infiniband/hw/mlx5/srq.c | 5 +-
> 5 files changed, 261 insertions(+), 219 deletions(-)
Will the clarification be continued for the shown change possibilities?
Regards,
Markus