Skip to contentSkip to conversation

IB/hfi1: hfi1 driver patches for-next

PATCH In review Dennis Dalessandro 4 participants 18 messages May 4, 2017 2 messages not in the archive

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

v2 cover IB/hfi1: hfi1 driver patches for-next

Doug,

This is a respin of [1] there were some merge problems with the last 5 in this series, thanks to Leon for notifciing.

Included are some contex init/usage fixes, a couple performance tweaks, and some general code clean up. This is all confined to the hfi1 driver.

I'm OK applying this to next vs the current merge window. We might want to pluck out patches 6 and 7 for -rc. They are marked as stable and I think would meet -rc criteria.

Patches apply cleanly on top of: 87f0faa : IB/SA: Add OPA addr header

Patches can can also be found in my GitHub repo at:
https://github.com/ddalessa/kernel/tree/for-next

[1] http://marc.info/?l=linux-rdma&m=149377206803082&w=2

---

Jakub Byczkowski (1):
IB/hfi1: Fix checks for Offline transient state

Michael J. Ruhl (9):
IB/hfi1: Return an error on memory allocation failure IB/hfi1: Fix a subcontext memory leak
IB/hfi1: Name function prototype parameters
IB/hfi1: Use filedata rather than filepointer
IB/hfi1: Search shared contexts on the opened device, not all devices IB/hfi1: Correctly clear the pkey
IB/hfi1: Clean up context initialization
IB/hfi1: Fix an assign/ordering issue with shared context IDs IB/hfi1: Clean up on context initialization failure

Mike Marciniszyn (2):
IB/hfi1,IB/rdmavt: Move r_adefered to r_lock cache line IB/hfi1: Fix yield logic in send engine

Sebastian Sanchez (2):
IB/hfi1: Get rid of divide when setting the tx request header IB/hfi1: Remove atomic operations for SDMA_REQ_HAVE_AHG bit

Tymoteusz Kielan (1):
IB/hfi1: Adjust default eager_buffer_size to 8MB

 drivers/infiniband/hw/hfi1/chip.c         |   47 +--
 drivers/infiniband/hw/hfi1/chip.h         |   10 -
 drivers/infiniband/hw/hfi1/driver.c       |   42 ---
 drivers/infiniband/hw/hfi1/file_ops.c     |  425 +++++++++++++++--------------
 drivers/infiniband/hw/hfi1/hfi.h          |  107 +++----
 drivers/infiniband/hw/hfi1/init.c         |   33 +-
 drivers/infiniband/hw/hfi1/intr.c         |    3 
 drivers/infiniband/hw/hfi1/qp.c           |    4 
 drivers/infiniband/hw/hfi1/rc.c           |   13 -
 drivers/infiniband/hw/hfi1/ruc.c          |   80 +++--
 drivers/infiniband/hw/hfi1/trace_ctxts.h  |   17 +
 drivers/infiniband/hw/hfi1/trace_tx.h     |   34 ++
 drivers/infiniband/hw/hfi1/user_exp_rcv.c |  180 +++++++-----
 drivers/infiniband/hw/hfi1/user_exp_rcv.h |   17 +
 drivers/infiniband/hw/hfi1/user_sdma.c    |  191 ++++++-------
 drivers/infiniband/hw/hfi1/user_sdma.h    |   18 +
 drivers/infiniband/hw/hfi1/verbs.h        |    5 
 drivers/infiniband/hw/hfi1/vnic_main.c    |    8 -
 include/rdma/rdmavt_qp.h                  |    1 
 19 files changed, 633 insertions(+), 602 deletions(-)

A message that is not in the archive (sent off-list or not yet received): 20170504120126.32747.45131.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org

#2 lore raw

PATCH v2 1/15 IB/hfi1: Fix checks for Offline transient state

From: Jakub Byczkowski <[email protected]>

In goto_offline() function pstate is masked by 0xff when compared to PLS_OFFLINE state. Mask should be 0xf0, since upper 4 bits specify the "major" state.

---
 drivers/infiniband/hw/hfi1/chip.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/infiniband/hw/hfi1/chip.c +1 -1
