linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle
@ 2019-09-11 11:30 Dennis Dalessandro
  2019-09-11 11:30 ` [PATCH for-next 1/3] IB/hfi1: Add traces for TID RDMA READ Dennis Dalessandro
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Dennis Dalessandro @ 2019-09-11 11:30 UTC (permalink / raw)
  To: jgg, dledford; +Cc: linux-rdma

Here are some small patches for the next merge window. The traces patch requires
one that is already in 5.3-rc4 and not yet in for-next. There will be a conflict
otherwise. Easy to resolve but will also conflict when merge with Linus. The 
pre-req patch is: IB/hfi1: Unsafe PSN checking for TID RDMA READ Resp packet [1]

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=35d5c8b82e2c32e8e29ca195bb4dac60ba7d97fc

---

Ira Weiny (1):
      IB/hfi1: Define variables as unsigned long to fix KASAN warning

Kaike Wan (2):
      IB/hfi1: Add traces for TID RDMA READ
      IB/{rdmavt, hfi1, qib}: Add a counter for credit waits


 drivers/infiniband/hw/hfi1/chip.c      |    2 +
 drivers/infiniband/hw/hfi1/chip.h      |    1 +
 drivers/infiniband/hw/hfi1/mad.c       |   45 ++++++++++++++------------------
 drivers/infiniband/hw/hfi1/rc.c        |   15 +++++------
 drivers/infiniband/hw/hfi1/tid_rdma.c  |    8 ++++++
 drivers/infiniband/hw/hfi1/trace_tid.h |   38 +++++++++++++++++++++++++++
 drivers/infiniband/hw/qib/qib_rc.c     |   10 +------
 drivers/infiniband/hw/qib/qib_sysfs.c  |    2 +
 include/rdma/rdma_vt.h                 |    1 +
 include/rdma/rdmavt_qp.h               |   35 +++++++++++++++++++++++++
 10 files changed, 115 insertions(+), 42 deletions(-)

--
-Denny

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH for-next 1/3] IB/hfi1: Add traces for TID RDMA READ
  2019-09-11 11:30 [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Dennis Dalessandro
@ 2019-09-11 11:30 ` Dennis Dalessandro
  2019-09-11 11:30 ` [PATCH for-next 2/3] IB/{rdmavt, hfi1, qib}: Add a counter for credit waits Dennis Dalessandro
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Dennis Dalessandro @ 2019-09-11 11:30 UTC (permalink / raw)
  To: jgg, dledford; +Cc: linux-rdma, Mike Marciniszyn, Kaike Wan

From: Kaike Wan <kaike.wan@intel.com>

This patch adds traces to debug packet loss and retry for TID RDMA READ
protocol.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Kaike Wan <kaike.wan@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/infiniband/hw/hfi1/rc.c        |    5 ++++
 drivers/infiniband/hw/hfi1/tid_rdma.c  |    8 +++++++
 drivers/infiniband/hw/hfi1/trace_tid.h |   38 ++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index 024a7c2..eeca08d 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -1483,6 +1483,11 @@ static void update_num_rd_atomic(struct rvt_qp *qp, u32 psn,
 			req->ack_pending = cur_seg - req->comp_seg;
 			priv->pending_tid_r_segs += req->ack_pending;
 			qp->s_num_rd_atomic += req->ack_pending;
+			trace_hfi1_tid_req_update_num_rd_atomic(qp, 0,
+								wqe->wr.opcode,
+								wqe->psn,
+								wqe->lpsn,
+								req);
 		} else {
 			priv->pending_tid_r_segs += req->total_segs;
 			qp->s_num_rd_atomic += req->total_segs;
diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c b/drivers/infiniband/hw/hfi1/tid_rdma.c
index 04061d5..ab126b1 100644
--- a/drivers/infiniband/hw/hfi1/tid_rdma.c
+++ b/drivers/infiniband/hw/hfi1/tid_rdma.c
@@ -2687,6 +2687,9 @@ static bool handle_read_kdeth_eflags(struct hfi1_ctxtdata *rcd,
 	u32 fpsn;
 
 	lockdep_assert_held(&qp->r_lock);
+	trace_hfi1_rsp_read_kdeth_eflags(qp, ibpsn);
+	trace_hfi1_sender_read_kdeth_eflags(qp);
+	trace_hfi1_tid_read_sender_kdeth_eflags(qp, 0);
 	spin_lock(&qp->s_lock);
 	/* If the psn is out of valid range, drop the packet */
 	if (cmp_psn(ibpsn, qp->s_last_psn) < 0 ||
@@ -2748,6 +2751,8 @@ static bool handle_read_kdeth_eflags(struct hfi1_ctxtdata *rcd,
 		goto s_unlock;
 
 	req = wqe_to_tid_req(wqe);
+	trace_hfi1_tid_req_read_kdeth_eflags(qp, 0, wqe->wr.opcode, wqe->psn,
+					     wqe->lpsn, req);
 	switch (rcv_type) {
 	case RHF_RCV_TYPE_EXPECTED:
 		switch (rte) {
@@ -2762,6 +2767,9 @@ static bool handle_read_kdeth_eflags(struct hfi1_ctxtdata *rcd,
 			 * packets that could be still in the fabric.
 			 */
 			flow = &req->flows[req->clear_tail];
+			trace_hfi1_tid_flow_read_kdeth_eflags(qp,
+							      req->clear_tail,
+							      flow);
 			if (priv->s_flags & HFI1_R_TID_SW_PSN) {
 				diff = cmp_psn(psn,
 					       flow->flow_state.r_next_psn);
diff --git a/drivers/infiniband/hw/hfi1/trace_tid.h b/drivers/infiniband/hw/hfi1/trace_tid.h
index 4388b59..343fb98 100644
--- a/drivers/infiniband/hw/hfi1/trace_tid.h
+++ b/drivers/infiniband/hw/hfi1/trace_tid.h
@@ -627,6 +627,12 @@
 	TP_ARGS(qp, index, flow)
 );
 
+DEFINE_EVENT(/* event */
+	hfi1_tid_flow_template, hfi1_tid_flow_read_kdeth_eflags,
+	TP_PROTO(struct rvt_qp *qp, int index, struct tid_rdma_flow *flow),
+	TP_ARGS(qp, index, flow)
+);
+
 DECLARE_EVENT_CLASS(/* tid_node */
 	hfi1_tid_node_template,
 	TP_PROTO(struct rvt_qp *qp, const char *msg, u32 index, u32 base,
@@ -851,6 +857,12 @@
 	TP_ARGS(qp, psn)
 );
 
+DEFINE_EVENT(/* event */
+	hfi1_responder_info_template, hfi1_rsp_read_kdeth_eflags,
+	TP_PROTO(struct rvt_qp *qp, u32 psn),
+	TP_ARGS(qp, psn)
+);
+
 DECLARE_EVENT_CLASS(/* sender_info */
 	hfi1_sender_info_template,
 	TP_PROTO(struct rvt_qp *qp),
@@ -955,6 +967,12 @@
 	TP_ARGS(qp)
 );
 
+DEFINE_EVENT(/* event */
+	hfi1_sender_info_template, hfi1_sender_read_kdeth_eflags,
+	TP_PROTO(struct rvt_qp *qp),
+	TP_ARGS(qp)
+);
+
 DECLARE_EVENT_CLASS(/* tid_read_sender */
 	hfi1_tid_read_sender_template,
 	TP_PROTO(struct rvt_qp *qp, char newreq),
@@ -1015,6 +1033,12 @@
 	TP_ARGS(qp, newreq)
 );
 
+DEFINE_EVENT(/* event */
+	hfi1_tid_read_sender_template, hfi1_tid_read_sender_kdeth_eflags,
+	TP_PROTO(struct rvt_qp *qp, char newreq),
+	TP_ARGS(qp, newreq)
+);
+
 DECLARE_EVENT_CLASS(/* tid_rdma_request */
 	hfi1_tid_rdma_request_template,
 	TP_PROTO(struct rvt_qp *qp, char newreq, u8 opcode, u32 psn, u32 lpsn,
@@ -1216,6 +1240,13 @@
 );
 
 DEFINE_EVENT(/* event */
+	hfi1_tid_rdma_request_template, hfi1_tid_req_read_kdeth_eflags,
+	TP_PROTO(struct rvt_qp *qp, char newreq, u8 opcode, u32 psn, u32 lpsn,
+		 struct tid_rdma_request *req),
+	TP_ARGS(qp, newreq, opcode, psn, lpsn, req)
+);
+
+DEFINE_EVENT(/* event */
 	hfi1_tid_rdma_request_template, hfi1_tid_req_make_rc_ack_write,
 	TP_PROTO(struct rvt_qp *qp, char newreq, u8 opcode, u32 psn, u32 lpsn,
 		 struct tid_rdma_request *req),
@@ -1229,6 +1260,13 @@
 	TP_ARGS(qp, newreq, opcode, psn, lpsn, req)
 );
 
+DEFINE_EVENT(/* event */
+	hfi1_tid_rdma_request_template, hfi1_tid_req_update_num_rd_atomic,
+	TP_PROTO(struct rvt_qp *qp, char newreq, u8 opcode, u32 psn, u32 lpsn,
+		 struct tid_rdma_request *req),
+	TP_ARGS(qp, newreq, opcode, psn, lpsn, req)
+);
+
 DECLARE_EVENT_CLASS(/* rc_rcv_err */
 	hfi1_rc_rcv_err_template,
 	TP_PROTO(struct rvt_qp *qp, u32 opcode, u32 psn, int diff),


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH for-next 2/3] IB/{rdmavt, hfi1, qib}: Add a counter for credit waits
  2019-09-11 11:30 [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Dennis Dalessandro
  2019-09-11 11:30 ` [PATCH for-next 1/3] IB/hfi1: Add traces for TID RDMA READ Dennis Dalessandro
