[PATCH 0/2] IB/hfi1: Fixes for 4.7 rc-6

[PATCH 0/2] IB/hfi1: Fixes for 4.7 rc-6

[Dennis Dalessandro]

Doug,

Here are two more patches for the 4.7 RC if possible. One Mike has marked as
stable.

These apply on the RC-5 tree and should apply to your trees without any issue. 

Patches can also be viewed in my repo at:
https://github.com/ddalessa/kernel/tree/for-4.7

---

Mike Marciniszyn (1):
      IB/hfi1: Correct issues with sc5 computation

Tadeusz Struk (1):
      IB/hfi1: Fix sleep inside atomic issue in init_asic_data


 drivers/infiniband/hw/hfi1/chip.c |   21 ++++++++++++++-------
 drivers/infiniband/hw/hfi1/ud.c   |   23 +++--------------------
 2 files changed, 17 insertions(+), 27 deletions(-)

--
-Denny

[PATCH 1/2] IB/hfi1: Correct issues with sc5 computation

[Dennis Dalessandro]

From: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

There are several computatations of the sc in the
ud receive routine.

Besides the code duplication, all are wrong when the
sc is greater than 15.   In that case the code incorrectly
or's a 1 into the computed sc instead of 1 shifted left
by 4.

Fix precomputed sc5 by using an already implemented routine
hdr2sc() and deleting flawed duplicated code.

Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # 4.6+
Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/ud.c |   23 +++--------------------
 1 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/ud.c b/drivers/infiniband/hw/hfi1/ud.c
index 1e503ad..be91f6f 100644
--- a/drivers/infiniband/hw/hfi1/ud.c
+++ b/drivers/infiniband/hw/hfi1/ud.c
@@ -678,8 +678,7 @@ void hfi1_ud_rcv(struct hfi1_packet *packet)
 	u32 tlen = packet->tlen;
 	struct rvt_qp *qp = packet->qp;
 	bool has_grh = rcv_flags & HFI1_HAS_GRH;
-	bool sc4_bit = has_sc4_bit(packet);
-	u8 sc;
+	u8 sc5 = hdr2sc((struct hfi1_message_header *)hdr, packet->rhf);
 	u32 bth1;
 	int is_mcast;
 	struct ib_grh *grh = NULL;
@@ -697,10 +696,8 @@ void hfi1_ud_rcv(struct hfi1_packet *packet)
 		 */
 		struct hfi1_pportdata *ppd = ppd_from_ibp(ibp);
 		u32 lqpn =  be32_to_cpu(ohdr->bth[1]) & RVT_QPN_MASK;
-		u8 sl, sc5;
+		u8 sl;
 