index 0f6916d..c96bb3a 100644
@@ -10233,7 +10233,7 @@ static int goto_offline(struct hfi1_pportdata *ppd, u8 rem_reason)
 	if (pstate == PLS_OFFLINE) {
 		do_transition = 0;	/* in right state */
 		do_wait = 0;		/* ...no need to wait */
-	} else if ((pstate & 0xff) == PLS_OFFLINE) {
+	} else if ((pstate & 0xf0) == PLS_OFFLINE) {
 		do_transition = 0;	/* in an offline transient state */
 		do_wait = 1;		/* ...wait for it to settle */
 	} else {

--
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

#3 lore raw

PATCH v2 2/15 IB/hfi1, IB/rdmavt: Move r_adefered to r_lock cache line

In reply to a message not in the archive

From: Mike Marciniszyn <[email protected]>

This field is causing excessive cache line bouncing.

There are spare bytes in the r_lock cache line so the best approach is to make an rvt QP field and remove from the hfi1 priv field.

---
 drivers/infiniband/hw/hfi1/qp.c    |    4 +---
 drivers/infiniband/hw/hfi1/rc.c    |   13 ++++---------
 drivers/infiniband/hw/hfi1/verbs.h |    1 -
 include/rdma/rdmavt_qp.h           |    1 +
 4 files changed, 6 insertions(+), 13 deletions(-)
drivers/infiniband/hw/hfi1/qp.c +1 -3
index 4573e4c..650305c 100644
@@ -731,9 +731,7 @@ void quiesce_qp(struct rvt_qp *qp)
 
 void notify_qp_reset(struct rvt_qp *qp)
 {
-	struct hfi1_qp_priv *priv = qp->priv;
-
-	priv->r_adefered = 0;
+	qp->r_adefered = 0;
 	clear_ahg(qp);
 }
 
drivers/infiniband/hw/hfi1/rc.c +4 -9
index 75a729c..069bdaf 100644
@@ -727,10 +727,9 @@ void hfi1_send_rc_ack(struct hfi1_ctxtdata *rcd, struct rvt_qp *qp,
 	struct ib_header hdr;
 	struct ib_other_headers *ohdr;
 	unsigned long flags;
-	struct hfi1_qp_priv *priv = qp->priv;
 
 	/* clear the defer count */
-	priv->r_adefered = 0;
+	qp->r_adefered = 0;
 
 	/* Don't send ACK or NAK if a RDMA read or atomic is pending. */
 	if (qp->s_flags & RVT_S_RESP_PENDING)
@@ -1604,9 +1603,7 @@ static inline void rc_defered_ack(struct hfi1_ctxtdata *rcd,
 
 static inline void rc_cancel_ack(struct rvt_qp *qp)
 {
-	struct hfi1_qp_priv *priv = qp->priv;
-
-	priv->r_adefered = 0;
+	qp->r_adefered = 0;
 	if (list_empty(&qp->rspwait))
 		return;
 	list_del_init(&qp->rspwait);
@@ -2314,13 +2311,11 @@ void hfi1_rc_rcv(struct hfi1_packet *packet)
 	qp->r_nak_state = 0;
 	/* Send an ACK if requested or required. */
 	if (psn & IB_BTH_REQ_ACK) {
-		struct hfi1_qp_priv *priv = qp->priv;
-
 		if (packet->numpkt == 0) {
 			rc_cancel_ack(qp);
 			goto send_ack;
 		}
-		if (priv->r_adefered >= HFI1_PSN_CREDIT) {
+		if (qp->r_adefered >= HFI1_PSN_CREDIT) {
 			rc_cancel_ack(qp);
 			goto send_ack;
 		}
@@ -2328,7 +2323,7 @@ void hfi1_rc_rcv(struct hfi1_packet *packet)
 			rc_cancel_ack(qp);
 			goto send_ack;
 		}
-		priv->r_adefered++;
+		qp->r_adefered++;
 		rc_defered_ack(rcd, qp);
 	}
 	return;
drivers/infiniband/hw/hfi1/verbs.h +0 -1
index 52ff275..c0913c6 100644
@@ -125,7 +125,6 @@ struct hfi1_qp_priv {
 	struct sdma_engine *s_sde;                /* current sde */
 	struct send_context *s_sendcontext;       /* current sendcontext */
 	u8 s_sc;		                  /* SC[0..4] for next packet */
-	u8 r_adefered;                            /* number of acks defered */
 	struct iowait s_iowait;
 	struct rvt_qp *owner;
 };
include/rdma/rdmavt_qp.h +1 -0
index 1d8141a..be6472e 100644
@@ -324,6 +324,7 @@ struct rvt_qp {
 	u8 r_state;             /* opcode of last packet received */
 	u8 r_flags;
 	u8 r_head_ack_queue;    /* index into s_ack_queue[] */
+	u8 r_adefered;          /* defered ack count */
 
 	struct list_head rspwait;       /* link for waiting to respond */
 

--
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

#4 lore raw

PATCH v2 3/15 IB/hfi1: Fix yield logic in send engine

In reply to a message not in the archive

From: Mike Marciniszyn <[email protected]>

When there are many RC QPs and an RDMA READ request is sent, timeouts occur on the requester side because of fairness among RC QPs on their relative SDMA engine on the responder side. This also hits write and send, but to a lesser extent.

Complicating the issue is that the current code checks if workqueue is congested before scheduling other QPs, however, this check is based on the number of active entries in the workqueue, which was found to be too big to for workqueue_congested() to be effective.

Fix by reducing the number of active entries as revealed by experimentation from the default of num_sdma to HFI1_MAX_ACTIVE_WORKQUEUE_ENTRIES. Retry counts were monitored to determine the correct value.

Tracing to investigate any future issues is also added.

---
 drivers/infiniband/hw/hfi1/init.c     |    3 +
 drivers/infiniband/hw/hfi1/ruc.c      |   80 +++++++++++++++++++++------------
 drivers/infiniband/hw/hfi1/trace_tx.h |   34 ++++++++++++++
 drivers/infiniband/hw/hfi1/verbs.h    |    4 ++
 4 files changed, 90 insertions(+), 31 deletions(-)
drivers/infiniband/hw/hfi1/init.c +2 -1
index 4d6b9f8..71b0204 100644
@@ -70,6 +70,7 @@
 #undef pr_fmt
 #define pr_fmt(fmt) DRIVER_NAME ": " fmt
 
+#define HFI1_MAX_ACTIVE_WORKQUEUE_ENTRIES 5
 /*
  * min buffers we want to have per context, after driver
  */
@@ -623,7 +624,7 @@ static int create_workqueues(struct hfi1_devdata *dd)
 				alloc_workqueue(
 				    "hfi%d_%d",
 				    WQ_SYSFS | WQ_HIGHPRI | WQ_CPU_INTENSIVE,
-				    dd->num_sdma,
+				    HFI1_MAX_ACTIVE_WORKQUEUE_ENTRIES,
 				    dd->unit, pidx);
 			if (!ppd->hfi1_wq)
 				goto wq_error;
drivers/infiniband/hw/hfi1/ruc.c +50 -30

124 lines not on this page · raw message

drivers/infiniband/hw/hfi1/trace_tx.h +34 -0
index 2c9ac57..c59809a 100644
@@ -676,6 +676,40 @@
 	)
 );
 
+DECLARE_EVENT_CLASS(
+	hfi1_do_send_template,
+	TP_PROTO(struct rvt_qp *qp, bool flag),
+	TP_ARGS(qp, flag),
+	TP_STRUCT__entry(
+		DD_DEV_ENTRY(dd_from_ibdev(qp->ibqp.device))
+		__field(u32, qpn)
+		__field(bool, flag)
+	),
+	TP_fast_assign(
+		DD_DEV_ASSIGN(dd_from_ibdev(qp->ibqp.device))
+		__entry->qpn = qp->ibqp.qp_num;
+		__entry->flag = flag;
+	),
+	TP_printk(
+		"[%s] qpn %x flag %d",
+		__get_str(dev),
+		__entry->qpn,
+		__entry->flag
+	)
+);
+
+DEFINE_EVENT(
+	hfi1_do_send_template, hfi1_rc_do_send,
+	TP_PROTO(struct rvt_qp *qp, bool flag),
+	TP_ARGS(qp, flag)
+);
+
+DEFINE_EVENT(
+	hfi1_do_send_template, hfi1_rc_expired_time_slice,
+	TP_PROTO(struct rvt_qp *qp, bool flag),
+	TP_ARGS(qp, flag)
+);
+
 #endif /* __HFI1_TRACE_TX_H */
 
 #undef TRACE_INCLUDE_PATH
drivers/infiniband/hw/hfi1/verbs.h +4 -0
index c0913c6..cd635d0 100644
@@ -139,6 +139,10 @@ struct hfi1_pkt_state {
 	struct hfi1_pportdata *ppd;
 	struct verbs_txreq *s_txreq;
 	unsigned long flags;
+	unsigned long timeout;
+	unsigned long timeout_int;
+	int cpu;
+	bool in_thread;
 };
 
 #define HFI1_PSN_CREDIT  16

--
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 v2 4/15 IB/hfi1: Get rid of divide when setting the tx request header

In reply to a message not in the archive

From: Sebastian Sanchez <[email protected]>

Div instructions show costly in profiles when
the tx request header is set. Using right shift
instead of a divide operation reduces the cycles
spent in the function that sets the tx request
header as shown in the profile. Use right shift
operation instead.

Profile before change:
43.24% 009

|
|--23.41%-- user_sdma_send_pkts
|          |
|          |--99.90%-- hfi1_user_sdma_process_requestAfter:

Profile after change:
45.75% 009

|
|--14.81%-- user_sdma_send_pkts
|          |
|          |--99.95%-- hfi1_user_sdma_process_request
---
 drivers/infiniband/hw/hfi1/user_sdma.c |   34 ++++++++++++++++----------------
 1 files changed, 17 insertions(+), 17 deletions(-)
drivers/infiniband/hw/hfi1/user_sdma.c +17 -17
index 0749689..8adb6df 100644
@@ -143,7 +143,9 @@
 
 /* KDETH OM multipliers and switch over point */
 #define KDETH_OM_SMALL     4
+#define KDETH_OM_SMALL_SHIFT     2
 #define KDETH_OM_LARGE     64
+#define KDETH_OM_LARGE_SHIFT     6
 #define KDETH_OM_MAX_SIZE  (1 << ((KDETH_OM_LARGE / KDETH_OM_SMALL) + 1))
 
 /* Tx request flag bits */
@@ -229,12 +231,6 @@ struct user_sdma_request {
 	 */
 	u32 tidoffset;
 	/*
-	 * KDETH.OM
-	 * Remember this because the header template always sets it
-	 * to 0.
-	 */
-	u8 omfactor;
-	/*
 	 * We copy the iovs for this request (based on
 	 * info.iovcnt). These are only the data vectors
 	 */
@@ -1323,6 +1319,7 @@ static int set_txreq_header(struct user_sdma_request *req,
 {
 	struct hfi1_user_sdma_pkt_q *pq = req->pq;
 	struct hfi1_pkt_header *hdr = &tx->hdr;
+	u8 omfactor; /* KDETH.OM */
 	u16 pbclen;
 	int ret;
 	u32 tidval = 0, lrhlen = get_lrh_len(*hdr, pad_len(datalen));
@@ -1400,8 +1397,9 @@ static int set_txreq_header(struct user_sdma_request *req,
 			}
 			tidval = req->tids[req->tididx];
 		}
-		req->omfactor = EXP_TID_GET(tidval, LEN) * PAGE_SIZE >=
-			KDETH_OM_MAX_SIZE ? KDETH_OM_LARGE : KDETH_OM_SMALL;
+		omfactor = EXP_TID_GET(tidval, LEN) * PAGE_SIZE >=
+			KDETH_OM_MAX_SIZE ? KDETH_OM_LARGE_SHIFT :
+			KDETH_OM_SMALL_SHIFT;
 		/* Set KDETH.TIDCtrl based on value for this TID. */
 		KDETH_SET(hdr->kdeth.ver_tid_offset, TIDCTRL,
 			  EXP_TID_GET(tidval, CTRL));
@@ -1416,12 +1414,12 @@ static int set_txreq_header(struct user_sdma_request *req,
 		 * transfer.
 		 */
 		SDMA_DBG(req, "TID offset %ubytes %uunits om%u",
-			 req->tidoffset, req->tidoffset / req->omfactor,
-			 req->omfactor != KDETH_OM_SMALL);
+			 req->tidoffset, req->tidoffset >> omfactor,
+			 omfactor != KDETH_OM_SMALL_SHIFT);
 		KDETH_SET(hdr->kdeth.ver_tid_offset, OFFSET,
-			  req->tidoffset / req->omfactor);
+			  req->tidoffset >> omfactor);
 		KDETH_SET(hdr->kdeth.ver_tid_offset, OM,
-			  req->omfactor != KDETH_OM_SMALL);
+			  omfactor != KDETH_OM_SMALL_SHIFT);
 	}
 done:
 	trace_hfi1_sdma_user_header(pq->dd, pq->ctxt, pq->subctxt,
@@ -1433,6 +1431,7 @@ static int set_txreq_header_ahg(struct user_sdma_request *req,
 				struct user_sdma_txreq *tx, u32 len)
 {
 	int diff = 0;
+	u8 omfactor; /* KDETH.OM */
 	struct hfi1_user_sdma_pkt_q *pq = req->pq;
 	struct hfi1_pkt_header *hdr = &req->hdr;
 	u16 pbclen = le16_to_cpu(hdr->pbc[0]);
@@ -1484,14 +1483,15 @@ static int set_txreq_header_ahg(struct user_sdma_request *req,
 			}
 			tidval = req->tids[req->tididx];
 		}
-		req->omfactor = ((EXP_TID_GET(tidval, LEN) *
+		omfactor = ((EXP_TID_GET(tidval, LEN) *
 				  PAGE_SIZE) >=
-				 KDETH_OM_MAX_SIZE) ? KDETH_OM_LARGE :
-			KDETH_OM_SMALL;
+				 KDETH_OM_MAX_SIZE) ? KDETH_OM_LARGE_SHIFT :
+				 KDETH_OM_SMALL_SHIFT;
 		/* KDETH.OM and KDETH.OFFSET (TID) */
 		AHG_HEADER_SET(req->ahg, diff, 7, 0, 16,
-			       ((!!(req->omfactor - KDETH_OM_SMALL)) << 15 |
-				((req->tidoffset / req->omfactor) & 0x7fff)));
+			       ((!!(omfactor - KDETH_OM_SMALL_SHIFT)) << 15 |
+				((req->tidoffset >> omfactor)
+				 & 0x7fff)));
 		/* KDETH.TIDCtrl, KDETH.TID, KDETH.Intr, KDETH.SH */
 		val = cpu_to_le16(((EXP_TID_GET(tidval, CTRL) & 0x3) << 10) |
 				   (EXP_TID_GET(tidval, IDX) & 0x3ff));

--
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 v2 5/15 IB/hfi1: Adjust default eager_buffer_size to 8MB

In reply to a message not in the archive

From: Tymoteusz Kielan <[email protected]>

Performance analysis shows benefits for PSM2 in increasing eager buffer size from 2MB to 8MB. The change has neutral impact on verbs. Make change to the module parameter's default value. Allocation ring down was verified to work with the larger buffer size.

---
 drivers/infiniband/hw/hfi1/init.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/hfi1/init.c +2 -2
index 71b0204..c573f06 100644
@@ -102,9 +102,9 @@
 module_param_named(rcvarr_split, hfi1_rcvarr_split, uint, S_IRUGO);
 MODULE_PARM_DESC(rcvarr_split, "Percent of context's RcvArray entries used for Eager buffers");
 
-static uint eager_buffer_size = (2 << 20); /* 2MB */
+static uint eager_buffer_size = (8 << 20); /* 8MB */
 module_param(eager_buffer_size, uint, S_IRUGO);
-MODULE_PARM_DESC(eager_buffer_size, "Size of the eager buffers, default: 2MB");
+MODULE_PARM_DESC(eager_buffer_size, "Size of the eager buffers, default: 8MB");
 
 static uint rcvhdrcnt = 2048; /* 2x the max eager buffer count */
 module_param_named(rcvhdrcnt, rcvhdrcnt, uint, S_IRUGO);

--
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): 20170504121421.32747.27378.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org

#8 lore raw

Re: [PATCH v2 05/15] IB/hfi1: Adjust default eager_buffer_size to 8MB

Quoted 7 lines · Dennis Dalessandro

From: Tymoteusz Kielan <[email protected]>

Performance analysis shows benefits for PSM2 in increasing eager buffer size from 2MB to 8MB. The change has neutral impact on verbs. Make change to the module parameter's default value. Allocation ring down was verified to work with the larger buffer size.

Quoted 6 lines

---

Impressive list :)

Quoted 24 lines
 drivers/infiniband/hw/hfi1/init.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/hfi1/init.c +2 -3
index 71b0204..c573f06 100644
@@ -102,9 +102,9 @@
 module_param_named(rcvarr_split, hfi1_rcvarr_split, uint, S_IRUGO);
 MODULE_PARM_DESC(rcvarr_split, "Percent of context's RcvArray entries used for Eager buffers");
 
-static uint eager_buffer_size = (2 << 20); /* 2MB */
+static uint eager_buffer_size = (8 << 20); /* 8MB */
 module_param(eager_buffer_size, uint, S_IRUGO);
-MODULE_PARM_DESC(eager_buffer_size, "Size of the eager buffers, default: 2MB");
+MODULE_PARM_DESC(eager_buffer_size, "Size of the eager buffers, default: 8MB");
 
 static uint rcvhdrcnt = 2048; /* 2x the max eager buffer count */
 module_param_named(rcvhdrcnt, rcvhdrcnt, uint, S_IRUGO);
 
--

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

#9 lore raw

PATCH v2 6/15 IB/hfi1: Return an error on memory allocation failure

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

If the eager buffer allocation fails, it is necessary to return an error code.

---
 drivers/infiniband/hw/hfi1/init.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/infiniband/hw/hfi1/init.c +1 -0
index c573f06..b2db776 100644
@@ -1778,6 +1778,7 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
 			    !HFI1_CAP_KGET_MASK(rcd->flags, MULTI_PKT_EGR)) {
 				dd_dev_err(dd, "ctxt%u: Failed to allocate eager buffers\n",
 					   rcd->ctxt);
+				ret = -ENOMEM;
 				goto bail_rcvegrbuf_phys;
 			}
 

--
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

#10 lore raw

PATCH v2 7/15 IB/hfi1: Fix a subcontext memory leak

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

The only context that frees user_exp_rcv data structures is the last context closed (from a sub-context set). This leaks the allocations from the other sub-contexts. Separate the common frees from the specific frees and call them at the appropriate time.

Using KEDR to check for memory leaks we get:

Before test:

[leak_check] Possible leaks: 25

After test:

[leak_check] Possible leaks: 31 (6 leaked data structures)

After patch applied (before and after test have the same value)

[leak_check] Possible leaks: 25

Each leak is 192 + 13440 + 6720 = 20352 bytes per sub-context.

---
 drivers/infiniband/hw/hfi1/file_ops.c     |    5 ++++-
 drivers/infiniband/hw/hfi1/user_exp_rcv.c |   32 ++++++++++++++++-------------
 drivers/infiniband/hw/hfi1/user_exp_rcv.h |    1 +
 3 files changed, 23 insertions(+), 15 deletions(-)