@ 2019-09-11 11:30 ` Dennis Dalessandro
  2019-09-11 11:30 ` [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning Dennis Dalessandro
  2019-09-16 13:58 ` [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Jason Gunthorpe
  3 siblings, 0 replies; 7+ messages in thread
From: Dennis Dalessandro @ 2019-09-11 11:30 UTC (permalink / raw)
  To: jgg, dledford; +Cc: linux-rdma, Mike Marciniszyn, Kaike Wan

From: Kaike Wan <kaike.wan@intel.com>

This patch adds a counter for credit waits to assist field debugging.

Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Kaike Wan <kaike.wan@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/infiniband/hw/hfi1/chip.c     |    2 ++
 drivers/infiniband/hw/hfi1/chip.h     |    1 +
 drivers/infiniband/hw/hfi1/rc.c       |   10 ++-------
 drivers/infiniband/hw/qib/qib_rc.c    |   10 ++-------
 drivers/infiniband/hw/qib/qib_sysfs.c |    2 ++
 include/rdma/rdma_vt.h                |    1 +
 include/rdma/rdmavt_qp.h              |   35 +++++++++++++++++++++++++++++++++
 7 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 67052dc..9b1fb84 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -4101,6 +4101,7 @@ static u64 access_dc_rcv_err_cnt(const struct cntr_entry *entry,
 def_access_ibp_counter(rdma_seq);
 def_access_ibp_counter(unaligned);
 def_access_ibp_counter(seq_naks);
+def_access_ibp_counter(rc_crwaits);
 
 static struct cntr_entry dev_cntrs[DEV_CNTR_LAST] = {
 [C_RCV_OVF] = RXE32_DEV_CNTR_ELEM(RcvOverflow, RCV_BUF_OVFL_CNT, CNTR_SYNTH),
@@ -5119,6 +5120,7 @@ static u64 access_dc_rcv_err_cnt(const struct cntr_entry *entry,
 [C_SW_IBP_RDMA_SEQ] = SW_IBP_CNTR(RdmaSeq, rdma_seq),
 [C_SW_IBP_UNALIGNED] = SW_IBP_CNTR(Unaligned, unaligned),
 [C_SW_IBP_SEQ_NAK] = SW_IBP_CNTR(SeqNak, seq_naks),
+[C_SW_IBP_RC_CRWAITS] = SW_IBP_CNTR(RcCrWait, rc_crwaits),
 [C_SW_CPU_RC_ACKS] = CNTR_ELEM("RcAcks", 0, 0, CNTR_NORMAL,
 			       access_sw_cpu_rc_acks),
 [C_SW_CPU_RC_QACKS] = CNTR_ELEM("RcQacks", 0, 0, CNTR_NORMAL,
diff --git a/drivers/infiniband/hw/hfi1/chip.h b/drivers/infiniband/hw/hfi1/chip.h
index b76cf81..4ca5ac8 100644
--- a/drivers/infiniband/hw/hfi1/chip.h
+++ b/drivers/infiniband/hw/hfi1/chip.h
@@ -1245,6 +1245,7 @@ enum {
 	C_SW_IBP_RDMA_SEQ,
 	C_SW_IBP_UNALIGNED,
 	C_SW_IBP_SEQ_NAK,
+	C_SW_IBP_RC_CRWAITS,
 	C_SW_CPU_RC_ACKS,
 	C_SW_CPU_RC_QACKS,
 	C_SW_CPU_RC_DELAYED_COMP,
diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index eeca08d..513a8aa 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -595,11 +595,8 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 		case IB_WR_SEND_WITH_IMM:
 		case IB_WR_SEND_WITH_INV:
 			/* If no credit, return. */
-			if (!(qp->s_flags & RVT_S_UNLIMITED_CREDIT) &&
-			    rvt_cmp_msn(wqe->ssn, qp->s_lsn + 1) > 0) {
-				qp->s_flags |= RVT_S_WAIT_SSN_CREDIT;
+			if (!rvt_rc_credit_avail(qp, wqe))
 				goto bail;
-			}
 			if (len > pmtu) {
 				qp->s_state = OP(SEND_FIRST);
 				len = pmtu;
@@ -632,11 +629,8 @@ int hfi1_make_rc_req(struct rvt_qp *qp, struct hfi1_pkt_state *ps)
 			goto no_flow_control;
 		case IB_WR_RDMA_WRITE_WITH_IMM:
 			/* If no credit, return. */
-			if (!(qp->s_flags & RVT_S_UNLIMITED_CREDIT) &&
-			    rvt_cmp_msn(wqe->ssn, qp->s_lsn + 1) > 0) {
-				qp->s_flags |= RVT_S_WAIT_SSN_CREDIT;
+			if (!rvt_rc_credit_avail(qp, wqe))
 				goto bail;
-			}
 no_flow_control:
 			put_ib_reth_vaddr(
 				wqe->rdma_wr.remote_addr,
diff --git a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
index 1d5e2d4..aaf7438 100644
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -313,11 +313,8 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 		case IB_WR_SEND:
 		case IB_WR_SEND_WITH_IMM:
 			/* If no credit, return. */
-			if (!(qp->s_flags & RVT_S_UNLIMITED_CREDIT) &&
-			    rvt_cmp_msn(wqe->ssn, qp->s_lsn + 1) > 0) {
-				qp->s_flags |= RVT_S_WAIT_SSN_CREDIT;
+			if (!rvt_rc_credit_avail(qp, wqe))
 				goto bail;
-			}
 			if (len > pmtu) {
 				qp->s_state = OP(SEND_FIRST);
 				len = pmtu;
@@ -344,11 +341,8 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 			goto no_flow_control;
 		case IB_WR_RDMA_WRITE_WITH_IMM:
 			/* If no credit, return. */
-			if (!(qp->s_flags & RVT_S_UNLIMITED_CREDIT) &&
-			    rvt_cmp_msn(wqe->ssn, qp->s_lsn + 1) > 0) {
-				qp->s_flags |= RVT_S_WAIT_SSN_CREDIT;
+			if (!rvt_rc_credit_avail(qp, wqe))
 				goto bail;
-			}
 no_flow_control:
 			ohdr->u.rc.reth.vaddr =
 				cpu_to_be64(wqe->rdma_wr.remote_addr);
diff --git a/drivers/infiniband/hw/qib/qib_sysfs.c b/drivers/infiniband/hw/qib/qib_sysfs.c
index 905206a..3926be7 100644
--- a/drivers/infiniband/hw/qib/qib_sysfs.c
+++ b/drivers/infiniband/hw/qib/qib_sysfs.c
@@ -436,6 +436,7 @@ struct qib_diagc_attr {
 QIB_DIAGC_ATTR(unaligned);
 QIB_DIAGC_ATTR(rc_dupreq);
 QIB_DIAGC_ATTR(rc_seqnak);
+QIB_DIAGC_ATTR(rc_crwaits);
 
 static struct attribute *diagc_default_attributes[] = {
 	&qib_diagc_attr_rc_resends.attr,
@@ -453,6 +454,7 @@ struct qib_diagc_attr {
 	&qib_diagc_attr_unaligned.attr,
 	&qib_diagc_attr_rc_dupreq.attr,
 	&qib_diagc_attr_rc_seqnak.attr,
+	&qib_diagc_attr_rc_crwaits.attr,
 	NULL
 };
 
diff --git a/include/rdma/rdma_vt.h b/include/rdma/rdma_vt.h
index 525848e..ac5a943 100644
--- a/include/rdma/rdma_vt.h
+++ b/include/rdma/rdma_vt.h
@@ -116,6 +116,7 @@ struct rvt_ibport {
 	u64 n_unaligned;
 	u64 n_rc_dupreq;
 	u64 n_rc_seqnak;
+	u64 n_rc_crwaits;
 	u16 pkey_violations;
 	u16 qkey_violations;
 	u16 mkey_violations;
diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h
index e06c77d..b550ae8 100644
--- a/include/rdma/rdmavt_qp.h
+++ b/include/rdma/rdmavt_qp.h
@@ -973,6 +973,41 @@ static inline void rvt_free_rq(struct rvt_rq *rq)
 	rq->wq = NULL;
 }
 
+/**
+ * rvt_to_iport - Get the ibport pointer
+ * @qp: the qp pointer
+ *
+ * This function returns the ibport pointer from the qp pointer.
+ */
+static inline struct rvt_ibport *rvt_to_iport(struct rvt_qp *qp)
+{
+	struct rvt_dev_info *rdi = ib_to_rvt(qp->ibqp.device);
+
+	return rdi->ports[qp->port_num - 1];
+}
+
+/**
+ * rvt_rc_credit_avail - Check if there are enough RC credits for the request
+ * @qp: the qp
+ * @wqe: the request
+ *
+ * This function returns false when there are not enough credits for the given
+ * request and true otherwise.
+ */
+static inline bool rvt_rc_credit_avail(struct rvt_qp *qp, struct rvt_swqe *wqe)
+{
+	lockdep_assert_held(&qp->s_lock);
+	if (!(qp->s_flags & RVT_S_UNLIMITED_CREDIT) &&
+	    rvt_cmp_msn(wqe->ssn, qp->s_lsn + 1) > 0) {
+		struct rvt_ibport *rvp = rvt_to_iport(qp);
+
+		qp->s_flags |= RVT_S_WAIT_SSN_CREDIT;
+		rvp->n_rc_crwaits++;
+		return false;
+	}
+	return true;
+}
+
 struct rvt_qp_iter *rvt_qp_iter_init(struct rvt_dev_info *rdi,
 				     u64 v,
 				     void (*cb)(struct rvt_qp *qp, u64 v));


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning
  2019-09-11 11:30 [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Dennis Dalessandro
  2019-09-11 11:30 ` [PATCH for-next 1/3] IB/hfi1: Add traces for TID RDMA READ Dennis Dalessandro
  2019-09-11 11:30 ` [PATCH for-next 2/3] IB/{rdmavt, hfi1, qib}: Add a counter for credit waits Dennis Dalessandro
@ 2019-09-11 11:30 ` Dennis Dalessandro
  2019-09-13 12:05   ` Jason Gunthorpe
  2019-09-16 13:58 ` [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Jason Gunthorpe
  3 siblings, 1 reply; 7+ messages in thread
From: Dennis Dalessandro @ 2019-09-11 11:30 UTC (permalink / raw)
  To: jgg, dledford; +Cc: linux-rdma, Mike Marciniszyn, Ira Weiny, stable, Kaike Wan

From: Ira Weiny <ira.weiny@intel.com>

Define the working variables to be unsigned long to be compatible with
for_each_set_bit and change types as needed.

While we are at it remove unused variables from a couple of functions.

This was found because of the following KASAN warning:
[ 1383.018461] ==================================================================
[ 1383.029116] BUG: KASAN: stack-out-of-bounds in find_first_bit+0x19/0x70
[ 1383.038646] Read of size 8 at addr ffff888362d778d0 by task kworker/u308:2/1889
[ 1383.049006]
[ 1383.052766] CPU: 21 PID: 1889 Comm: kworker/u308:2 Tainted: G W         5.3.0-rc2-mm1+ #2
[ 1383.064765] Hardware name: Intel Corporation W2600CR/W2600CR, BIOS SE5C600.86B.02.04.0003.102320141138 10/23/2014
[ 1383.078498] Workqueue: ib-comp-unb-wq ib_cq_poll_work [ib_core]
[ 1383.087314] Call Trace:
[ 1383.092211]  dump_stack+0x9a/0xf0
[ 1383.098074]  ? find_first_bit+0x19/0x70
[ 1383.104546]  print_address_description+0x6c/0x332
[ 1383.111986]  ? find_first_bit+0x19/0x70
[ 1383.118443]  ? find_first_bit+0x19/0x70
[ 1383.124882]  __kasan_report.cold.6+0x1a/0x3b
[ 1383.131808]  ? find_first_bit+0x19/0x70
[ 1383.138244]  kasan_report+0xe/0x12
[ 1383.144169]  find_first_bit+0x19/0x70
[ 1383.150413]  pma_get_opa_portstatus+0x5cc/0xa80 [hfi1]
[ 1383.158275]  ? ret_from_fork+0x3a/0x50
[ 1383.164583]  ? pma_get_opa_port_ectrs+0x200/0x200 [hfi1]
[ 1383.172629]  ? stack_trace_consume_entry+0x80/0x80
[ 1383.180097]  hfi1_process_mad+0x39b/0x26c0 [hfi1]
[ 1383.187420]  ? __lock_acquire+0x65e/0x21b0
[ 1383.194081]  ? clear_linkup_counters+0xb0/0xb0 [hfi1]
[ 1383.201788]  ? check_chain_key+0x1d7/0x2e0
[ 1383.208391]  ? lock_downgrade+0x3a0/0x3a0
[ 1383.214906]  ? match_held_lock+0x2e/0x250
[ 1383.221479]  ib_mad_recv_done+0x698/0x15e0 [ib_core]
[ 1383.229187]  ? clear_linkup_counters+0xb0/0xb0 [hfi1]
[ 1383.236977]  ? ib_mad_send_done+0xc80/0xc80 [ib_core]
[ 1383.244778]  ? mark_held_locks+0x79/0xa0
[ 1383.251341]  ? _raw_spin_unlock_irqrestore+0x44/0x60
[ 1383.259086]  ? rvt_poll_cq+0x1e1/0x340 [rdmavt]
[ 1383.266362]  __ib_process_cq+0x97/0x100 [ib_core]
[ 1383.273822]  ib_cq_poll_work+0x31/0xb0 [ib_core]
[ 1383.281169]  process_one_work+0x4ee/0xa00
[ 1383.287822]  ? pwq_dec_nr_in_flight+0x110/0x110
[ 1383.295047]  ? do_raw_spin_lock+0x113/0x1d0
[ 1383.301880]  worker_thread+0x57/0x5a0
[ 1383.308138]  ? process_one_work+0xa00/0xa00
[ 1383.314937]  kthread+0x1bb/0x1e0
[ 1383.320643]  ? kthread_create_on_node+0xc0/0xc0
[ 1383.327822]  ret_from_fork+0x3a/0x50
[ 1383.333928]
[ 1383.337640] The buggy address belongs to the page:
[ 1383.345080] page:ffffea000d8b5dc0 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0
[ 1383.356490] flags: 0x17ffffc0000000()
[ 1383.362705] raw: 0017ffffc0000000 0000000000000000 ffffea000d8b5dc8 0000000000000000
[ 1383.373530] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
[ 1383.384321] page dumped because: kasan: bad access detected
[ 1383.392659]
[ 1383.396350] addr ffff888362d778d0 is located in stack of task kworker/u308:2/1889 at offset 32 in frame:
[ 1383.409191]  pma_get_opa_portstatus+0x0/0xa80 [hfi1]
[ 1383.416843]
[ 1383.420497] this frame has 1 object:
[ 1383.426494]  [32, 36) 'vl_select_mask'
[ 1383.426495]
[ 1383.436313] Memory state around the buggy address:
[ 1383.443706]  ffff888362d77780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1383.453865]  ffff888362d77800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1383.464033] >ffff888362d77880: 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 f2 f2 00 00
[ 1383.474199]                                                  ^
[ 1383.482828]  ffff888362d77900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 1383.493071]  ffff888362d77980: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 f2 f2
[ 1383.503314]
==================================================================

