diff mbox

IB/hfi1: Fix hard lockup due to not using save/restore spin lock

Message ID 20160601143812.26872.85826.stgit@phlsvslse11.ph.intel.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Marciniszyn, Mike June 1, 2016, 2:38 p.m. UTC
Upstream commit 7049de65c9e5 into 4.6.

Commit b9b06cb6feda
("IB/hfi1: Fix missing lock/unlock in verbs drain callback")
added a spin lock.

Unfortunately, the new lock code can be called from a base
level interrupt state, and an interrupt that can get stacked
will attempt to get the same lock.

Fix by using the flag save/restore spin lock variation.

Reviewed-by: Sebastian Sanchez <sebastian.sanchez@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>
---
 drivers/staging/rdma/hfi1/qp.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


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

Comments

Leon Romanovsky June 1, 2016, 3:44 p.m. UTC | #1
On Wed, Jun 01, 2016 at 10:38:12AM -0400, Mike Marciniszyn wrote:
> Upstream commit 7049de65c9e5 into 4.6.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Is it part of the commit message?

> 
> Commit b9b06cb6feda
> ("IB/hfi1: Fix missing lock/unlock in verbs drain callback")
> added a spin lock.
> 
> Unfortunately, the new lock code can be called from a base
> level interrupt state, and an interrupt that can get stacked
> will attempt to get the same lock.
> 
> Fix by using the flag save/restore spin lock variation.
> 
> Reviewed-by: Sebastian Sanchez <sebastian.sanchez@intel.com>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> Signed-off-by: Doug Ledford <dledford@redhat.com>
> ---
>  drivers/staging/rdma/hfi1/qp.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rdma/hfi1/qp.c b/drivers/staging/rdma/hfi1/qp.c
> index dc9119e..0401955 100644
> --- a/drivers/staging/rdma/hfi1/qp.c
> +++ b/drivers/staging/rdma/hfi1/qp.c
> @@ -512,6 +512,7 @@ static void iowait_wakeup(struct iowait *wait, int reason)
>  static void iowait_sdma_drained(struct iowait *wait)
>  {
>  	struct rvt_qp *qp = iowait_to_qp(wait);
> +	unsigned long flags;
>  
>  	/*
>  	 * This happens when the send engine notes
> @@ -519,12 +520,12 @@ static void iowait_sdma_drained(struct iowait *wait)
>  	 * do the flush work until that QP's
>  	 * sdma work has finished.
>  	 */
> -	spin_lock(&qp->s_lock);
> +	spin_lock_irqsave(&qp->s_lock, flags);
>  	if (qp->s_flags & RVT_S_WAIT_DMA) {
>  		qp->s_flags &= ~RVT_S_WAIT_DMA;
>  		hfi1_schedule_send(qp);
>  	}
> -	spin_unlock(&qp->s_lock);
> +	spin_unlock_irqrestore(&qp->s_lock, flags);
>  }
>  
>  /**
> 
> --
> 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
Greg KH June 1, 2016, 4:14 p.m. UTC | #2
On Wed, Jun 01, 2016 at 06:44:40PM +0300, Leon Romanovsky wrote:
> On Wed, Jun 01, 2016 at 10:38:12AM -0400, Mike Marciniszyn wrote:
> > Upstream commit 7049de65c9e5 into 4.6.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Is it part of the commit message?

It's enough for me to know what to do here :)

thanks,

greg k-h
--
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
Marciniszyn, Mike June 1, 2016, 4:25 p.m. UTC | #3
> 
> On Wed, Jun 01, 2016 at 10:38:12AM -0400, Mike Marciniszyn wrote:
> > Upstream commit 7049de65c9e5 into 4.6.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Is it part of the commit message?
> 

Yes per Documentation/stable_kernel_rules.txt:

   --- Option 3 ---

   Send the patch, after verifying that it follows the above rules, to
   stable@vger.kernel.org.  You must note the upstream commit ID in the
   changelog of your submission, as well as the kernel version you wish
   it to be applied to.

Mike
--
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
Leon Romanovsky June 1, 2016, 4:47 p.m. UTC | #4
On Wed, Jun 01, 2016 at 09:14:59AM -0700, Greg KH wrote:
> On Wed, Jun 01, 2016 at 06:44:40PM +0300, Leon Romanovsky wrote:
> > On Wed, Jun 01, 2016 at 10:38:12AM -0400, Mike Marciniszyn wrote:
> > > Upstream commit 7049de65c9e5 into 4.6.
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > Is it part of the commit message?
> 
> It's enough for me to know what to do here :)

Good to know, I always used CC: stable notation.

> 
> thanks,
> 
> greg k-h
Marciniszyn, Mike June 1, 2016, 4:50 p.m. UTC | #5
> 
> On Wed, Jun 01, 2016 at 09:14:59AM -0700, Greg KH wrote:
> > On Wed, Jun 01, 2016 at 06:44:40PM +0300, Leon Romanovsky wrote:
> > > On Wed, Jun 01, 2016 at 10:38:12AM -0400, Mike Marciniszyn wrote:
> > > > Upstream commit 7049de65c9e5 into 4.6.
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > Is it part of the commit message?
> >
> > It's enough for me to know what to do here :)
> 
> Good to know, I always used CC: stable notation.
> 

The patch didn't port cleanly because of the staging -> drivers/infiniband move.

Hence the option 3 choice for submission.

Mike
--
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
diff mbox

Patch

diff --git a/drivers/staging/rdma/hfi1/qp.c b/drivers/staging/rdma/hfi1/qp.c
index dc9119e..0401955 100644
--- a/drivers/staging/rdma/hfi1/qp.c
+++ b/drivers/staging/rdma/hfi1/qp.c
@@ -512,6 +512,7 @@  static void iowait_wakeup(struct iowait *wait, int reason)
 static void iowait_sdma_drained(struct iowait *wait)
 {
 	struct rvt_qp *qp = iowait_to_qp(wait);
+	unsigned long flags;
 
 	/*
 	 * This happens when the send engine notes
@@ -519,12 +520,12 @@  static void iowait_sdma_drained(struct iowait *wait)
 	 * do the flush work until that QP's
 	 * sdma work has finished.
 	 */
-	spin_lock(&qp->s_lock);
+	spin_lock_irqsave(&qp->s_lock, flags);
 	if (qp->s_flags & RVT_S_WAIT_DMA) {
 		qp->s_flags &= ~RVT_S_WAIT_DMA;
 		hfi1_schedule_send(qp);
 	}
-	spin_unlock(&qp->s_lock);
+	spin_unlock_irqrestore(&qp->s_lock, flags);
 }
 
 /**