Skip to contentSkip to conversation

Sparse fixes for 4.12

PATCH In review Leon Romanovsky 3 participants 7 messages Apr 22, 2017 – Apr 28, 2017 2 messages not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Sparse fixes for 4.12

Hi Doug,

Please find below 4 patches, which fix 45 sparse warnings.

Patches #1 and #2 are trivial changes. The first adds static keyword to the functions declarations and second adds includes of the relevant headers.

Patch #3 refactors code to get rid of one temporal variable. That variable was needed to determine if QP is found. Once removed, it allowed to write the code with lock and unlock placed together.

Patch #4 fixes wrong type casting. The opcode is in CPU format and wqe_word is in le32 format.

This series is based on k.o/for-4.12-rdma-netdevice branch.

Thanks

Leon Romanovsky (4):
Ib/core: Mark local uverbs_std_types functions to be static Ib/usnic: Explicitly include usnic headers IB/usnic: Simplify the code to balance loc/unlock calls IB/nes: Fix incorrect type in assignment

 drivers/infiniband/core/uverbs_std_types.c   | 48 ++++++++++++++--------------
 drivers/infiniband/hw/nes/nes_hw.c           |  2 +-
 drivers/infiniband/hw/usnic/usnic_ib_sysfs.c |  1 +
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 46 +++++++++++++-------------
 4 files changed, 50 insertions(+), 47 deletions(-)

--
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170422142852.18882-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#2 lore raw

PATCH 1/4 Ib/core: Mark local uverbs_std_types functions to be static

Functions declared in uverbs_std_types.c are local to that file, but they lack static declarations. This produces a lot of sparse warnings, like the one below:

drivers/infiniband/core/uverbs_std_types.c:41:5: warning: symbol
'uverbs_free_ah' was not declared. Should it be static?

So mark them as static.

CC Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 drivers/infiniband/core/uverbs_std_types.c | 48 +++++++++++++++---------------
 1 file changed, 24 insertions(+), 24 deletions(-)
drivers/infiniband/core/uverbs_std_types.c +24 -24
index e3338b19d6a2..ad3caad40945 100644
@@ -38,26 +38,26 @@
 #include "rdma_core.h"
 #include "uverbs.h"
 