drivers/infiniband/hw/hfi1/file_ops.c +4 -1
index 3d9bce4..afdf3ef 100644
@@ -756,6 +756,9 @@ static int hfi1_file_close(struct inode *inode, struct file *fp)
 	/* release the cpu */
 	hfi1_put_proc_affinity(fdata->rec_cpu_num);
 
+	/* clean up rcv side */
+	hfi1_user_exp_rcv_free(fdata);
+
 	/*
 	 * Clear any left over, unhandled events so the next process that
 	 * gets this context doesn't get confused.
@@ -795,7 +798,7 @@ static int hfi1_file_close(struct inode *inode, struct file *fp)
 
 	dd->rcd[uctxt->ctxt] = NULL;
 
-	hfi1_user_exp_rcv_free(fdata);
+	hfi1_user_exp_rcv_grp_free(uctxt);
 	hfi1_clear_ctxt_pkey(dd, uctxt->ctxt);
 
 	uctxt->rcvwait_to = 0;
drivers/infiniband/hw/hfi1/user_exp_rcv.c +18 -14
index 35c6e7e..c7f13df 100644
@@ -252,36 +252,40 @@ int hfi1_user_exp_rcv_init(struct file *fp)
 	return ret;
 }
 
+void hfi1_user_exp_rcv_grp_free(struct hfi1_ctxtdata *uctxt)
+{
+	struct tid_group *grp, *gptr;
+
+	list_for_each_entry_safe(grp, gptr, &uctxt->tid_group_list.list,
+				 list) {
+		list_del_init(&grp->list);
+		kfree(grp);
+	}
+	hfi1_clear_tids(uctxt);
+}
+
 int hfi1_user_exp_rcv_free(struct hfi1_filedata *fd)
 {
 	struct hfi1_ctxtdata *uctxt = fd->uctxt;
-	struct tid_group *grp, *gptr;
 
-	if (!test_bit(HFI1_CTXT_SETUP_DONE, &uctxt->event_flags))
-		return 0;
 	/*
 	 * The notifier would have been removed when the process'es mm
 	 * was freed.
 	 */
-	if (fd->handler)
+	if (fd->handler) {
 		hfi1_mmu_rb_unregister(fd->handler);
-
-	kfree(fd->invalid_tids);
-
-	if (!uctxt->cnt) {
+	} else {
 		if (!EXP_TID_SET_EMPTY(uctxt->tid_full_list))
 			unlock_exp_tids(uctxt, &uctxt->tid_full_list, fd);
 		if (!EXP_TID_SET_EMPTY(uctxt->tid_used_list))
 			unlock_exp_tids(uctxt, &uctxt->tid_used_list, fd);
-		list_for_each_entry_safe(grp, gptr, &uctxt->tid_group_list.list,
-					 list) {
-			list_del_init(&grp->list);
-			kfree(grp);
-		}
-		hfi1_clear_tids(uctxt);
 	}
 
+	kfree(fd->invalid_tids);
+	fd->invalid_tids = NULL;
+
 	kfree(fd->entry_to_rb);
+	fd->entry_to_rb = NULL;
 	return 0;
 }
 
drivers/infiniband/hw/hfi1/user_exp_rcv.h +1 -0
index 9bc8d9f..d1d7d3d 100644
@@ -70,6 +70,7 @@
 		(tid) |= EXP_TID_SET(field, (value));			\
 	} while (0)
 
+void hfi1_user_exp_rcv_grp_free(struct hfi1_ctxtdata *uctxt);
 int hfi1_user_exp_rcv_init(struct file *);
 int hfi1_user_exp_rcv_free(struct hfi1_filedata *);
 int hfi1_user_exp_rcv_setup(struct file *, struct hfi1_tid_info *);

--
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

#11 lore raw

PATCH v2 8/15 IB/hfi1: Name function prototype parameters

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

To improve the readability of function prototypes, give the parameters names.

---
 drivers/infiniband/hw/hfi1/chip.c         |    8 ++-
 drivers/infiniband/hw/hfi1/chip.h         |    6 ++
 drivers/infiniband/hw/hfi1/driver.c       |    4 +-
 drivers/infiniband/hw/hfi1/file_ops.c     |   54 ++++++++++++----------
 drivers/infiniband/hw/hfi1/hfi.h          |   72 +++++++++++++++--------------
 drivers/infiniband/hw/hfi1/init.c         |    2 -
 drivers/infiniband/hw/hfi1/user_exp_rcv.c |   30 +++++++-----
 drivers/infiniband/hw/hfi1/user_exp_rcv.h |   12 ++---
 drivers/infiniband/hw/hfi1/user_sdma.c    |   62 +++++++++++++------------
 drivers/infiniband/hw/hfi1/user_sdma.h    |   13 +++--
 10 files changed, 143 insertions(+), 120 deletions(-)
drivers/infiniband/hw/hfi1/chip.c +4 -4
index c96bb3a..5aa95da 100644
@@ -1055,7 +1055,7 @@ static void handle_lcb_err(struct hfi1_devdata *dd,
 static void handle_sdma_err(struct hfi1_devdata *dd, u32 unused, u64 reg);
 static void handle_egress_err(struct hfi1_devdata *dd, u32 unused, u64 reg);
 static void handle_txe_err(struct hfi1_devdata *dd, u32 unused, u64 reg);
-static void set_partition_keys(struct hfi1_pportdata *);
+static void set_partition_keys(struct hfi1_pportdata *ppd);
 static const char *link_state_name(u32 state);
 static const char *link_state_reason_name(struct hfi1_pportdata *ppd,
 					  u32 state);
@@ -1068,9 +1068,9 @@ static int wait_logical_linkstate(struct hfi1_pportdata *ppd, u32 state,
 				  int msecs);
 static void read_planned_down_reason_code(struct hfi1_devdata *dd, u8 *pdrrc);
 static void read_link_down_reason(struct hfi1_devdata *dd, u8 *ldr);
-static void handle_temp_err(struct hfi1_devdata *);
-static void dc_shutdown(struct hfi1_devdata *);
-static void dc_start(struct hfi1_devdata *);
+static void handle_temp_err(struct hfi1_devdata *dd);
+static void dc_shutdown(struct hfi1_devdata *dd);
+static void dc_start(struct hfi1_devdata *dd);
 static int qos_rmt_entries(struct hfi1_devdata *dd, unsigned int *mp,
 			   unsigned int *np);
 static void clear_full_mgmt_pkey(struct hfi1_pportdata *ppd);
drivers/infiniband/hw/hfi1/chip.h +4 -2
index b9dbf16..4c3c88d 100644
@@ -636,7 +636,8 @@ static inline void write_uctxt_csr(struct hfi1_devdata *dd, int ctxt,
 	write_csr(dd, offset0 + (0x1000 * ctxt), value);
 }
 
-u64 create_pbc(struct hfi1_pportdata *ppd, u64, int, u32, u32);
+u64 create_pbc(struct hfi1_pportdata *ppd, u64 flags, int srate_mbs, u32 vl,
+	       u32 dw_len);
 
 /* firmware.c */
 #define SBUS_MASTER_BROADCAST 0xfd
@@ -728,7 +729,8 @@ void set_link_down_reason(struct hfi1_pportdata *ppd, u8 lcl_reason,
 void set_intr_state(struct hfi1_devdata *dd, u32 enable);
 void apply_link_downgrade_policy(struct hfi1_pportdata *ppd,
 				 int refresh_widths);
-void update_usrhead(struct hfi1_ctxtdata *, u32, u32, u32, u32, u32);
+void update_usrhead(struct hfi1_ctxtdata *rcd, u32 hd, u32 updegr, u32 egrhd,
+		    u32 intr_adjust, u32 npkts);
 int stop_drain_data_vls(struct hfi1_devdata *dd);
 int open_fill_data_vls(struct hfi1_devdata *dd);
 u32 ns_to_cclock(struct hfi1_devdata *dd, u32 ns);
drivers/infiniband/hw/hfi1/driver.c +2 -2
index 5278954..fb319d2 100644
@@ -85,8 +85,8 @@
 MODULE_PARM_DESC(cu, "Credit return units");
 
 unsigned long hfi1_cap_mask = HFI1_CAP_MASK_DEFAULT;
-static int hfi1_caps_set(const char *, const struct kernel_param *);
-static int hfi1_caps_get(char *, const struct kernel_param *);
+static int hfi1_caps_set(const char *val, const struct kernel_param *kp);
+static int hfi1_caps_get(char *buffer, const struct kernel_param *kp);
 static const struct kernel_param_ops cap_ops = {
 	.set = hfi1_caps_set,
 	.get = hfi1_caps_get
drivers/infiniband/hw/hfi1/file_ops.c +30 -24

60 lines not on this page · raw message

drivers/infiniband/hw/hfi1/hfi.h +37 -35

119 lines not on this page · raw message

drivers/infiniband/hw/hfi1/init.c +1 -1
index b2db776..e872644 100644
@@ -118,7 +118,7 @@
 module_param(user_credit_return_threshold, uint, S_IRUGO);
 MODULE_PARM_DESC(user_credit_return_threshold, "Credit return threshold for user send contexts, return when unreturned credits passes this many blocks (in percent of allocated blocks, 0 is off)");
 
-static inline u64 encode_rcv_header_entry_size(u16);
+static inline u64 encode_rcv_header_entry_size(u16 size);
 
 static struct idr hfi1_unit_table;
 u32 hfi1_cpulist_count;
drivers/infiniband/hw/hfi1/user_exp_rcv.c +18 -12
index c7f13df..6090933 100644
@@ -82,20 +82,26 @@ struct tid_pageset {
 		 (unsigned long)(len) - 1) & PAGE_MASK) -	       \
 	       ((unsigned long)vaddr & PAGE_MASK)) >> PAGE_SHIFT))
 
-static void unlock_exp_tids(struct hfi1_ctxtdata *, struct exp_tid_set *,
-			    struct hfi1_filedata *);
-static u32 find_phys_blocks(struct page **, unsigned, struct tid_pageset *);
-static int set_rcvarray_entry(struct file *, unsigned long, u32,
-			      struct tid_group *, struct page **, unsigned);
-static int tid_rb_insert(void *, struct mmu_rb_node *);
+static void unlock_exp_tids(struct hfi1_ctxtdata *uctxt,
+			    struct exp_tid_set *set,
+			    struct hfi1_filedata *fd);
+static u32 find_phys_blocks(struct page **pages, unsigned npages,
+			    struct tid_pageset *list);
+static int set_rcvarray_entry(struct file *fp, unsigned long vaddr,
+			      u32 rcventry, struct tid_group *grp,
+			      struct page **pages, unsigned npages);
+static int tid_rb_insert(void *arg, struct mmu_rb_node *node);
 static void cacheless_tid_rb_remove(struct hfi1_filedata *fdata,
 				    struct tid_rb_node *tnode);