-		sc5 = (be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf;
-		sc5 |= sc4_bit;
 		sl = ibp->sc_to_sl[sc5];
 
 		process_becn(ppd, sl, 0, lqpn, 0, IB_CC_SVCTYPE_UD);
@@ -717,10 +714,6 @@ void hfi1_ud_rcv(struct hfi1_packet *packet)
 
 	if (!is_mcast && (opcode != IB_OPCODE_CNP) && bth1 & HFI1_FECN_SMASK) {
 		u16 slid = be16_to_cpu(hdr->lrh[3]);
-		u8 sc5;
-
-		sc5 = (be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf;
-		sc5 |= sc4_bit;
 
 		return_cnp(ibp, qp, src_qp, pkey, dlid, slid, sc5, grh);
 	}
@@ -745,10 +738,6 @@ void hfi1_ud_rcv(struct hfi1_packet *packet)
 		if (qp->ibqp.qp_num > 1) {
 			struct hfi1_pportdata *ppd = ppd_from_ibp(ibp);
 			u16 slid;
-			u8 sc5;
-
-			sc5 = (be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf;
-			sc5 |= sc4_bit;
 
 			slid = be16_to_cpu(hdr->lrh[3]);
 			if (unlikely(rcv_pkey_check(ppd, pkey, sc5, slid))) {
@@ -790,10 +779,6 @@ void hfi1_ud_rcv(struct hfi1_packet *packet)
 		/* Received on QP0, and so by definition, this is an SMP */
 		struct opa_smp *smp = (struct opa_smp *)data;
 		u16 slid = be16_to_cpu(hdr->lrh[3]);
-		u8 sc5;
-
-		sc5 = (be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf;
-		sc5 |= sc4_bit;
 
 		if (opa_smp_check(ibp, pkey, sc5, qp, slid, smp))
 			goto drop;
@@ -890,9 +875,7 @@ void hfi1_ud_rcv(struct hfi1_packet *packet)
 	}
 
 	wc.slid = be16_to_cpu(hdr->lrh[3]);
-	sc = (be16_to_cpu(hdr->lrh[0]) >> 12) & 0xf;
-	sc |= sc4_bit;
-	wc.sl = ibp->sc_to_sl[sc];
+	wc.sl = ibp->sc_to_sl[sc5];
 
 	/*
 	 * Save the LMC lower bits if the destination LID is a unicast LID.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] IB/hfi1: Fix sleep inside atomic issue in init_asic_data

[Dennis Dalessandro]

From: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

The critical section should protect only the list traversal
and dd->asic_data modification, not the memory allocation.
The fix pulls the allocation out of the critical section.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Dean Luick <dean.luick-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Tadeusz Struk <tadeusz.struk-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/chip.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index f5de851..0662451 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -14113,8 +14113,14 @@ static int init_asic_data(struct hfi1_devdata *dd)
 {
 	unsigned long flags;
 	struct hfi1_devdata *tmp, *peer = NULL;
+	struct hfi1_asic_data *asic_data;
 	int ret = 0;
 
+	/* pre-allocate the asic structure in case we are the first device */
+	asic_data = kzalloc(sizeof(*dd->asic_data), GFP_KERNEL);
+	if (!asic_data)
+		return -ENOMEM;
+
 	spin_lock_irqsave(&hfi1_devs_lock, flags);
 	/* Find our peer device */
 	list_for_each_entry(tmp, &hfi1_dev_list, list) {
@@ -14126,19 +14132,20 @@ static int init_asic_data(struct hfi1_devdata *dd)
 	}
 
 	if (peer) {
+		/* use already allocated structure */
 		dd->asic_data = peer->asic_data;
+		kfree(asic_data);
 	} else {
-		dd->asic_data = kzalloc(sizeof(*dd->asic_data), GFP_KERNEL);
-		if (!dd->asic_data) {
-			ret = -ENOMEM;
-			goto done;
-		}
+		dd->asic_data = asic_data;
 		mutex_init(&dd->asic_data->asic_resource_mutex);
 	}
 	dd->asic_data->dds[dd->hfi1_id] = dd; /* self back-pointer */
-
-done:
 	spin_unlock_irqrestore(&hfi1_devs_lock, flags);
+
+	/* first one through - set up i2c devices */
+	if (!peer)
+		ret = set_up_i2c(dd, dd->asic_data);
+
 	return ret;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] IB/hfi1: Fix sleep inside atomic issue in init_asic_data

[Foraker, Jim]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2899 bytes --]

     Thanks for getting this issue fixed.  One comment below.

On 7/1/16, 3:57 PM, "linux-rdma-owner@vger.kernel.org on behalf of Dennis
Dalessandro" <linux-rdma-owner@vger.kernel.org on behalf of
dennis.dalessandro@intel.com> wrote:

>From: Tadeusz Struk <tadeusz.struk@intel.com>
>
>The critical section should protect only the list traversal
>and dd->asic_data modification, not the memory allocation.
>The fix pulls the allocation out of the critical section.
>
>Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
>Reviewed-by: Sebastian Sanchez <sebastian.sanchez@intel.com>
>Reviewed-by: Dean Luick <dean.luick@intel.com>
>Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
>Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
>---
> drivers/infiniband/hw/hfi1/chip.c |   21 ++++++++++++++-------
> 1 files changed, 14 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/infiniband/hw/hfi1/chip.c
>b/drivers/infiniband/hw/hfi1/chip.c
>index f5de851..0662451 100644
>--- a/drivers/infiniband/hw/hfi1/chip.c
>+++ b/drivers/infiniband/hw/hfi1/chip.c
>@@ -14113,8 +14113,14 @@ static int init_asic_data(struct hfi1_devdata
>*dd)
> {
> 	unsigned long flags;
> 	struct hfi1_devdata *tmp, *peer = NULL;
>+	struct hfi1_asic_data *asic_data;
> 	int ret = 0;
> 
>+	/* pre-allocate the asic structure in case we are the first device */
>+	asic_data = kzalloc(sizeof(*dd->asic_data), GFP_KERNEL);
>+	if (!asic_data)
>+		return -ENOMEM;
>+
> 	spin_lock_irqsave(&hfi1_devs_lock, flags);
> 	/* Find our peer device */
> 	list_for_each_entry(tmp, &hfi1_dev_list, list) {
>@@ -14126,19 +14132,20 @@ static int init_asic_data(struct hfi1_devdata
>*dd)
> 	}
> 
> 	if (peer) {
>+		/* use already allocated structure */
> 		dd->asic_data = peer->asic_data;
>+		kfree(asic_data);
> 	} else {
>-		dd->asic_data = kzalloc(sizeof(*dd->asic_data), GFP_KERNEL);
>-		if (!dd->asic_data) {
>-			ret = -ENOMEM;
>-			goto done;
>-		}
>+		dd->asic_data = asic_data;
> 		mutex_init(&dd->asic_data->asic_resource_mutex);
> 	}
> 	dd->asic_data->dds[dd->hfi1_id] = dd; /* self back-pointer */
>-
>-done:
> 	spin_unlock_irqrestore(&hfi1_devs_lock, flags);
>+
>+	/* first one through - set up i2c devices */
>+	if (!peer)
>+		ret = set_up_i2c(dd, dd->asic_data);

     This function doesn’t exist until patch 11 of your for-4.8 series
(where it is defined but never used), and seems unrelated to fixing the
allocation issue.  Patch mixup?

     Jim

>+
> 	return ret;
> }
> 
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH 0/2] IB/hfi1: Fixes for 4.7 rc-6

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 926 bytes --]

On 7/1/2016 6:56 PM, Dennis Dalessandro wrote:
> Doug,
> 
> Here are two more patches for the 4.7 RC if possible. One Mike has marked as
> stable.
> 
> These apply on the RC-5 tree and should apply to your trees without any issue. 
> 
> Patches can also be viewed in my repo at:
> https://github.com/ddalessa/kernel/tree/for-4.7
> 
> ---
> 
> Mike Marciniszyn (1):
>       IB/hfi1: Correct issues with sc5 computation
> 
> Tadeusz Struk (1):
>       IB/hfi1: Fix sleep inside atomic issue in init_asic_data
> 
> 
>  drivers/infiniband/hw/hfi1/chip.c |   21 ++++++++++++++-------
>  drivers/infiniband/hw/hfi1/ud.c   |   23 +++--------------------
>  2 files changed, 17 insertions(+), 27 deletions(-)
> 
> --
> -Denny
> 

I applied the 1/2 patch from this series, and the v3, 2/2 patch from Mike.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG Key ID: 0E572FDD


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]