Cc: <stable@vger.kernel.org>
Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Kaike Wan <kaike.wan@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/infiniband/hw/hfi1/mad.c |   45 ++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
index 184dba3..d8ff063 100644
--- a/drivers/infiniband/hw/hfi1/mad.c
+++ b/drivers/infiniband/hw/hfi1/mad.c
@@ -2326,7 +2326,7 @@ struct opa_port_status_req {
 	__be32 vl_select_mask;
 };
 
-#define VL_MASK_ALL		0x000080ff
+#define VL_MASK_ALL		0x00000000000080ffUL
 
 struct opa_port_status_rsp {
 	__u8 port_num;
@@ -2625,15 +2625,14 @@ static int pma_get_opa_classportinfo(struct opa_pma_mad *pmp,
 }
 
 static void a0_portstatus(struct hfi1_pportdata *ppd,
-			  struct opa_port_status_rsp *rsp, u32 vl_select_mask)
+			  struct opa_port_status_rsp *rsp)
 {
 	if (!is_bx(ppd->dd)) {
 		unsigned long vl;
 		u64 sum_vl_xmit_wait = 0;
-		u32 vl_all_mask = VL_MASK_ALL;
+		unsigned long vl_all_mask = VL_MASK_ALL;
 
-		for_each_set_bit(vl, (unsigned long *)&(vl_all_mask),
-				 8 * sizeof(vl_all_mask)) {
+		for_each_set_bit(vl, &vl_all_mask, BITS_PER_LONG) {
 			u64 tmp = sum_vl_xmit_wait +
 				  read_port_cntr(ppd, C_TX_WAIT_VL,
 						 idx_from_vl(vl));
@@ -2730,12 +2729,12 @@ static int pma_get_opa_portstatus(struct opa_pma_mad *pmp,
 		(struct opa_port_status_req *)pmp->data;
 	struct hfi1_devdata *dd = dd_from_ibdev(ibdev);
 	struct opa_port_status_rsp *rsp;
-	u32 vl_select_mask = be32_to_cpu(req->vl_select_mask);
+	unsigned long vl_select_mask = be32_to_cpu(req->vl_select_mask);
 	unsigned long vl;
 	size_t response_data_size;
 	u32 nports = be32_to_cpu(pmp->mad_hdr.attr_mod) >> 24;
 	u8 port_num = req->port_num;
-	u8 num_vls = hweight32(vl_select_mask);
+	u8 num_vls = hweight64(vl_select_mask);
 	struct _vls_pctrs *vlinfo;
 	struct hfi1_ibport *ibp = to_iport(ibdev, port);
 	struct hfi1_pportdata *ppd = ppd_from_ibp(ibp);
@@ -2770,7 +2769,7 @@ static int pma_get_opa_portstatus(struct opa_pma_mad *pmp,
 
 	hfi1_read_link_quality(dd, &rsp->link_quality_indicator);
 
-	rsp->vl_select_mask = cpu_to_be32(vl_select_mask);
+	rsp->vl_select_mask = cpu_to_be32((u32)vl_select_mask);
 	rsp->port_xmit_data = cpu_to_be64(read_dev_cntr(dd, C_DC_XMIT_FLITS,
 					  CNTR_INVALID_VL));
 	rsp->port_rcv_data = cpu_to_be64(read_dev_cntr(dd, C_DC_RCV_FLITS,
@@ -2841,8 +2840,7 @@ static int pma_get_opa_portstatus(struct opa_pma_mad *pmp,
 	 * So in the for_each_set_bit() loop below, we don't need
 	 * any additional checks for vl.
 	 */
-	for_each_set_bit(vl, (unsigned long *)&(vl_select_mask),
-			 8 * sizeof(vl_select_mask)) {
+	for_each_set_bit(vl, &vl_select_mask, BITS_PER_LONG) {
 		memset(vlinfo, 0, sizeof(*vlinfo));
 
 		tmp = read_dev_cntr(dd, C_DC_RX_FLIT_VL, idx_from_vl(vl));
@@ -2883,7 +2881,7 @@ static int pma_get_opa_portstatus(struct opa_pma_mad *pmp,
 		vfi++;
 	}
 
-	a0_portstatus(ppd, rsp, vl_select_mask);
+	a0_portstatus(ppd, rsp);
 
 	if (resp_len)
 		*resp_len += response_data_size;
@@ -2930,16 +2928,14 @@ static u64 get_error_counter_summary(struct ib_device *ibdev, u8 port,
 	return error_counter_summary;
 }
 
-static void a0_datacounters(struct hfi1_pportdata *ppd, struct _port_dctrs *rsp,
-			    u32 vl_select_mask)
+static void a0_datacounters(struct hfi1_pportdata *ppd, struct _port_dctrs *rsp)
 {
 	if (!is_bx(ppd->dd)) {
 		unsigned long vl;
 		u64 sum_vl_xmit_wait = 0;
-		u32 vl_all_mask = VL_MASK_ALL;
+		unsigned long vl_all_mask = VL_MASK_ALL;
 
-		for_each_set_bit(vl, (unsigned long *)&(vl_all_mask),
-				 8 * sizeof(vl_all_mask)) {
+		for_each_set_bit(vl, &vl_all_mask, BITS_PER_LONG) {
 			u64 tmp = sum_vl_xmit_wait +
 				  read_port_cntr(ppd, C_TX_WAIT_VL,
 						 idx_from_vl(vl));
@@ -2994,7 +2990,7 @@ static int pma_get_opa_datacounters(struct opa_pma_mad *pmp,
 	u64 port_mask;
 	u8 port_num;
 	unsigned long vl;
-	u32 vl_select_mask;
+	unsigned long vl_select_mask;
 	int vfi;
 	u16 link_width;
 	u16 link_speed;
@@ -3071,8 +3067,7 @@ static int pma_get_opa_datacounters(struct opa_pma_mad *pmp,
 	 * So in the for_each_set_bit() loop below, we don't need
 	 * any additional checks for vl.
 	 */
-	for_each_set_bit(vl, (unsigned long *)&(vl_select_mask),
-			 8 * sizeof(req->vl_select_mask)) {
+	for_each_set_bit(vl, &vl_select_mask, BITS_PER_LONG) {
 		memset(vlinfo, 0, sizeof(*vlinfo));
 
 		rsp->vls[vfi].port_vl_xmit_data =
@@ -3120,7 +3115,7 @@ static int pma_get_opa_datacounters(struct opa_pma_mad *pmp,
 		vfi++;
 	}
 
-	a0_datacounters(ppd, rsp, vl_select_mask);
+	a0_datacounters(ppd, rsp);
 
 	if (resp_len)
 		*resp_len += response_data_size;
@@ -3215,7 +3210,7 @@ static int pma_get_opa_porterrors(struct opa_pma_mad *pmp,
 	struct _vls_ectrs *vlinfo;
 	unsigned long vl;
 	u64 port_mask, tmp;
-	u32 vl_select_mask;
+	unsigned long vl_select_mask;
 	int vfi;
 
 	req = (struct opa_port_error_counters64_msg *)pmp->data;
@@ -3273,8 +3268,7 @@ static int pma_get_opa_porterrors(struct opa_pma_mad *pmp,
 	vlinfo = &rsp->vls[0];
 	vfi = 0;
 	vl_select_mask = be32_to_cpu(req->vl_select_mask);
-	for_each_set_bit(vl, (unsigned long *)&(vl_select_mask),
-			 8 * sizeof(req->vl_select_mask)) {
+	for_each_set_bit(vl, &vl_select_mask, BITS_PER_LONG) {
 		memset(vlinfo, 0, sizeof(*vlinfo));
 		rsp->vls[vfi].port_vl_xmit_discards =
 			cpu_to_be64(read_port_cntr(ppd, C_SW_XMIT_DSCD_VL,
@@ -3485,7 +3479,7 @@ static int pma_set_opa_portstatus(struct opa_pma_mad *pmp,
 	u32 nports = be32_to_cpu(pmp->mad_hdr.attr_mod) >> 24;
 	u64 portn = be64_to_cpu(req->port_select_mask[3]);
 	u32 counter_select = be32_to_cpu(req->counter_select_mask);
-	u32 vl_select_mask = VL_MASK_ALL; /* clear all per-vl cnts */
+	unsigned long vl_select_mask = VL_MASK_ALL; /* clear all per-vl cnts */
 	unsigned long vl;
 
 	if ((nports != 1) || (portn != 1 << port)) {
@@ -3579,8 +3573,7 @@ static int pma_set_opa_portstatus(struct opa_pma_mad *pmp,
 	if (counter_select & CS_UNCORRECTABLE_ERRORS)
 		write_dev_cntr(dd, C_DC_UNC_ERR, CNTR_INVALID_VL, 0);
 
-	for_each_set_bit(vl, (unsigned long *)&(vl_select_mask),
-			 8 * sizeof(vl_select_mask)) {
+	for_each_set_bit(vl, &vl_select_mask, BITS_PER_LONG) {
 		if (counter_select & CS_PORT_XMIT_DATA)
 			write_port_cntr(ppd, C_TX_FLIT_VL, idx_from_vl(vl), 0);
 


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning
  2019-09-11 11:30 ` [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning Dennis Dalessandro
@ 2019-09-13 12:05   ` Jason Gunthorpe
  2019-09-13 12:43     ` Marciniszyn, Mike
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Gunthorpe @ 2019-09-13 12:05 UTC (permalink / raw)
  To: Dennis Dalessandro
  Cc: dledford, linux-rdma, Mike Marciniszyn, Ira Weiny, stable, Kaike Wan

On Wed, Sep 11, 2019 at 07:30:53AM -0400, Dennis Dalessandro wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Define the working variables to be unsigned long to be compatible with
> for_each_set_bit and change types as needed.
> 
> While we are at it remove unused variables from a couple of functions.
> 
> This was found because of the following KASAN warning:
> [ 1383.018461] ==================================================================
> [ 1383.029116] BUG: KASAN: stack-out-of-bounds in find_first_bit+0x19/0x70
> [ 1383.038646] Read of size 8 at addr ffff888362d778d0 by task kworker/u308:2/1889
> [ 1383.049006]
> [ 1383.052766] CPU: 21 PID: 1889 Comm: kworker/u308:2 Tainted: G W         5.3.0-rc2-mm1+ #2
> [ 1383.064765] Hardware name: Intel Corporation W2600CR/W2600CR, BIOS SE5C600.86B.02.04.0003.102320141138 10/23/2014
> [ 1383.078498] Workqueue: ib-comp-unb-wq ib_cq_poll_work [ib_core]
> [ 1383.087314] Call Trace:
> [ 1383.092211]  dump_stack+0x9a/0xf0
> [ 1383.098074]  ? find_first_bit+0x19/0x70
> [ 1383.104546]  print_address_description+0x6c/0x332
> [ 1383.111986]  ? find_first_bit+0x19/0x70
> [ 1383.118443]  ? find_first_bit+0x19/0x70
> [ 1383.124882]  __kasan_report.cold.6+0x1a/0x3b
> [ 1383.131808]  ? find_first_bit+0x19/0x70
> [ 1383.138244]  kasan_report+0xe/0x12
> [ 1383.144169]  find_first_bit+0x19/0x70
> [ 1383.150413]  pma_get_opa_portstatus+0x5cc/0xa80 [hfi1]
> [ 1383.158275]  ? ret_from_fork+0x3a/0x50
> [ 1383.164583]  ? pma_get_opa_port_ectrs+0x200/0x200 [hfi1]
> [ 1383.172629]  ? stack_trace_consume_entry+0x80/0x80
> [ 1383.180097]  hfi1_process_mad+0x39b/0x26c0 [hfi1]
> [ 1383.187420]  ? __lock_acquire+0x65e/0x21b0
> [ 1383.194081]  ? clear_linkup_counters+0xb0/0xb0 [hfi1]
> [ 1383.201788]  ? check_chain_key+0x1d7/0x2e0
> [ 1383.208391]  ? lock_downgrade+0x3a0/0x3a0
> [ 1383.214906]  ? match_held_lock+0x2e/0x250
> [ 1383.221479]  ib_mad_recv_done+0x698/0x15e0 [ib_core]
> [ 1383.229187]  ? clear_linkup_counters+0xb0/0xb0 [hfi1]
> [ 1383.236977]  ? ib_mad_send_done+0xc80/0xc80 [ib_core]
> [ 1383.244778]  ? mark_held_locks+0x79/0xa0
> [ 1383.251341]  ? _raw_spin_unlock_irqrestore+0x44/0x60
> [ 1383.259086]  ? rvt_poll_cq+0x1e1/0x340 [rdmavt]
> [ 1383.266362]  __ib_process_cq+0x97/0x100 [ib_core]
> [ 1383.273822]  ib_cq_poll_work+0x31/0xb0 [ib_core]
> [ 1383.281169]  process_one_work+0x4ee/0xa00
> [ 1383.287822]  ? pwq_dec_nr_in_flight+0x110/0x110
> [ 1383.295047]  ? do_raw_spin_lock+0x113/0x1d0
> [ 1383.301880]  worker_thread+0x57/0x5a0
> [ 1383.308138]  ? process_one_work+0xa00/0xa00
> [ 1383.314937]  kthread+0x1bb/0x1e0
> [ 1383.320643]  ? kthread_create_on_node+0xc0/0xc0
> [ 1383.327822]  ret_from_fork+0x3a/0x50
> [ 1383.333928]
> [ 1383.337640] The buggy address belongs to the page:
> [ 1383.345080] page:ffffea000d8b5dc0 refcount:0 mapcount:0 mapping:0000000000000000 index:0x0
> [ 1383.356490] flags: 0x17ffffc0000000()
> [ 1383.362705] raw: 0017ffffc0000000 0000000000000000 ffffea000d8b5dc8 0000000000000000
> [ 1383.373530] raw: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
> [ 1383.384321] page dumped because: kasan: bad access detected
> [ 1383.392659]
> [ 1383.396350] addr ffff888362d778d0 is located in stack of task kworker/u308:2/1889 at offset 32 in frame:
> [ 1383.409191]  pma_get_opa_portstatus+0x0/0xa80 [hfi1]
> [ 1383.416843]
> [ 1383.420497] this frame has 1 object:
> [ 1383.426494]  [32, 36) 'vl_select_mask'
> [ 1383.426495]
> [ 1383.436313] Memory state around the buggy address:
> [ 1383.443706]  ffff888362d77780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1383.453865]  ffff888362d77800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1383.464033] >ffff888362d77880: 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 f2 f2 00 00
> [ 1383.474199]                                                  ^
> [ 1383.482828]  ffff888362d77900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 1383.493071]  ffff888362d77980: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 04 f2 f2 f2
> [ 1383.503314]
> ==================================================================
> 
> Cc: <stable@vger.kernel.org>
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>

This needs a fixes line, can you tell me what it is?

> -		for_each_set_bit(vl, (unsigned long *)&(vl_all_mask),
> -				 8 * sizeof(vl_all_mask)) {

Well, that was obviously wrong!

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning
  2019-09-13 12:05   ` Jason Gunthorpe
@ 2019-09-13 12:43     ` Marciniszyn, Mike
  0 siblings, 0 replies; 7+ messages in thread
From: Marciniszyn, Mike @ 2019-09-13 12:43 UTC (permalink / raw)
  To: Jason Gunthorpe, Dalessandro, Dennis
  Cc: dledford, linux-rdma, Weiny, Ira, stable, Wan, Kaike

> > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> 
> This needs a fixes line, can you tell me what it is?

This is day 1 bug:

Fixes: 7724105686e7 ("IB/hfi1: add driver files")

Mike

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle
  2019-09-11 11:30 [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Dennis Dalessandro
                   ` (2 preceding siblings ...)
  2019-09-11 11:30 ` [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning Dennis Dalessandro
@ 2019-09-16 13:58 ` Jason Gunthorpe
  3 siblings, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2019-09-16 13:58 UTC (permalink / raw)
  To: Dennis Dalessandro; +Cc: dledford, linux-rdma

On Wed, Sep 11, 2019 at 07:30:34AM -0400, Dennis Dalessandro wrote:
> Here are some small patches for the next merge window. The traces patch requires
> one that is already in 5.3-rc4 and not yet in for-next. There will be a conflict
> otherwise. Easy to resolve but will also conflict when merge with Linus. The 
> pre-req patch is: IB/hfi1: Unsafe PSN checking for TID RDMA READ Resp packet [1]
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=35d5c8b82e2c32e8e29ca195bb4dac60ba7d97fc
> 
> 
> Ira Weiny (1):
>       IB/hfi1: Define variables as unsigned long to fix KASAN warning
> 
> Kaike Wan (2):
>       IB/hfi1: Add traces for TID RDMA READ
>       IB/{rdmavt, hfi1, qib}: Add a counter for credit waits

Applied to for-next

Thanks,
Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-09-16 13:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11 11:30 [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Dennis Dalessandro
2019-09-11 11:30 ` [PATCH for-next 1/3] IB/hfi1: Add traces for TID RDMA READ Dennis Dalessandro
2019-09-11 11:30 ` [PATCH for-next 2/3] IB/{rdmavt, hfi1, qib}: Add a counter for credit waits Dennis Dalessandro
2019-09-11 11:30 ` [PATCH for-next 3/3] IB/hfi1: Define variables as unsigned long to fix KASAN warning Dennis Dalessandro
2019-09-13 12:05   ` Jason Gunthorpe
2019-09-13 12:43     ` Marciniszyn, Mike
2019-09-16 13:58 ` [PATCH for-next 0/3] hfi1 and rdmavt fixes for next cycle Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).