-static void tid_rb_remove(void *, struct mmu_rb_node *);
-static int tid_rb_invalidate(void *, struct mmu_rb_node *);
-static int program_rcvarray(struct file *, unsigned long, struct tid_group *,
-			    struct tid_pageset *, unsigned, u16, struct page **,
-			    u32 *, unsigned *, unsigned *);
-static int unprogram_rcvarray(struct file *, u32, struct tid_group **);
+static void tid_rb_remove(void *arg, struct mmu_rb_node *node);
+static int tid_rb_invalidate(void *arg, struct mmu_rb_node *mnode);
+static int program_rcvarray(struct file *fp, unsigned long vaddr,
+			    struct tid_group *grp,
+			    struct tid_pageset *sets,
+			    unsigned start, u16 count, struct page **pages,
+			    u32 *tidlist, unsigned *tididx, unsigned *pmapped);
+static int unprogram_rcvarray(struct file *fp, u32 tidinfo,
+			      struct tid_group **grp);
 static void clear_tid_node(struct hfi1_filedata *fd, struct tid_rb_node *node);
 
 static struct mmu_rb_ops tid_rb_ops = {
drivers/infiniband/hw/hfi1/user_exp_rcv.h +6 -6
index d1d7d3d..8a33a09 100644
@@ -1,7 +1,7 @@
 #ifndef _HFI1_USER_EXP_RCV_H
 #define _HFI1_USER_EXP_RCV_H
 /*
- * Copyright(c) 2015, 2016 Intel Corporation.
+ * Copyright(c) 2015 - 2017 Intel Corporation.
  *
  * This file is provided under a dual BSD/GPLv2 license.  When using or
  * redistributing this file, you may do so under either license.
@@ -71,10 +71,10 @@
 	} while (0)
 
 void hfi1_user_exp_rcv_grp_free(struct hfi1_ctxtdata *uctxt);
-int hfi1_user_exp_rcv_init(struct file *);
-int hfi1_user_exp_rcv_free(struct hfi1_filedata *);
-int hfi1_user_exp_rcv_setup(struct file *, struct hfi1_tid_info *);
-int hfi1_user_exp_rcv_clear(struct file *, struct hfi1_tid_info *);
-int hfi1_user_exp_rcv_invalid(struct file *, struct hfi1_tid_info *);
+int hfi1_user_exp_rcv_init(struct file *fp);
+int hfi1_user_exp_rcv_free(struct hfi1_filedata *fd);
+int hfi1_user_exp_rcv_setup(struct file *fp, struct hfi1_tid_info *tinfo);
+int hfi1_user_exp_rcv_clear(struct file *fp, struct hfi1_tid_info *tinfo);
+int hfi1_user_exp_rcv_invalid(struct file *fp, struct hfi1_tid_info *tinfo);
 
 #endif /* _HFI1_USER_EXP_RCV_H */
drivers/infiniband/hw/hfi1/user_sdma.c +33 -29

77 lines not on this page · raw message

drivers/infiniband/hw/hfi1/user_sdma.h +8 -5
index 3900171..95ec783 100644
@@ -1,5 +1,7 @@
+#ifndef _HFI1_USER_SDMA_H
+#define _HFI1_USER_SDMA_H
 /*
- * Copyright(c) 2015, 2016 Intel Corporation.
+ * Copyright(c) 2015 - 2017 Intel Corporation.
  *
  * This file is provided under a dual BSD/GPLv2 license.  When using or
  * redistributing this file, you may do so under either license.
@@ -78,7 +80,8 @@ struct hfi1_user_sdma_comp_q {
 	struct hfi1_sdma_comp_entry *comps;
 };
 
-int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *, struct file *);
-int hfi1_user_sdma_free_queues(struct hfi1_filedata *);
-int hfi1_user_sdma_process_request(struct file *, struct iovec *, unsigned long,
-				   unsigned long *);
+int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *uctxt, struct file *fp);
+int hfi1_user_sdma_free_queues(struct hfi1_filedata *fd);
+int hfi1_user_sdma_process_request(struct file *fp, struct iovec *iovec,
+				   unsigned long dim, unsigned long *count);
+#endif /* _HFI1_USER_SDMA_H */

--
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

#12 lore raw

PATCH v2 9/15 IB/hfi1: Use filedata rather than filepointer

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

Since almost all functions that use the hfi1_filedata get the pointer from the file pointer, simplify by only passing the hfi1_filedata pointer.

---
 drivers/infiniband/hw/hfi1/file_ops.c     |   68 ++++++++++++++---------------
 drivers/infiniband/hw/hfi1/user_exp_rcv.c |   45 +++++++++----------
 drivers/infiniband/hw/hfi1/user_exp_rcv.h |   11 +++--
 drivers/infiniband/hw/hfi1/user_sdma.c    |   14 +++---
 drivers/infiniband/hw/hfi1/user_sdma.h    |    9 +++-
 5 files changed, 72 insertions(+), 75 deletions(-)
drivers/infiniband/hw/hfi1/file_ops.c +33 -35
index 0b77283..ab798a8 100644
@@ -80,16 +80,18 @@
 static int assign_ctxt(struct file *fp, struct hfi1_user_info *uinfo);
 static int init_subctxts(struct hfi1_ctxtdata *uctxt,
 			 const struct hfi1_user_info *uinfo);
-static int user_init(struct file *fp);
-static int get_ctxt_info(struct file *fp, void __user *ubase, __u32 len);
-static int get_base_info(struct file *fp, void __user *ubase, __u32 len);
-static int setup_ctxt(struct file *fp);
+static int user_init(struct hfi1_filedata *fd);
+static int get_ctxt_info(struct hfi1_filedata *fd, void __user *ubase,
+			 __u32 len);
+static int get_base_info(struct hfi1_filedata *fd, void __user *ubase,
+			 __u32 len);
+static int setup_ctxt(struct hfi1_filedata *fd);
 static int setup_subctxt(struct hfi1_ctxtdata *uctxt);
-static int get_user_context(struct file *fp, struct hfi1_user_info *uinfo,
-			    int devno);
-static int find_shared_ctxt(struct file *fp,
+static int get_user_context(struct hfi1_filedata *fd,
+			    struct hfi1_user_info *uinfo, int devno);
+static int find_shared_ctxt(struct hfi1_filedata *fd,
 			    const struct hfi1_user_info *uinfo);
-static int allocate_ctxt(struct file *fp, struct hfi1_devdata *dd,
+static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
 			 struct hfi1_user_info *uinfo);
 static unsigned int poll_urgent(struct file *fp, struct poll_table_struct *pt);
 static unsigned int poll_next(struct file *fp, struct poll_table_struct *pt);
@@ -238,17 +240,17 @@ static long hfi1_file_ioctl(struct file *fp, unsigned int cmd,
 		ret = assign_ctxt(fp, &uinfo);
 		if (ret < 0)
 			return ret;
-		ret = setup_ctxt(fp);
+		ret = setup_ctxt(fd);
 		if (ret)
 			return ret;
-		ret = user_init(fp);
+		ret = user_init(fd);
 		break;
 	case HFI1_IOCTL_CTXT_INFO:
-		ret = get_ctxt_info(fp, (void __user *)(unsigned long)arg,
+		ret = get_ctxt_info(fd, (void __user *)(unsigned long)arg,
 				    sizeof(struct hfi1_ctxt_info));
 		break;
 	case HFI1_IOCTL_USER_INFO:
-		ret = get_base_info(fp, (void __user *)(unsigned long)arg,
+		ret = get_base_info(fd, (void __user *)(unsigned long)arg,
 				    sizeof(struct hfi1_base_info));
 		break;
 	case HFI1_IOCTL_CREDIT_UPD:
@@ -262,7 +264,7 @@ static long hfi1_file_ioctl(struct file *fp, unsigned int cmd,
 				   sizeof(tinfo)))
 			return -EFAULT;
 
-		ret = hfi1_user_exp_rcv_setup(fp, &tinfo);
+		ret = hfi1_user_exp_rcv_setup(fd, &tinfo);
 		if (!ret) {
 			/*
 			 * Copy the number of tidlist entries we used
@@ -284,7 +286,7 @@ static long hfi1_file_ioctl(struct file *fp, unsigned int cmd,
 				   sizeof(tinfo)))
 			return -EFAULT;
 
-		ret = hfi1_user_exp_rcv_clear(fp, &tinfo);
+		ret = hfi1_user_exp_rcv_clear(fd, &tinfo);
 		if (ret)
 			break;
 		addr = arg + offsetof(struct hfi1_tid_info, tidcnt);
@@ -299,7 +301,7 @@ static long hfi1_file_ioctl(struct file *fp, unsigned int cmd,
 				   sizeof(tinfo)))
 			return -EFAULT;
 
-		ret = hfi1_user_exp_rcv_invalid(fp, &tinfo);
+		ret = hfi1_user_exp_rcv_invalid(fd, &tinfo);
 		if (ret)
 			break;
 		addr = arg + offsetof(struct hfi1_tid_info, tidcnt);
@@ -436,7 +438,7 @@ static ssize_t hfi1_write_iter(struct kiocb *kiocb, struct iov_iter *from)
 		unsigned long count = 0;
 
 		ret = hfi1_user_sdma_process_request(
-			kiocb->ki_filp,	(struct iovec *)(from->iov + done),
+			fd, (struct iovec *)(from->iov + done),
 			dim, &count);
 		if (ret) {
 			reqs = ret;
@@ -863,7 +865,7 @@ static int assign_ctxt(struct file *fp, struct hfi1_user_info *uinfo)
 	if (uinfo->subctxt_cnt) {
 		struct hfi1_filedata *fd = fp->private_data;
 
-		ret = find_shared_ctxt(fp, uinfo);
+		ret = find_shared_ctxt(fd, uinfo);
 		if (ret < 0)
 			goto done_unlock;
 		if (ret) {
@@ -878,7 +880,7 @@ static int assign_ctxt(struct file *fp, struct hfi1_user_info *uinfo)
 	 */
 	if (!ret) {
 		i_minor = iminor(file_inode(fp)) - HFI1_USER_MINOR_BASE;
-		ret = get_user_context(fp, uinfo, i_minor);
+		ret = get_user_context(fp->private_data, uinfo, i_minor);
 	}
 done_unlock:
 	mutex_unlock(&hfi1_mutex);
@@ -886,8 +888,8 @@ static int assign_ctxt(struct file *fp, struct hfi1_user_info *uinfo)
 	return ret;
 }
 
-static int get_user_context(struct file *fp, struct hfi1_user_info *uinfo,
-			    int devno)
+static int get_user_context(struct hfi1_filedata *fd,
+			    struct hfi1_user_info *uinfo, int devno)
 {
 	struct hfi1_devdata *dd = NULL;
 	int devmax, npresent, nup;
@@ -905,15 +907,14 @@ static int get_user_context(struct file *fp, struct hfi1_user_info *uinfo,
 	else if (!dd->freectxts)
 		return -EBUSY;
 
-	return allocate_ctxt(fp, dd, uinfo);
+	return allocate_ctxt(fd, dd, uinfo);
 }
 
-static int find_shared_ctxt(struct file *fp,
+static int find_shared_ctxt(struct hfi1_filedata *fd,
 			    const struct hfi1_user_info *uinfo)
 {
 	int devmax, ndev, i;
 	int ret = 0;
-	struct hfi1_filedata *fd = fp->private_data;
 
 	devmax = hfi1_count_units(NULL, NULL);
 
@@ -960,10 +961,9 @@ static int find_shared_ctxt(struct file *fp,
 	return ret;
 }
 
-static int allocate_ctxt(struct file *fp, struct hfi1_devdata *dd,
+static int allocate_ctxt(struct hfi1_filedata *fd, struct hfi1_devdata *dd,
 			 struct hfi1_user_info *uinfo)
 {
-	struct hfi1_filedata *fd = fp->private_data;
 	struct hfi1_ctxtdata *uctxt;
 	unsigned ctxt;
 	int ret, numa;
@@ -1113,10 +1113,9 @@ static int setup_subctxt(struct hfi1_ctxtdata *uctxt)
 	return ret;
 }
 
-static int user_init(struct file *fp)
+static int user_init(struct hfi1_filedata *fd)
 {
 	unsigned int rcvctrl_ops = 0;
-	struct hfi1_filedata *fd = fp->private_data;
 	struct hfi1_ctxtdata *uctxt = fd->uctxt;
 
 	/* make sure that the context has already been setup */
@@ -1179,10 +1178,10 @@ static int user_init(struct file *fp)
 	return 0;
 }
 
-static int get_ctxt_info(struct file *fp, void __user *ubase, __u32 len)
+static int get_ctxt_info(struct hfi1_filedata *fd, void __user *ubase,
+			 __u32 len)
 {
 	struct hfi1_ctxt_info cinfo;
-	struct hfi1_filedata *fd = fp->private_data;
 	struct hfi1_ctxtdata *uctxt = fd->uctxt;
 	int ret = 0;
 
@@ -1220,9 +1219,8 @@ static int get_ctxt_info(struct file *fp, void __user *ubase, __u32 len)
 	return ret;
 }
 
-static int setup_ctxt(struct file *fp)
+static int setup_ctxt(struct hfi1_filedata *fd)
 {
-	struct hfi1_filedata *fd = fp->private_data;
 	struct hfi1_ctxtdata *uctxt = fd->uctxt;
 	struct hfi1_devdata *dd = uctxt->dd;
 	int ret = 0;
@@ -1257,7 +1255,7 @@ static int setup_ctxt(struct file *fp)
 			goto done;
 	}
 
-	ret = hfi1_user_sdma_alloc_queues(uctxt, fp);
+	ret = hfi1_user_sdma_alloc_queues(uctxt, fd);
 	if (ret)
 		goto done;
 	/*
@@ -1269,7 +1267,7 @@ static int setup_ctxt(struct file *fp)
 	 * (due to the above wait_event_interruptible() until the master
 	 * is setup.
 	 */
-	ret = hfi1_user_exp_rcv_init(fp);
+	ret = hfi1_user_exp_rcv_init(fd);
 	if (ret)
 		goto done;
 
@@ -1278,10 +1276,10 @@ static int setup_ctxt(struct file *fp)
 	return ret;
 }
 
-static int get_base_info(struct file *fp, void __user *ubase, __u32 len)
+static int get_base_info(struct hfi1_filedata *fd, void __user *ubase,
+			 __u32 len)
 {
 	struct hfi1_base_info binfo;
-	struct hfi1_filedata *fd = fp->private_data;
 	struct hfi1_ctxtdata *uctxt = fd->uctxt;
 	struct hfi1_devdata *dd = uctxt->dd;
 	ssize_t sz;
drivers/infiniband/hw/hfi1/user_exp_rcv.c +20 -25

154 lines not on this page · raw message

drivers/infiniband/hw/hfi1/user_exp_rcv.h +7 -4
index 8a33a09..9787511 100644
@@ -71,10 +71,13 @@
 	} while (0)
 
 void hfi1_user_exp_rcv_grp_free(struct hfi1_ctxtdata *uctxt);
-int hfi1_user_exp_rcv_init(struct file *fp);
+int hfi1_user_exp_rcv_init(struct hfi1_filedata *fd);
 int hfi1_user_exp_rcv_free(struct hfi1_filedata *fd);
-int hfi1_user_exp_rcv_setup(struct file *fp, struct hfi1_tid_info *tinfo);
-int hfi1_user_exp_rcv_clear(struct file *fp, struct hfi1_tid_info *tinfo);
-int hfi1_user_exp_rcv_invalid(struct file *fp, struct hfi1_tid_info *tinfo);
+int hfi1_user_exp_rcv_setup(struct hfi1_filedata *fd,
+			    struct hfi1_tid_info *tinfo);
+int hfi1_user_exp_rcv_clear(struct hfi1_filedata *fd,
+			    struct hfi1_tid_info *tinfo);
+int hfi1_user_exp_rcv_invalid(struct hfi1_filedata *fd,
+			      struct hfi1_tid_info *tinfo);
 
 #endif /* _HFI1_USER_EXP_RCV_H */
drivers/infiniband/hw/hfi1/user_sdma.c +6 -8
index 17142c5..4ac4e9d 100644
@@ -372,9 +372,9 @@ static void sdma_kmem_cache_ctor(void *obj)
 	memset(tx, 0, sizeof(*tx));
 }
 
-int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *uctxt, struct file *fp)
+int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *uctxt,
+				struct hfi1_filedata *fd)
 {
-	struct hfi1_filedata *fd;
 	int ret = 0;
 	char buf[64];
 	struct hfi1_devdata *dd;
@@ -382,13 +382,11 @@ int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *uctxt, struct file *fp)
 	struct hfi1_user_sdma_pkt_q *pq;
 	unsigned long flags;
 
-	if (!uctxt || !fp) {
+	if (!uctxt || !fd) {
 		ret = -EBADF;
 		goto done;
 	}
 
-	fd = fp->private_data;
-
 	if (!hfi1_sdma_comp_ring_size) {
 		ret = -EINVAL;
 		goto done;
@@ -536,11 +534,11 @@ static u8 dlid_to_selector(u16 dlid)
 	return mapping[hash];
 }
 
-int hfi1_user_sdma_process_request(struct file *fp, struct iovec *iovec,
-				   unsigned long dim, unsigned long *count)
+int hfi1_user_sdma_process_request(struct hfi1_filedata *fd,
+				   struct iovec *iovec, unsigned long dim,
+				   unsigned long *count)
 {
 	int ret = 0, i;
-	struct hfi1_filedata *fd = fp->private_data;
 	struct hfi1_ctxtdata *uctxt = fd->uctxt;
 	struct hfi1_user_sdma_pkt_q *pq = fd->pq;
 	struct hfi1_user_sdma_comp_q *cq = fd->cq;
drivers/infiniband/hw/hfi1/user_sdma.h +6 -3
index 95ec783..9181d7c 100644
@@ -80,8 +80,11 @@ struct hfi1_user_sdma_comp_q {
 	struct hfi1_sdma_comp_entry *comps;
 };
 
-int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *uctxt, struct file *fp);
+int hfi1_user_sdma_alloc_queues(struct hfi1_ctxtdata *uctxt,
+				struct hfi1_filedata *fd);
 int hfi1_user_sdma_free_queues(struct hfi1_filedata *fd);
-int hfi1_user_sdma_process_request(struct file *fp, struct iovec *iovec,
-				   unsigned long dim, unsigned long *count);
+int hfi1_user_sdma_process_request(struct hfi1_filedata *fd,
+				   struct iovec *iovec, unsigned long dim,
+				   unsigned long *count);
+
 #endif /* _HFI1_USER_SDMA_H */

--
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

#13 lore raw

PATCH v2 10/15 IB/hfi1: Remove atomic operations for SDMA_REQ_HAVE_AHG bit

In reply to a message not in the archive

From: Sebastian Sanchez <[email protected]>

The AHG index is only accessed in the request call from user space, so there's no need for atomic semantics. Replace atomic operations for SDMA_REQ_HAVE_AHG bit with a test of the AHG index.

---
 drivers/infiniband/hw/hfi1/user_sdma.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)
drivers/infiniband/hw/hfi1/user_sdma.c +8 -14
index 4ac4e9d..6b72267 100644
@@ -155,9 +155,8 @@
 /* SDMA request flag bits */
 #define SDMA_REQ_FOR_THREAD 1
 #define SDMA_REQ_SEND_DONE  2
-#define SDMA_REQ_HAVE_AHG   3
-#define SDMA_REQ_HAS_ERROR  4
-#define SDMA_REQ_DONE_ERROR 5
+#define SDMA_REQ_HAS_ERROR  3
+#define SDMA_REQ_DONE_ERROR 4
 
 #define SDMA_PKT_Q_INACTIVE BIT(0)
 #define SDMA_PKT_Q_ACTIVE   BIT(1)
@@ -216,7 +215,7 @@ struct user_sdma_request {
 	 * each request will need it's own engine pointer.
 	 */
 	struct sdma_engine *sde;
-	u8 ahg_idx;
+	s8 ahg_idx;
 	u32 ahg[9];
 	/*
 	 * KDETH.Offset (Eager) field
@@ -614,6 +613,7 @@ int hfi1_user_sdma_process_request(struct hfi1_filedata *fd,
 	req->pq = pq;
 	req->cq = cq;
 	req->status = -1;
+	req->ahg_idx = -1;
 	INIT_LIST_HEAD(&req->txps);
 
 	memcpy(&req->info, &info, sizeof(info));
@@ -764,14 +764,8 @@ int hfi1_user_sdma_process_request(struct hfi1_filedata *fd,
 	}
 
 	/* We don't need an AHG entry if the request contains only one packet */
-	if (req->info.npkts > 1 && HFI1_CAP_IS_USET(SDMA_AHG)) {
-		int ahg = sdma_ahg_alloc(req->sde);
-
-		if (likely(ahg >= 0)) {
-			req->ahg_idx = (u8)ahg;
-			set_bit(SDMA_REQ_HAVE_AHG, &req->flags);
-		}
-	}
+	if (req->info.npkts > 1 && HFI1_CAP_IS_USET(SDMA_AHG))
+		req->ahg_idx = sdma_ahg_alloc(req->sde);
 
 	set_comp_state(pq, cq, info.comp_idx, QUEUED, 0);
 	atomic_inc(&pq->n_reqs);
@@ -989,7 +983,7 @@ static int user_sdma_send_pkts(struct user_sdma_request *req, unsigned maxpkts)
 			}
 		}
 
-		if (test_bit(SDMA_REQ_HAVE_AHG, &req->flags)) {
+		if (req->ahg_idx >= 0) {
 			if (!req->seqnum) {
 				u16 pbclen = le16_to_cpu(req->hdr.pbc[0]);
 				u32 lrhlen = get_lrh_len(req->hdr,
@@ -1119,7 +1113,7 @@ static int user_sdma_send_pkts(struct user_sdma_request *req, unsigned maxpkts)
 		 * happen due to the sequential manner in which
 		 * descriptors are processed.
 		 */
-		if (test_bit(SDMA_REQ_HAVE_AHG, &req->flags))
+		if (req->ahg_idx >= 0)
 			sdma_ahg_free(req->sde, req->ahg_idx);
 	}
 	return ret;

--
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

#14 lore raw

PATCH v2 11/15 IB/hfi1: Search shared contexts on the opened device, not all devices

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

The search for available shared contexts walks each registered hfi1 device. This search is too broad because other devices may not be on the same fabric, and using its contexts could cause unexpected behavior.

Removed walking the list of devices, limiting the search to the opened device. With the device walk removed, the hfi1_devdata (dd) is not available. Added it to the hfi1_filedata for reference.

With this change, hfi1_count_units() was rendered obsolete and was removed.

---
 drivers/infiniband/hw/hfi1/driver.c   |   36 ----------
 drivers/infiniband/hw/hfi1/file_ops.c |  122 +++++++++++++--------------------
 drivers/infiniband/hw/hfi1/hfi.h      |    2 -
 3 files changed, 49 insertions(+), 111 deletions(-)
drivers/infiniband/hw/hfi1/driver.c +0 -36
index fb319d2..566d152 100644
@@ -211,42 +211,6 @@ int hfi1_count_active_units(void)
 }
 
 /*
- * Return count of all units, optionally return in arguments
- * the number of usable (present) units, and the number of
- * ports that are up.
- */
-int hfi1_count_units(int *npresentp, int *nupp)
-{
-	int nunits = 0, npresent = 0, nup = 0;
-	struct hfi1_devdata *dd;
-	unsigned long flags;
-	int pidx;
-	struct hfi1_pportdata *ppd;
-
-	spin_lock_irqsave(&hfi1_devs_lock, flags);
-
-	list_for_each_entry(dd, &hfi1_dev_list, list) {
-		nunits++;
-		if ((dd->flags & HFI1_PRESENT) && dd->kregbase)
-			npresent++;
-		for (pidx = 0; pidx < dd->num_pports; ++pidx) {
-			ppd = dd->pport + pidx;
-			if (ppd->lid && ppd->linkup)
-				nup++;
-		}
-	}
-
-	spin_unlock_irqrestore(&hfi1_devs_lock, flags);
-
-	if (npresentp)
-		*npresentp = npresent;
-	if (nupp)
-		*nupp = nup;
-
-	return nunits;
-}
-
-/*
  * Get address of eager buffer from it's index (allocated in chunks, not
  * contiguous).
  */
drivers/infiniband/hw/hfi1/file_ops.c +48 -74

199 lines not on this page · raw message

drivers/infiniband/hw/hfi1/hfi.h +1 -1
index a1e9b8b..e0c4c5e 100644
@@ -1238,6 +1238,7 @@ static inline bool hfi1_vnic_is_rsm_full(struct hfi1_devdata *dd, int spare)
 
 /* Private data for file operations */
 struct hfi1_filedata {
+	struct hfi1_devdata *dd;
 	struct hfi1_ctxtdata *uctxt;
 	unsigned subctxt;
 	struct hfi1_user_sdma_comp_q *cq;
@@ -1264,7 +1265,6 @@ struct hfi1_filedata {
 extern unsigned long *hfi1_cpulist;
 
 int hfi1_init(struct hfi1_devdata *dd, int reinit);
-int hfi1_count_units(int *npresentp, int *nupp);
 int hfi1_count_active_units(void);
 
 int hfi1_diag_add(struct hfi1_devdata *dd);

--
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

#15 lore raw

PATCH v2 12/15 IB/hfi1: Correctly clear the pkey

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

In the close path the context is removed from the device array, and then the clear pkey function is called. The pkey function trys to get the context from the device array, but because it was removed the clearing does not occur.

Rework pkey clear function to work as expected. Update the function variable to reflect the correct size and name of the hw_context.

---
 drivers/infiniband/hw/hfi1/chip.c      |   34 +++++++++++++-------------------
 drivers/infiniband/hw/hfi1/chip.h      |    2 +-
 drivers/infiniband/hw/hfi1/file_ops.c  |    2 +-
 drivers/infiniband/hw/hfi1/vnic_main.c |    2 +-
 4 files changed, 17 insertions(+), 23 deletions(-)
drivers/infiniband/hw/hfi1/chip.c +14 -20
index 5aa95da..9f72d55 100644
@@ -14528,30 +14528,24 @@ int hfi1_set_ctxt_pkey(struct hfi1_devdata *dd, unsigned ctxt, u16 pkey)
 	return ret;
 }
 
-int hfi1_clear_ctxt_pkey(struct hfi1_devdata *dd, unsigned ctxt)
+int hfi1_clear_ctxt_pkey(struct hfi1_devdata *dd, struct hfi1_ctxtdata *ctxt)
 {
-	struct hfi1_ctxtdata *rcd;
-	unsigned sctxt;
-	int ret = 0;
+	u8 hw_ctxt;
 	u64 reg;
 
-	if (ctxt < dd->num_rcv_contexts) {
-		rcd = dd->rcd[ctxt];
-	} else {
-		ret = -EINVAL;
-		goto done;
-	}
-	if (!rcd || !rcd->sc) {
-		ret = -EINVAL;
-		goto done;
-	}
-	sctxt = rcd->sc->hw_context;
-	reg = read_kctxt_csr(dd, sctxt, SEND_CTXT_CHECK_ENABLE);
+	if (!ctxt || !ctxt->sc)
+		return -EINVAL;
+
+	if (ctxt->ctxt >= dd->num_rcv_contexts)
+		return -EINVAL;
+
+	hw_ctxt = ctxt->sc->hw_context;
+	reg = read_kctxt_csr(dd, hw_ctxt, SEND_CTXT_CHECK_ENABLE);
 	reg &= ~SEND_CTXT_CHECK_ENABLE_CHECK_PARTITION_KEY_SMASK;
-	write_kctxt_csr(dd, sctxt, SEND_CTXT_CHECK_ENABLE, reg);
-	write_kctxt_csr(dd, sctxt, SEND_CTXT_CHECK_PARTITION_KEY, 0);
-done:
-	return ret;
+	write_kctxt_csr(dd, hw_ctxt, SEND_CTXT_CHECK_ENABLE, reg);
+	write_kctxt_csr(dd, hw_ctxt, SEND_CTXT_CHECK_PARTITION_KEY, 0);
+
+	return 0;
 }
 
 /*
drivers/infiniband/hw/hfi1/chip.h +1 -1
index 4c3c88d..760aa2e 100644
@@ -1362,7 +1362,7 @@ void hfi1_put_tid(struct hfi1_devdata *dd, u32 index,
 int hfi1_set_ctxt_jkey(struct hfi1_devdata *dd, unsigned ctxt, u16 jkey);
 int hfi1_clear_ctxt_jkey(struct hfi1_devdata *dd, unsigned ctxt);
 int hfi1_set_ctxt_pkey(struct hfi1_devdata *dd, unsigned ctxt, u16 pkey);
-int hfi1_clear_ctxt_pkey(struct hfi1_devdata *dd, unsigned ctxt);
+int hfi1_clear_ctxt_pkey(struct hfi1_devdata *dd, struct hfi1_ctxtdata *ctxt);
 void hfi1_read_link_quality(struct hfi1_devdata *dd, u8 *link_quality);
 void hfi1_init_vnic_rsm(struct hfi1_devdata *dd);
 void hfi1_deinit_vnic_rsm(struct hfi1_devdata *dd);
drivers/infiniband/hw/hfi1/file_ops.c +1 -1
index 9574fc4..5a62440 100644
@@ -810,7 +810,7 @@ static int hfi1_file_close(struct inode *inode, struct file *fp)
 	dd->rcd[uctxt->ctxt] = NULL;
 
 	hfi1_user_exp_rcv_grp_free(uctxt);
-	hfi1_clear_ctxt_pkey(dd, uctxt->ctxt);
+	hfi1_clear_ctxt_pkey(dd, uctxt);
 
 	uctxt->rcvwait_to = 0;
 	uctxt->piowait_to = 0;
drivers/infiniband/hw/hfi1/vnic_main.c +1 -1
index 392f4d5..b1572c7 100644
@@ -209,7 +209,7 @@ static void deallocate_vnic_ctxt(struct hfi1_devdata *dd,
 	uctxt->event_flags = 0;
 
 	hfi1_clear_tids(uctxt);
-	hfi1_clear_ctxt_pkey(dd, uctxt->ctxt);
+	hfi1_clear_ctxt_pkey(dd, uctxt);
 
 	hfi1_stats.sps_ctxts--;
 	hfi1_free_ctxtdata(dd, uctxt);

--
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

#16 lore raw

PATCH v2 13/15 IB/hfi1: Clean up context initialization

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

Context initialization mixes base context init with sub context init. This is bad because contexts can be reused, and on reuse, reinit things that should not re-initialized.

Normalize comments and function names to refer to base context and sub context (not main, shared or slaves).

Separate the base context initialization from sub context initialization.

hfi1_init_ctxt() cannot return an error so changed to a void and remove error message.

---
 drivers/infiniband/hw/hfi1/chip.c         |    3 
 drivers/infiniband/hw/hfi1/chip.h         |    2 
 drivers/infiniband/hw/hfi1/file_ops.c     |  179 +++++++++++++++--------------
 drivers/infiniband/hw/hfi1/hfi.h          |   13 +-
 drivers/infiniband/hw/hfi1/init.c         |   10 --
 drivers/infiniband/hw/hfi1/trace_ctxts.h  |   17 ++-
 drivers/infiniband/hw/hfi1/user_exp_rcv.c |   84 +++++++-------
 drivers/infiniband/hw/hfi1/user_exp_rcv.h |    3 
 drivers/infiniband/hw/hfi1/vnic_main.c    |    6 -
 9 files changed, 159 insertions(+), 158 deletions(-)
drivers/infiniband/hw/hfi1/chip.c +1 -2
index 9f72d55..972da41 100644
@@ -12662,7 +12662,7 @@ u8 hfi1_ibphys_portstate(struct hfi1_pportdata *ppd)
 #define SET_STATIC_RATE_CONTROL_SMASK(r) \
 (r |= SEND_CTXT_CHECK_ENABLE_DISALLOW_PBC_STATIC_RATE_CONTROL_SMASK)
 
-int hfi1_init_ctxt(struct send_context *sc)
+void hfi1_init_ctxt(struct send_context *sc)
 {
 	if (sc) {
 		struct hfi1_devdata *dd = sc->dd;
@@ -12679,7 +12679,6 @@ int hfi1_init_ctxt(struct send_context *sc)
 		write_kctxt_csr(dd, sc->hw_context,
 				SEND_CTXT_CHECK_ENABLE, reg);
 	}
-	return 0;
 }
 
 int hfi1_tempsense_rd(struct hfi1_devdata *dd, struct hfi1_temp *temp)
drivers/infiniband/hw/hfi1/chip.h +1 -1
index 760aa2e..cbe455d 100644
@@ -1349,7 +1349,7 @@ enum {
 void hfi1_clear_tids(struct hfi1_ctxtdata *rcd);
 struct ib_header *hfi1_get_msgheader(
 				struct hfi1_devdata *dd, __le32 *rhf_addr);
-int hfi1_init_ctxt(struct send_context *sc);
+void hfi1_init_ctxt(struct send_context *sc);
 void hfi1_put_tid(struct hfi1_devdata *dd, u32 index,
 		  u32 type, unsigned long pa, u16 order);
 void hfi1_quiet_serdes(struct hfi1_pportdata *ppd);
drivers/infiniband/hw/hfi1/file_ops.c +90 -89

289 lines not on this page · raw message

drivers/infiniband/hw/hfi1/hfi.h +5 -8
index e0c4c5e..1b7203a 100644
@@ -224,13 +224,12 @@ struct hfi1_ctxtdata {
 	 * (ignoring forks, dup, etc. for now)
 	 */
 	int cnt;
+	/* Device context index */
+	unsigned ctxt;
 	/*
-	 * how much space to leave at start of eager TID entries for
-	 * protocol use, on each TID
+	 * non-zero if ctxt can be shared, and defines the maximum number of
+	 * sub contexts allowed.
 	 */
-	/* instead of calculating it */
-	unsigned ctxt;
-	/* non-zero if ctxt is being shared. */
 	u16 subctxt_cnt;
 	/* non-zero if ctxt is being shared. */
 	u16 subctxt_id;
@@ -1725,12 +1724,10 @@ struct cc_state *get_cc_state_protected(struct hfi1_pportdata *ppd)
 #define HFI1_PBC_LENGTH_MASK                     ((1 << 11) - 1)
 
 /* ctxt_flag bit offsets */
-		/* context has been setup */
-#define HFI1_CTXT_SETUP_DONE 1
 		/* waiting for a packet to arrive */
 #define HFI1_CTXT_WAITING_RCV   2
 		/* master has not finished initializing */
-#define HFI1_CTXT_MASTER_UNINIT 4
+#define HFI1_CTXT_BASE_UNINIT 4
 		/* waiting for an urgent packet to arrive */
 #define HFI1_CTXT_WAITING_URG 5
 
drivers/infiniband/hw/hfi1/init.c +2 -8
index e872644..52a6364 100644
@@ -176,13 +176,7 @@ int hfi1_create_ctxts(struct hfi1_devdata *dd)
 			goto nomem;
 		}
 
-		ret = hfi1_init_ctxt(rcd->sc);
-		if (ret < 0) {
-			dd_dev_err(dd,
-				   "Failed to setup kernel receive context, failing\n");
-			ret = -EFAULT;
-			goto bail;
-		}
+		hfi1_init_ctxt(rcd->sc);
 	}
 
 	/*
@@ -194,7 +188,7 @@ int hfi1_create_ctxts(struct hfi1_devdata *dd)
 	return 0;
 nomem:
 	ret = -ENOMEM;
-bail:
+
 	if (dd->rcd) {
 		for (i = 0; i < dd->num_rcv_contexts; ++i)
 			hfi1_free_ctxtdata(dd, dd->rcd[i]);
drivers/infiniband/hw/hfi1/trace_ctxts.h +12 -5
index 26ae789..4eb4cc7 100644
@@ -57,12 +57,14 @@
 
 #define UCTXT_FMT \
 	"cred:%u, credaddr:0x%llx, piobase:0x%p, rcvhdr_cnt:%u, "	\
-	"rcvbase:0x%llx, rcvegrc:%u, rcvegrb:0x%llx"
+	"rcvbase:0x%llx, rcvegrc:%u, rcvegrb:0x%llx, subctxt_cnt:%u"
 TRACE_EVENT(hfi1_uctxtdata,
-	    TP_PROTO(struct hfi1_devdata *dd, struct hfi1_ctxtdata *uctxt),
-	    TP_ARGS(dd, uctxt),
+	    TP_PROTO(struct hfi1_devdata *dd, struct hfi1_ctxtdata *uctxt,
+		     unsigned int subctxt),
+	    TP_ARGS(dd, uctxt, subctxt),
 	    TP_STRUCT__entry(DD_DEV_ENTRY(dd)
 			     __field(unsigned int, ctxt)
+			     __field(unsigned int, subctxt)
 			     __field(u32, credits)
 			     __field(u64, hw_free)
 			     __field(void __iomem *, piobase)
@@ -70,9 +72,11 @@
 			     __field(u64, rcvhdrq_dma)
 			     __field(u32, eager_cnt)
 			     __field(u64, rcvegr_dma)
+			     __field(unsigned int, subctxt_cnt)
 			     ),
 	    TP_fast_assign(DD_DEV_ASSIGN(dd);
 			   __entry->ctxt = uctxt->ctxt;
+			   __entry->subctxt = subctxt;
 			   __entry->credits = uctxt->sc->credits;
 			   __entry->hw_free = le64_to_cpu(*uctxt->sc->hw_free);
 			   __entry->piobase = uctxt->sc->base_addr;
@@ -80,17 +84,20 @@
 			   __entry->rcvhdrq_dma = uctxt->rcvhdrq_dma;
 			   __entry->eager_cnt = uctxt->egrbufs.alloced;
 			   __entry->rcvegr_dma = uctxt->egrbufs.rcvtids[0].dma;
+			   __entry->subctxt_cnt = uctxt->subctxt_cnt;
 			   ),
-	    TP_printk("[%s] ctxt %u " UCTXT_FMT,
+	    TP_printk("[%s] ctxt %u:%u " UCTXT_FMT,
 		      __get_str(dev),
 		      __entry->ctxt,
+		      __entry->subctxt,
 		      __entry->credits,
 		      __entry->hw_free,
 		      __entry->piobase,
 		      __entry->rcvhdrq_cnt,
 		      __entry->rcvhdrq_dma,
 		      __entry->eager_cnt,
-		      __entry->rcvegr_dma
+		      __entry->rcvegr_dma,
+		      __entry->subctxt_cnt
 		      )
 );
 
drivers/infiniband/hw/hfi1/user_exp_rcv.c +45 -39

131 lines not on this page · raw message

drivers/infiniband/hw/hfi1/user_exp_rcv.h +2 -1
index 9787511..5250c89 100644
@@ -71,8 +71,9 @@
 	} while (0)
 
 void hfi1_user_exp_rcv_grp_free(struct hfi1_ctxtdata *uctxt);
+int hfi1_user_exp_rcv_grp_init(struct hfi1_filedata *fd);
 int hfi1_user_exp_rcv_init(struct hfi1_filedata *fd);
-int hfi1_user_exp_rcv_free(struct hfi1_filedata *fd);
+void hfi1_user_exp_rcv_free(struct hfi1_filedata *fd);
 int hfi1_user_exp_rcv_setup(struct hfi1_filedata *fd,
 			    struct hfi1_tid_info *tinfo);
 int hfi1_user_exp_rcv_clear(struct hfi1_filedata *fd,
drivers/infiniband/hw/hfi1/vnic_main.c +1 -5
index b1572c7..b601c29 100644
@@ -67,9 +67,7 @@ static int setup_vnic_ctxt(struct hfi1_devdata *dd, struct hfi1_ctxtdata *uctxt)
 	unsigned int rcvctrl_ops = 0;
 	int ret;
 
-	ret = hfi1_init_ctxt(uctxt->sc);
-	if (ret)
-		goto done;
+	hfi1_init_ctxt(uctxt->sc);
 
 	uctxt->do_interrupt = &handle_receive_interrupt;
 
@@ -82,8 +80,6 @@ static int setup_vnic_ctxt(struct hfi1_devdata *dd, struct hfi1_ctxtdata *uctxt)
 	if (ret)
 		goto done;
 
-	set_bit(HFI1_CTXT_SETUP_DONE, &uctxt->event_flags);
-
 	if (uctxt->rcvhdrtail_kvaddr)
 		clear_rcvhdrtail(uctxt);
 

--
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

#17 lore raw

PATCH v2 14/15 IB/hfi1: Fix an assign/ordering issue with shared context IDs

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

The current algorithm for generating sub-context IDs is FILO. If the contexts are not closed in that order, the uniqueness of the ID will be compromised. I.e. logging the creation/deletion of context IDs with an application that assigns and closes in a FIFO order reveals:

cache_id: assign: uctxt: 3 sub_ctxt: 0
cache_id: assign: uctxt: 3 sub_ctxt: 1
cache_id: assign: uctxt: 3 sub_ctxt: 2
cache_id: close: uctxt: 3 sub_ctxt: 0
cache_id: assign: uctxt: 3 sub_ctxt: 2 <<<

The sub_ctxt ID 2 is reused incorrectly.

Update the sub-context ID assign algorithm to use a bitmask of in_use contexts. The new algorithm will allow the contexts to be closed in any order, and will only re-use unused contexts.

Size subctxt and subctxt_cnt to match the user API size.

---
 drivers/infiniband/hw/hfi1/driver.c    |    2 +
 drivers/infiniband/hw/hfi1/file_ops.c  |   51 +++++++++++++++++++-------------
 drivers/infiniband/hw/hfi1/hfi.h       |    8 +++--
 drivers/infiniband/hw/hfi1/init.c      |    3 +-
 drivers/infiniband/hw/hfi1/intr.c      |    3 +-
 drivers/infiniband/hw/hfi1/user_sdma.h |    2 +
 6 files changed, 41 insertions(+), 28 deletions(-)
drivers/infiniband/hw/hfi1/driver.c +1 -1
index 566d152..a50870e 100644
@@ -1289,7 +1289,7 @@ int hfi1_reset_device(int unit)
 	if (dd->rcd)
 		for (i = dd->first_dyn_alloc_ctxt;
 		     i < dd->num_rcv_contexts; i++) {
-			if (!dd->rcd[i] || !dd->rcd[i]->cnt)
+			if (!dd->rcd[i])
 				continue;
 			spin_unlock_irqrestore(&dd->uctxt_lock, flags);
 			ret = -EBUSY;
drivers/infiniband/hw/hfi1/file_ops.c +31 -20

136 lines not on this page · raw message

drivers/infiniband/hw/hfi1/hfi.h +4 -4
index 1b7203a..f3d75fc 100644
@@ -228,7 +228,7 @@ struct hfi1_ctxtdata {
 	unsigned ctxt;
 	/*
 	 * non-zero if ctxt can be shared, and defines the maximum number of
-	 * sub contexts allowed.
+	 * sub-contexts for this device context.
 	 */
 	u16 subctxt_cnt;
 	/* non-zero if ctxt is being shared. */
@@ -287,10 +287,10 @@ struct hfi1_ctxtdata {
 	void *subctxt_rcvegrbuf;
 	/* An array of pages for the eager header queue entries * N */
 	void *subctxt_rcvhdr_base;
+	/* Bitmask of in use context(s) */
+	DECLARE_BITMAP(in_use_ctxts, HFI1_MAX_SHARED_CTXTS);
 	/* The version of the library which opened this ctxt */
 	u32 userversion;
-	/* Bitmask of active slaves */
-	u32 active_slaves;
 	/* Type of packets or conditions we want to poll for */
 	u16 poll_type;
 	/* receive packet sequence counter */
@@ -1239,9 +1239,9 @@ static inline bool hfi1_vnic_is_rsm_full(struct hfi1_devdata *dd, int spare)
 struct hfi1_filedata {
 	struct hfi1_devdata *dd;
 	struct hfi1_ctxtdata *uctxt;
-	unsigned subctxt;
 	struct hfi1_user_sdma_comp_q *cq;
 	struct hfi1_user_sdma_pkt_q *pq;
+	u16 subctxt;
 	/* for cpu affinity; -1 if none */
 	int rec_cpu_num;
 	u32 tid_n_pinned;
drivers/infiniband/hw/hfi1/init.c +2 -1
index 52a6364..694a8ec 100644
@@ -53,6 +53,7 @@
 #include <linux/module.h>
 #include <linux/printk.h>
 #include <linux/hrtimer.h>
+#include <linux/bitmap.h>
 #include <rdma/rdma_vt.h>
 
 #include "hfi.h"
@@ -222,7 +223,7 @@ struct hfi1_ctxtdata *hfi1_create_ctxtdata(struct hfi1_pportdata *ppd, u32 ctxt,
 		INIT_LIST_HEAD(&rcd->qp_wait_list);
 		rcd->ppd = ppd;
 		rcd->dd = dd;
-		rcd->cnt = 1;
+		__set_bit(0, rcd->in_use_ctxts);
 		rcd->ctxt = ctxt;
 		dd->rcd[ctxt] = rcd;
 		rcd->numa_id = numa;
drivers/infiniband/hw/hfi1/intr.c +2 -1
index 232014d..ba265d0 100644
@@ -47,6 +47,7 @@
 
 #include <linux/pci.h>
 #include <linux/delay.h>
+#include <linux/bitmap.h>
 
 #include "hfi.h"
 #include "common.h"
@@ -189,7 +190,7 @@ void handle_user_interrupt(struct hfi1_ctxtdata *rcd)
 	unsigned long flags;
 
 	spin_lock_irqsave(&dd->uctxt_lock, flags);
-	if (!rcd->cnt)
+	if (bitmap_empty(rcd->in_use_ctxts, HFI1_MAX_SHARED_CTXTS))
 		goto done;
 
 	if (test_and_clear_bit(HFI1_CTXT_WAITING_RCV, &rcd->event_flags)) {
drivers/infiniband/hw/hfi1/user_sdma.h +1 -1
index 9181d7c..e5b10ae 100644
@@ -58,7 +58,7 @@
 struct hfi1_user_sdma_pkt_q {
 	struct list_head list;
 	unsigned ctxt;
-	unsigned subctxt;
+	u16 subctxt;
 	u16 n_max_reqs;
 	atomic_t n_reqs;
 	u16 reqidx;

--
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

#18 lore raw

PATCH v2 15/15 IB/hfi1: Clean up on context initialization failure

In reply to a message not in the archive

From: Michael J. Ruhl <[email protected]>

The error path for context initialization is not consistent. Cleanup all resources on failure.

Removed unused variable user_event_mask.

Add the _BASE_FAILED bit to the event flags so that a base context can notify waiting sub contexts that they cannot continue.

Running out of sub contexts is an EBUSY result, not EINVAL.

---
 drivers/infiniband/hw/hfi1/file_ops.c     |   62 ++++++++++++++++++-----------
 drivers/infiniband/hw/hfi1/hfi.h          |   16 +++----
 drivers/infiniband/hw/hfi1/init.c         |   10 ++---
 drivers/infiniband/hw/hfi1/user_exp_rcv.c |   31 ++++++++-------
 drivers/infiniband/hw/hfi1/user_sdma.c    |   61 ++++++++++++++---------------
 5 files changed, 95 insertions(+), 85 deletions(-)
drivers/infiniband/hw/hfi1/file_ops.c +38 -24
index 9c177ef..3158128 100644
@@ -82,7 +82,7 @@
 static int init_subctxts(struct hfi1_ctxtdata *uctxt,
 			 const struct hfi1_user_info *uinfo);
 static int init_user_ctxt(struct hfi1_filedata *fd);
-static int user_init(struct hfi1_ctxtdata *uctxt);
+static void user_init(struct hfi1_ctxtdata *uctxt);
 static int get_ctxt_info(struct hfi1_filedata *fd, void __user *ubase,
 			 __u32 len);
 static int get_base_info(struct hfi1_filedata *fd, void __user *ubase,
@@ -847,7 +847,7 @@ static u64 kvirt_to_phys(void *addr)
 
 static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
 {
-	int ret = 0;
+	int ret;
 	unsigned int swmajor, swminor;
 
 	swmajor = uinfo->userversion >> 16;
@@ -857,14 +857,16 @@ static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
 	swminor = uinfo->userversion & 0xffff;
 
 	mutex_lock(&hfi1_mutex);
-	/* First, lets check if we need to get a sub context? */
+	/*
+	 * Get a sub context if necessary.
+	 * ret < 0 error, 0 no context, 1 sub-context found
+	 */
+	ret = 0;
 	if (uinfo->subctxt_cnt) {
-		/* < 0 error, 0 no context, 1 sub-context found */
 		ret = find_sub_ctxt(fd, uinfo);
-		if (ret > 0) {
+		if (ret > 0)
 			fd->rec_cpu_num =
 				hfi1_get_proc_affinity(fd->uctxt->numa_id);
-		}
 	}
 
 	/*
@@ -885,17 +887,27 @@ static int assign_ctxt(struct hfi1_filedata *fd, struct hfi1_user_info *uinfo)
 		ret = wait_event_interruptible(fd->uctxt->wait, !test_bit(
 					       HFI1_CTXT_BASE_UNINIT,
 					       &fd->uctxt->event_flags));
+		if (test_bit(HFI1_CTXT_BASE_FAILED, &fd->uctxt->event_flags)) {
+			clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts);
+			return -ENOMEM;
+		}
 		/* The only thing a sub context needs is the user_xxx stuff */
 		if (!ret)
-			init_user_ctxt(fd);
+			ret = init_user_ctxt(fd);
+
+		if (ret)
+			clear_bit(fd->subctxt, fd->uctxt->in_use_ctxts);
 	} else if (!ret) {
 		ret = setup_base_ctxt(fd);
-
-		/*
-		 * Base context is done, notify anybody using a sub-context
-		 * that is waiting for this completion
-		 */
-		if (!ret && fd->uctxt->subctxt_cnt) {
+		if (fd->uctxt->subctxt_cnt) {
+			/* If there is an error, set the failed bit. */
+			if (ret)
+				set_bit(HFI1_CTXT_BASE_FAILED,
+					&fd->uctxt->event_flags);
+			/*
+			 * Base context is done, notify anybody using a
+			 * sub-context that is waiting for this completion
+			 */
 			clear_bit(HFI1_CTXT_BASE_UNINIT,
 				  &fd->uctxt->event_flags);
 			wake_up(&fd->uctxt->wait);
@@ -945,7 +957,7 @@ static int find_sub_ctxt(struct hfi1_filedata *fd,
 		subctxt = find_first_zero_bit(uctxt->in_use_ctxts,
 					      HFI1_MAX_SHARED_CTXTS);
 		if (subctxt >= uctxt->subctxt_cnt)
-			return -EINVAL;
+			return -EBUSY;
 
 		fd->uctxt = uctxt;
 		fd->subctxt = subctxt;
@@ -1118,7 +1130,7 @@ static int setup_subctxt(struct hfi1_ctxtdata *uctxt)
 	return ret;
 }
 
-static int user_init(struct hfi1_ctxtdata *uctxt)
+static void user_init(struct hfi1_ctxtdata *uctxt)
 {
 	unsigned int rcvctrl_ops = 0;
 
@@ -1168,8 +1180,6 @@ static int user_init(struct hfi1_ctxtdata *uctxt)
 	else
 		rcvctrl_ops |= HFI1_RCVCTRL_TAILUPD_DIS;
 	hfi1_rcvctrl(uctxt->dd, rcvctrl_ops, uctxt->ctxt);
-
-	return 0;
 }
 
 static int get_ctxt_info(struct hfi1_filedata *fd, void __user *ubase,
@@ -1238,28 +1248,32 @@ static int setup_base_ctxt(struct hfi1_filedata *fd)
 	/* Now allocate the RcvHdr queue and eager buffers. */
 	ret = hfi1_create_rcvhdrq(dd, uctxt);
 	if (ret)
-		goto done;
+		return ret;
 
 	ret = hfi1_setup_eagerbufs(uctxt);
 	if (ret)
-		goto done;
+		goto setup_failed;
 
 	/* If sub-contexts are enabled, do the appropriate setup */
 	if (uctxt->subctxt_cnt)
 		ret = setup_subctxt(uctxt);
 	if (ret)
-		goto done;
+		goto setup_failed;
 
 	ret = hfi1_user_exp_rcv_grp_init(fd);
 	if (ret)
-		goto done;
+		goto setup_failed;
 
 	ret = init_user_ctxt(fd);
 	if (ret)
-		goto done;
+		goto setup_failed;
 
-	ret = user_init(uctxt);
-done:
+	user_init(uctxt);
+
+	return 0;
+
+setup_failed:
+	hfi1_free_ctxtdata(dd, uctxt);
 	return ret;
 }
 
drivers/infiniband/hw/hfi1/hfi.h +6 -10
index f3d75fc..509df98 100644
@@ -196,12 +196,6 @@ struct hfi1_ctxtdata {
 	void *rcvhdrq;
 	/* kernel virtual address where hdrqtail is updated */
 	volatile __le64 *rcvhdrtail_kvaddr;
-	/*
-	 * Shared page for kernel to signal user processes that send buffers
-	 * need disarming.  The process should call HFI1_CMD_DISARM_BUFS
-	 * or HFI1_CMD_ACK_EVENT with IPATH_EVENT_DISARM_BUFS set.
-	 */
-	unsigned long *user_event_mask;
 	/* when waiting for rcv or pioavail */
 	wait_queue_head_t wait;
 	/* rcvhdrq size (for freeing) */
@@ -1724,12 +1718,14 @@ struct cc_state *get_cc_state_protected(struct hfi1_pportdata *ppd)
 #define HFI1_PBC_LENGTH_MASK                     ((1 << 11) - 1)
 
 /* ctxt_flag bit offsets */
+		/* base context has not finished initializing */
+#define HFI1_CTXT_BASE_UNINIT 1
+		/* base context initaliation failed */
+#define HFI1_CTXT_BASE_FAILED 2
 		/* waiting for a packet to arrive */
-#define HFI1_CTXT_WAITING_RCV   2
-		/* master has not finished initializing */
-#define HFI1_CTXT_BASE_UNINIT 4
+#define HFI1_CTXT_WAITING_RCV 3
 		/* waiting for an urgent packet to arrive */
-#define HFI1_CTXT_WAITING_URG 5
+#define HFI1_CTXT_WAITING_URG 4
 
 /* free up any allocated data at closes */
 struct hfi1_devdata *hfi1_init_dd(struct pci_dev *pdev,
drivers/infiniband/hw/hfi1/init.c +4 -6
index 694a8ec..4a11d4d 100644
@@ -964,7 +964,6 @@ void hfi1_free_ctxtdata(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd)
 	kfree(rcd->egrbufs.buffers);
 
 	sc_free(rcd->sc);
-	vfree(rcd->user_event_mask);
 	vfree(rcd->subctxt_uregbase);
 	vfree(rcd->subctxt_rcvegrbuf);
 	vfree(rcd->subctxt_rcvhdr_base);
@@ -1683,8 +1682,6 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd)
 	dd_dev_err(dd,
 		   "attempt to allocate 1 page for ctxt %u rcvhdrqtailaddr failed\n",
 		   rcd->ctxt);
-	vfree(rcd->user_event_mask);
-	rcd->user_event_mask = NULL;
 	dma_free_coherent(&dd->pcidev->dev, amt, rcd->rcvhdrq,
 			  rcd->rcvhdrq_dma);
 	rcd->rcvhdrq = NULL;
@@ -1851,7 +1848,7 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
 			  "ctxt%u: current Eager buffer size is invalid %u\n",
 			  rcd->ctxt, rcd->egrbufs.rcvtid_size);
 		ret = -EINVAL;
-		goto bail;
+		goto bail_rcvegrbuf_phys;
 	}
 
 	for (idx = 0; idx < rcd->egrbufs.alloced; idx++) {
@@ -1859,7 +1856,8 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
 			     rcd->egrbufs.rcvtids[idx].dma, order);
 		cond_resched();
 	}
-	goto bail;
+
+	return 0;
 
 bail_rcvegrbuf_phys:
 	for (idx = 0; idx < rcd->egrbufs.alloced &&
@@ -1873,6 +1871,6 @@ int hfi1_setup_eagerbufs(struct hfi1_ctxtdata *rcd)
 		rcd->egrbufs.buffers[idx].dma = 0;
 		rcd->egrbufs.buffers[idx].len = 0;
 	}
-bail:
+
 	return ret;
 }
drivers/infiniband/hw/hfi1/user_exp_rcv.c +17 -14
index 4c66f8d..a8f0aa4 100644
@@ -160,6 +160,7 @@ int hfi1_user_exp_rcv_grp_init(struct hfi1_filedata *fd)
 	struct hfi1_devdata *dd = fd->dd;
 	u32 tidbase;
 	u32 i;
+	struct tid_group *grp, *gptr;
 
 	exp_tid_group_init(&uctxt->tid_group_list);
 	exp_tid_group_init(&uctxt->tid_used_list);
@@ -168,17 +169,10 @@ int hfi1_user_exp_rcv_grp_init(struct hfi1_filedata *fd)
 	tidbase = uctxt->expected_base;
 	for (i = 0; i < uctxt->expected_count /
 		     dd->rcv_entries.group_size; i++) {
-		struct tid_group *grp;
-
 		grp = kzalloc(sizeof(*grp), GFP_KERNEL);
-		if (!grp) {
-			/*
-			 * If we fail here, the groups already
-			 * allocated will be freed by the close
-			 * call.
-			 */
-			return -ENOMEM;
-		}
+		if (!grp)
+			goto grp_failed;
+
 		grp->size = dd->rcv_entries.group_size;
 		grp->base = tidbase;
 		tid_group_add_tail(grp, &uctxt->tid_group_list);
@@ -186,6 +180,15 @@ int hfi1_user_exp_rcv_grp_init(struct hfi1_filedata *fd)
 	}
 
 	return 0;
+
+grp_failed:
+	list_for_each_entry_safe(grp, gptr, &uctxt->tid_group_list.list,
+				 list) {
+		list_del_init(&grp->list);
+		kfree(grp);
+	}
+
+	return -ENOMEM;
 }
 
 /*
@@ -213,11 +216,11 @@ int hfi1_user_exp_rcv_init(struct hfi1_filedata *fd)
 		fd->invalid_tids = kcalloc(uctxt->expected_count,
 					   sizeof(*fd->invalid_tids),
 					   GFP_KERNEL);
-		/*
-		 * NOTE: If this is an error, shouldn't we cleanup enry_to_rb?
-		 */
-		if (!fd->invalid_tids)
+		if (!fd->invalid_tids) {
+			kfree(fd->entry_to_rb);
+			fd->entry_to_rb = NULL;
 			return -ENOMEM;
+		}
 
 		/*
 		 * Register MMU notifier callbacks. If the registration
drivers/infiniband/hw/hfi1/user_sdma.c +30 -31

117 lines not on this page · raw message

--
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

#19 lore raw
In reply to a message not in the archive
Quoted 10 lines · Dennis Dalessandro

Doug,

This is a respin of [1] there were some merge problems with the last 5 in
this series, thanks to Leon for notifciing.

Included are some contex init/usage fixes, a couple performance tweaks, and some
general code clean up. This is all confined to the hfi1 driver.

Quoted 5 lines

I'm OK applying this to next vs the current merge window. We might want to pluck
out patches 6 and 7 for -rc. They are marked as stable and I think would meet
-rc criteria.

I think at least 14/15 should be -rc worthy too unless I read the problem description wrong.  In any case, I took the series.  I'm going to wait for 0day and linux-next before submitting to Linus (probably send it Monday or Tuesday of next week).

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