-int uverbs_free_ah(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_ah(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	return ib_destroy_ah((struct ib_ah *)uobject->object);
 }
 
-int uverbs_free_flow(struct ib_uobject *uobject,
-		     enum rdma_remove_reason why)
+static int uverbs_free_flow(struct ib_uobject *uobject,
+			    enum rdma_remove_reason why)
 {
 	return ib_destroy_flow((struct ib_flow *)uobject->object);
 }
 
-int uverbs_free_mw(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_mw(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	return uverbs_dealloc_mw((struct ib_mw *)uobject->object);
 }
 
-int uverbs_free_qp(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_qp(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	struct ib_qp *qp = uobject->object;
 	struct ib_uqp_object *uqp =
@@ -82,8 +82,8 @@ int uverbs_free_qp(struct ib_uobject *uobject,
 	return ret;
 }
 
-int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
-			    enum rdma_remove_reason why)
+static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
+				   enum rdma_remove_reason why)
 {
 	struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
 	struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
@@ -95,8 +95,8 @@ int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
 	return ret;
 }
 
-int uverbs_free_wq(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_wq(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	struct ib_wq *wq = uobject->object;
 	struct ib_uwq_object *uwq =
@@ -109,8 +109,8 @@ int uverbs_free_wq(struct ib_uobject *uobject,
 	return ret;
 }
 
-int uverbs_free_srq(struct ib_uobject *uobject,
-		    enum rdma_remove_reason why)
+static int uverbs_free_srq(struct ib_uobject *uobject,
+			   enum rdma_remove_reason why)
 {
 	struct ib_srq *srq = uobject->object;
 	struct ib_uevent_object *uevent =
@@ -134,8 +134,8 @@ int uverbs_free_srq(struct ib_uobject *uobject,
 	return ret;
 }
 
-int uverbs_free_cq(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_cq(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	struct ib_cq *cq = uobject->object;
 	struct ib_uverbs_event_queue *ev_queue = cq->cq_context;
@@ -153,14 +153,14 @@ int uverbs_free_cq(struct ib_uobject *uobject,
 	return ret;
 }
 
-int uverbs_free_mr(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_mr(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	return ib_dereg_mr((struct ib_mr *)uobject->object);
 }
 
-int uverbs_free_xrcd(struct ib_uobject *uobject,
-		     enum rdma_remove_reason why)
+static int uverbs_free_xrcd(struct ib_uobject *uobject,
+			    enum rdma_remove_reason why)
 {
 	struct ib_xrcd *xrcd = uobject->object;
 	struct ib_uxrcd_object *uxrcd =
@@ -178,8 +178,8 @@ int uverbs_free_xrcd(struct ib_uobject *uobject,
 	return ret;
 }
 
-int uverbs_free_pd(struct ib_uobject *uobject,
-		   enum rdma_remove_reason why)
+static int uverbs_free_pd(struct ib_uobject *uobject,
+			  enum rdma_remove_reason why)
 {
 	struct ib_pd *pd = uobject->object;
 
@@ -190,8 +190,8 @@ int uverbs_free_pd(struct ib_uobject *uobject,
 	return 0;
 }
 
-int uverbs_hot_unplug_completion_event_file(struct ib_uobject_file *uobj_file,
-					    enum rdma_remove_reason why)
+static int uverbs_hot_unplug_completion_event_file(struct ib_uobject_file *uobj_file,
+						   enum rdma_remove_reason why)
 {
 	struct ib_uverbs_completion_event_file *comp_event_file =
 		container_of(uobj_file, struct ib_uverbs_completion_event_file,
Signature · 6 lines
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20170422142852.18882-2-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org

#4 lore raw

Re: [PATCH rdma-next 1/4] Ib/core: Mark local uverbs_std_types functions to be static

Quoted 141 lines · Leon Romanovsky

Functions declared in uverbs_std_types.c are local to that file, but they lack static declarations. This produces a lot of sparse warnings, like the one below:

drivers/infiniband/core/uverbs_std_types.c:41:5: warning: symbol
'uverbs_free_ah' was not declared. Should it be static?

So mark them as static.

CC Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/[email protected]>
---
 drivers/infiniband/core/uverbs_std_types.c | 48 +++++++++++++++---------------
 1 file changed, 24 insertions(+), 24 deletions(-)
drivers/infiniband/core/uverbs_std_types.c +24 -25
index e3338b19d6a2..ad3caad40945 100644
@@ -38,26 +38,26 @@
 #include "rdma_core.h"
 #include "uverbs.h"
 
-int uverbs_free_ah(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_ah(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        return ib_destroy_ah((struct ib_ah *)uobject->object);
 }
 
-int uverbs_free_flow(struct ib_uobject *uobject,
-                    enum rdma_remove_reason why)
+static int uverbs_free_flow(struct ib_uobject *uobject,
+                           enum rdma_remove_reason why)
 {
        return ib_destroy_flow((struct ib_flow *)uobject->object);
 }
 
-int uverbs_free_mw(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_mw(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        return uverbs_dealloc_mw((struct ib_mw *)uobject->object);
 }
 
-int uverbs_free_qp(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_qp(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        struct ib_qp *qp = uobject->object;
        struct ib_uqp_object *uqp =
@@ -82,8 +82,8 @@ int uverbs_free_qp(struct ib_uobject *uobject,
        return ret;
 }
 
-int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
-                           enum rdma_remove_reason why)
+static int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
+                                  enum rdma_remove_reason why)
 {
        struct ib_rwq_ind_table *rwq_ind_tbl = uobject->object;
        struct ib_wq **ind_tbl = rwq_ind_tbl->ind_tbl;
@@ -95,8 +95,8 @@ int uverbs_free_rwq_ind_tbl(struct ib_uobject *uobject,
        return ret;
 }
 
-int uverbs_free_wq(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_wq(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        struct ib_wq *wq = uobject->object;
        struct ib_uwq_object *uwq =
@@ -109,8 +109,8 @@ int uverbs_free_wq(struct ib_uobject *uobject,
        return ret;
 }
 
-int uverbs_free_srq(struct ib_uobject *uobject,
-                   enum rdma_remove_reason why)
+static int uverbs_free_srq(struct ib_uobject *uobject,
+                          enum rdma_remove_reason why)
 {
        struct ib_srq *srq = uobject->object;
        struct ib_uevent_object *uevent =
@@ -134,8 +134,8 @@ int uverbs_free_srq(struct ib_uobject *uobject,
        return ret;
 }
 
-int uverbs_free_cq(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_cq(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        struct ib_cq *cq = uobject->object;
        struct ib_uverbs_event_queue *ev_queue = cq->cq_context;
@@ -153,14 +153,14 @@ int uverbs_free_cq(struct ib_uobject *uobject,
        return ret;
 }
 
-int uverbs_free_mr(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_mr(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        return ib_dereg_mr((struct ib_mr *)uobject->object);
 }
 
-int uverbs_free_xrcd(struct ib_uobject *uobject,
-                    enum rdma_remove_reason why)
+static int uverbs_free_xrcd(struct ib_uobject *uobject,
+                           enum rdma_remove_reason why)
 {
        struct ib_xrcd *xrcd = uobject->object;
        struct ib_uxrcd_object *uxrcd =
@@ -178,8 +178,8 @@ int uverbs_free_xrcd(struct ib_uobject *uobject,
        return ret;
 }
 
-int uverbs_free_pd(struct ib_uobject *uobject,
-                  enum rdma_remove_reason why)
+static int uverbs_free_pd(struct ib_uobject *uobject,
+                         enum rdma_remove_reason why)
 {
        struct ib_pd *pd = uobject->object;
 
@@ -190,8 +190,8 @@ int uverbs_free_pd(struct ib_uobject *uobject,
        return 0;
 }
 
-int uverbs_hot_unplug_completion_event_file(struct ib_uobject_file *uobj_file,
-                                           enum rdma_remove_reason why)
+static int uverbs_hot_unplug_completion_event_file(struct ib_uobject_file *uobj_file,
+                                                  enum rdma_remove_reason why)
 {
        struct ib_uverbs_completion_event_file *comp_event_file =
                container_of(uobj_file, struct ib_uverbs_completion_event_file,
--
Quoted 1 line

2.12.2

Yeah, you're right. Unless we use them in some custom driver's parse tree, they should be static.

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

PATCH 2/4 Ib/usnic: Explicitly include usnic headers

In reply to a message not in the archive

Sparse tool complains about undeclared symbols in usnic_ib_verbs.c and usnic_ib_sysfs.c This is caused by lack of direct include of appropriate usnic_ib_verbs.h and usnic_ib_sysfs.h, where all these functions were declared.

Simple include eliminates 30 warnings similar to the below one:

drivers/infiniband/hw/usnic/usnic_ib_sysfs.c:304:6: warning: symbol
'usnic_ib_sysfs_unregister_usdev' was not declared. Should it be static?

CC Christian Benvenuti <[email protected]>
---
 drivers/infiniband/hw/usnic/usnic_ib_sysfs.c | 1 +
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 1 +
 2 files changed, 2 insertions(+)
drivers/infiniband/hw/usnic/usnic_ib_sysfs.c +1 -0
index 04443242e258..32956f9f5715 100644
@@ -44,6 +44,7 @@
 #include "usnic_vnic.h"
 #include "usnic_ib_verbs.h"
 #include "usnic_log.h"
+#include "usnic_ib_sysfs.h"
 
 static ssize_t usnic_ib_show_board(struct device *device,
 					struct device_attribute *attr,
drivers/infiniband/hw/usnic/usnic_ib_verbs.c +1 -0
index 3284730d3c09..bcd6f7b1c634 100644
@@ -46,6 +46,7 @@
 #include "usnic_log.h"
 #include "usnic_uiom.h"
 #include "usnic_transport.h"
+#include "usnic_ib_verbs.h"
 
 #define USNIC_DEFAULT_TRANSPORT USNIC_TRANSPORT_ROCE_CUSTOM
 

--
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw

PATCH 3/4 IB/usnic: Simplify the code to balance loc/unlock calls

In reply to a message not in the archive

Simplify code in find_free_vf_and_create_qp_grp() to avoid sparse error regarding call to unlock in the block other than lock was called.

drivers/infiniband/hw/usnic/usnic_ib_verbs.c:206:9: warning: context imbalance
in 'find_free_vf_and_create_qp_grp' - different lock contexts for basic block

CC Christian Benvenuti <[email protected]>
---
 drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 45 ++++++++++++++--------------
 1 file changed, 23 insertions(+), 22 deletions(-)
drivers/infiniband/hw/usnic/usnic_ib_verbs.c +23 -22
index bcd6f7b1c634..44a93326f7b5 100644
@@ -152,7 +152,7 @@ find_free_vf_and_create_qp_grp(struct usnic_ib_dev *us_ibdev,
 	struct usnic_vnic *vnic;
 	struct usnic_ib_qp_grp *qp_grp;
 	struct device *dev, **dev_list;
-	int i, found = 0;
+	int i;
 
 	BUG_ON(!mutex_is_locked(&us_ibdev->usdev_lock));
 
@@ -174,8 +174,13 @@ find_free_vf_and_create_qp_grp(struct usnic_ib_dev *us_ibdev,
 						us_ibdev->ib_dev.name,
 						pci_name(usnic_vnic_get_pdev(
 									vnic)));
-				found = 1;
-				break;
+				qp_grp = usnic_ib_qp_grp_create(us_ibdev->ufdev,
+								vf, pd,
+								res_spec,
+								trans_spec);
+
+				spin_unlock(&vf->lock);
+				goto qp_grp_check;
 			}
 			spin_unlock(&vf->lock);
 
@@ -183,34 +188,30 @@ find_free_vf_and_create_qp_grp(struct usnic_ib_dev *us_ibdev,
 		usnic_uiom_free_dev_list(dev_list);
 	}
 
-	if (!found) {
-		/* Try to find resources on an unused vf */
-		list_for_each_entry(vf, &us_ibdev->vf_dev_list, link) {
-			spin_lock(&vf->lock);
-			vnic = vf->vnic;
-			if (vf->qp_grp_ref_cnt == 0 &&
-				usnic_vnic_check_room(vnic, res_spec) == 0) {
-				found = 1;
-				break;
-			}
+	/* Try to find resources on an unused vf */
+	list_for_each_entry(vf, &us_ibdev->vf_dev_list, link) {
+		spin_lock(&vf->lock);
+		vnic = vf->vnic;
+		if (vf->qp_grp_ref_cnt == 0 &&
+		    usnic_vnic_check_room(vnic, res_spec) == 0) {
+			qp_grp = usnic_ib_qp_grp_create(us_ibdev->ufdev, vf,
+							pd, res_spec,
+							trans_spec);
+
 			spin_unlock(&vf->lock);
+			goto qp_grp_check;
 		}
+		spin_unlock(&vf->lock);
 	}
 
-	if (!found) {
-		usnic_info("No free qp grp found on %s\n",
-				us_ibdev->ib_dev.name);
-		return ERR_PTR(-ENOMEM);
-	}
+	usnic_info("No free qp grp found on %s\n", us_ibdev->ib_dev.name);
+	return ERR_PTR(-ENOMEM);
 
-	qp_grp = usnic_ib_qp_grp_create(us_ibdev->ufdev, vf, pd, res_spec,
-						trans_spec);
-	spin_unlock(&vf->lock);
+qp_grp_check:
 	if (IS_ERR_OR_NULL(qp_grp)) {
 		usnic_err("Failed to allocate qp_grp\n");
 		return ERR_PTR(qp_grp ? PTR_ERR(qp_grp) : -ENOMEM);
 	}
-
 	return qp_grp;
 }
 

--
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

PATCH 4/4 IB/nes: Fix incorrect type in assignment

In reply to a message not in the archive

Fix mismatch between types, wqe_words are in le32 format, while opcode in CPU format.

The following sparse warnings are helped to find it:

drivers/infiniband/hw/nes/nes_hw.c:3058:24: warning: incorrect type in assignment (different base types)
drivers/infiniband/hw/nes/nes_hw.c:3058:24: expected unsigned int [unsigned] [assigned] [usertype] opcode
drivers/infiniband/hw/nes/nes_hw.c:3058:24: got restricted __le32 <noident>

---
 drivers/infiniband/hw/nes/nes_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/nes/nes_hw.c +1 -1
index 19acd13c6cb1..4bbff7a33d18 100644
@@ -3055,7 +3055,7 @@ static void nes_cqp_ce_handler(struct nes_device *nesdev, struct nes_hw_cq *cq)
 		memcpy(cqp_wqe, &cqp_request->cqp_wqe, sizeof(*cqp_wqe));
 		barrier();
 
-		opcode = cqp_wqe->wqe_words[NES_CQP_WQE_OPCODE_IDX];
+		opcode = le32_to_cpu(cqp_wqe->wqe_words[NES_CQP_WQE_OPCODE_IDX]);
 		if ((opcode & NES_CQP_OPCODE_MASK) == NES_CQP_DOWNLOAD_SEGMENT)
 			ctx_index = NES_CQP_WQE_DL_COMP_CTX_LOW_IDX;
 		else

--
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
In reply to a message not in the archive
Quoted 15 lines · Leon Romanovsky

Hi Doug,

Please find below 4 patches, which fix 45 sparse warnings.

Patches #1 and #2 are trivial changes. The first adds static keyword to the functions declarations and second adds includes of the relevant
headers.

Patch #3 refactors code to get rid of one temporal variable. That variable
was needed to determine if QP is found. Once removed, it allowed to write
the code with lock and unlock placed together.

Quoted 5 lines

Patch #4 fixes wrong type casting. The opcode is in CPU format and wqe_word
is in le32 format.

This series is based on k.o/for-4.12-rdma-netdevice branch.

Thanks, series applied.

Signature · 9 lines
Doug Ledford <[email protected]>
    GPG KeyID: B826A3330E572FDD
   
Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html