[PATCH for-next 0/2] uverbs and mlx5 fixes for 4.8

[PATCH for-next 0/2] uverbs and mlx5 fixes for 4.8

[Leon Romanovsky]

Hi Doug,

Here are two more fixes which are queued for 4.8. They apply cleanly
on top of your k.o/for-4.8 branch.

ConnectX-4/Connect-IB fix is marked to be sent to stable.

These patches are available in the "topic/fixes-ib-next" branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git

Or for browsing:
https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/?h=topic/fixes-ib-next

Thanks

Jason Gunthorpe (1):
  IB/uverbs: Fix race between uverbs_close and remove_one

Slava Shwartsman (1):
  IB/mlx5: Fix iteration overrun in GSI qps

 drivers/infiniband/core/uverbs.h      |  1 +
 drivers/infiniband/core/uverbs_main.c | 37 +++++++++++++++++++++++------------
 drivers/infiniband/hw/mlx5/gsi.c      | 19 ++++++------------
 3 files changed, 31 insertions(+), 26 deletions(-)

-- 
2.1.4

--
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 for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Leon Romanovsky]

From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Fixes an oops that might happen if uverbs_close races with
remove_one.

Both contexts may run ib_uverbs_cleanup_ucontext, it depends
on the flow.

Currently, there is no protection for a case that remove_one
didn't make the cleanup it runs to its end, the underlying
ib_device was freed then uverbs_close will call
ib_uverbs_cleanup_ucontext and OOPs.

Above might happen if uverbs_close deleted the file from the list
then remove_one didn't find it and runs to its end.

Fixes to protect against that case by a new cleanup lock so that
ib_uverbs_cleanup_ucontext will be called always before that
remove_one is ended.

Fixes: 35d4a0b63dc0 ("IB/uverbs: Fix race between ib_uverbs_open and remove_one")
Reported-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/core/uverbs.h      |  1 +
 drivers/infiniband/core/uverbs_main.c | 37 +++++++++++++++++++++++------------
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/core/uverbs.h b/drivers/infiniband/core/uverbs.h
index b7f3b8d..df26a74 100644
--- a/drivers/infiniband/core/uverbs.h
+++ b/drivers/infiniband/core/uverbs.h
@@ -116,6 +116,7 @@ struct ib_uverbs_event_file {
 struct ib_uverbs_file {
 	struct kref				ref;
 	struct mutex				mutex;
+	struct mutex                            cleanup_mutex; /* protect cleanup */
 	struct ib_uverbs_device		       *device;
 	struct ib_ucontext		       *ucontext;
 	struct ib_event_handler			event_handler;
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 426e0ac..0012fa5 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -969,6 +969,7 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)
 	file->async_file = NULL;
 	kref_init(&file->ref);
 	mutex_init(&file->mutex);
+	mutex_init(&file->cleanup_mutex);
 
 	filp->private_data = file;
 	kobject_get(&dev->kobj);
@@ -994,18 +995,20 @@ static int ib_uverbs_close(struct inode *inode, struct file *filp)
 {
 	struct ib_uverbs_file *file = filp->private_data;
 	struct ib_uverbs_device *dev = file->device;
-	struct ib_ucontext *ucontext = NULL;
+
+	mutex_lock(&file->cleanup_mutex);
+	if (file->ucontext) {
+		ib_uverbs_cleanup_ucontext(file, file->ucontext);
+		file->ucontext = NULL;
+	}
+	mutex_unlock(&file->cleanup_mutex);
 
 	mutex_lock(&file->device->lists_mutex);
-	ucontext = file->ucontext;
-	file->ucontext = NULL;
 	if (!file->is_closed) {
 		list_del(&file->list);
 		file->is_closed = 1;
 	}
 	mutex_unlock(&file->device->lists_mutex);
-	if (ucontext)
-		ib_uverbs_cleanup_ucontext(file, ucontext);
 
 	if (file->async_file)
 		kref_put(&file->async_file->ref, ib_uverbs_release_event_file);
@@ -1219,22 +1222,30 @@ static void ib_uverbs_free_hw_resources(struct ib_uverbs_device *uverbs_dev,
 	mutex_lock(&uverbs_dev->lists_mutex);
 	while (!list_empty(&uverbs_dev->uverbs_file_list)) {
 		struct ib_ucontext *ucontext;
-
 		file = list_first_entry(&uverbs_dev->uverbs_file_list,
 					struct ib_uverbs_file, list);
 		file->is_closed = 1;
-		ucontext = file->ucontext;
 		list_del(&file->list);
-		file->ucontext = NULL;
 		kref_get(&file->ref);
 		mutex_unlock(&uverbs_dev->lists_mutex);
-		/* We must release the mutex before going ahead and calling
-		 * disassociate_ucontext. disassociate_ucontext might end up
-		 * indirectly calling uverbs_close, for example due to freeing
-		 * the resources (e.g mmput).
-		 */
+
 		ib_uverbs_event_handler(&file->event_handler, &event);
+
+		mutex_lock(&file->cleanup_mutex);
+		ucontext = file->ucontext;
+		file->ucontext = NULL;
+		mutex_unlock(&file->cleanup_mutex);
+
+		/* At this point ib_uverbs_close cannot be running
+		 * ib_uverbs_cleanup_ucontext
+		 */
 		if (ucontext) {
+			/* We must release the mutex before going ahead and
+			 * calling disassociate_ucontext. disassociate_ucontext
+			 * might end up indirectly calling uverbs_close,
+			 * for example due to freeing the resources
+			 * (e.g mmput).
+			 */
 			ib_dev->disassociate_ucontext(ucontext);
 			ib_uverbs_cleanup_ucontext(file, ucontext);
 		}
-- 
2.1.4

--
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 for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1615 bytes --]

On Sun, 2016-07-03 at 15:28 +0300, Leon Romanovsky wrote:
> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> 
> Fixes an oops that might happen if uverbs_close races with
> remove_one.
> 
> Both contexts may run ib_uverbs_cleanup_ucontext, it depends
> on the flow.
> 
> Currently, there is no protection for a case that remove_one
> didn't make the cleanup it runs to its end, the underlying
> ib_device was freed then uverbs_close will call
> ib_uverbs_cleanup_ucontext and OOPs.
> 
> Above might happen if uverbs_close deleted the file from the list
> then remove_one didn't find it and runs to its end.
> 
> Fixes to protect against that case by a new cleanup lock so that
> ib_uverbs_cleanup_ucontext will be called always before that
> remove_one is ended.
> 
> Fixes: 35d4a0b63dc0 ("IB/uverbs: Fix race between ib_uverbs_open and
> remove_one")
> Reported-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

The only reason I hadn't taken this patch before is because Jason said
it was totally untested and someone (Devesh in this case) needed to
test it to make sure it resolved their problem.  I don't see a test-by
line here, so has this happened?

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Leon Romanovsky]

On Tue, Aug 2, 2016 at 9:31 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Sun, 2016-07-03 at 15:28 +0300, Leon Romanovsky wrote:
>> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>>
>> Fixes an oops that might happen if uverbs_close races with
>> remove_one.
>>
>> Both contexts may run ib_uverbs_cleanup_ucontext, it depends
>> on the flow.
>>
>> Currently, there is no protection for a case that remove_one
>> didn't make the cleanup it runs to its end, the underlying
>> ib_device was freed then uverbs_close will call
>> ib_uverbs_cleanup_ucontext and OOPs.
>>
>> Above might happen if uverbs_close deleted the file from the list
>> then remove_one didn't find it and runs to its end.
>>
>> Fixes to protect against that case by a new cleanup lock so that
>> ib_uverbs_cleanup_ucontext will be called always before that
>> remove_one is ended.
>>
>> Fixes: 35d4a0b63dc0 ("IB/uverbs: Fix race between ib_uverbs_open and
>> remove_one")
>> Reported-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
>> Signed-off-by: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> The only reason I hadn't taken this patch before is because Jason said
> it was totally untested and someone (Devesh in this case) needed to
> test it to make sure it resolved their problem.  I don't see a test-by
> line here, so has this happened?

I can't say about Devesh, but we verified it as part of our verification phase.
Do you need anything special except Yishai's SOB?

>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>               GPG KeyID: 0E572FDD
--
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 for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Jason Gunthorpe]

On Tue, Aug 02, 2016 at 09:50:14PM +0300, Leon Romanovsky wrote:
> > The only reason I hadn't taken this patch before is because Jason said
> > it was totally untested and someone (Devesh in this case) needed to
> > test it to make sure it resolved their problem.  I don't see a test-by
> > line here, so has this happened?
> 
> I can't say about Devesh, but we verified it as part of our verification phase.
> Do you need anything special except Yishai's SOB?

I was hoping to see Devesh confirm that the race he was able to
reproduce is in fact closed, unless Mellanox did the reproduction?

At least your testing shows it doesn't make things worse, so it is
probably OK to go ahead if Devesh does not respond soon.

Jason
--
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 for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

On Tue, 2016-08-02 at 13:26 -0600, Jason Gunthorpe wrote:
> On Tue, Aug 02, 2016 at 09:50:14PM +0300, Leon Romanovsky wrote:
> > 
> > > 
> > > The only reason I hadn't taken this patch before is because Jason
> > > said
> > > it was totally untested and someone (Devesh in this case) needed
> > > to
> > > test it to make sure it resolved their problem.  I don't see a
> > > test-by
> > > line here, so has this happened?
> > 
> > I can't say about Devesh, but we verified it as part of our
> > verification phase.
> > Do you need anything special except Yishai's SOB?
> 
> I was hoping to see Devesh confirm that the race he was able to
> reproduce is in fact closed, unless Mellanox did the reproduction?
> 
> At least your testing shows it doesn't make things worse, so it is
> probably OK to go ahead if Devesh does not respond soon.
> 
> Jason

I'm not sure what Devesh is off doing now a days, so I went ahead and
took this.  With the fact that Mellanox tested it, I'd rather have it
in than out.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

On Tue, Aug 02, 2016 at 10:43:14PM -0400, Doug Ledford wrote:
> On Tue, 2016-08-02 at 13:26 -0600, Jason Gunthorpe wrote:
> > On Tue, Aug 02, 2016 at 09:50:14PM +0300, Leon Romanovsky wrote:
> > > 
> > > > 
> > > > The only reason I hadn't taken this patch before is because Jason
> > > > said
> > > > it was totally untested and someone (Devesh in this case) needed
> > > > to
> > > > test it to make sure it resolved their problem.  I don't see a
> > > > test-by
> > > > line here, so has this happened?
> > > 
> > > I can't say about Devesh, but we verified it as part of our
> > > verification phase.
> > > Do you need anything special except Yishai's SOB?
> > 
> > I was hoping to see Devesh confirm that the race he was able to
> > reproduce is in fact closed, unless Mellanox did the reproduction?
> > 
> > At least your testing shows it doesn't make things worse, so it is
> > probably OK to go ahead if Devesh does not respond soon.
> > 
> > Jason
> 
> I'm not sure what Devesh is off doing now a days, so I went ahead and
> took this.  With the fact that Mellanox tested it, I'd rather have it
> in than out.

Thanks

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



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH for-next 1/2] IB/uverbs: Fix race between uverbs_close and remove_one

[Devesh Sharma]

Hi Jason et al.

I could not respond to this mail chain. I was quite busy somewhere
else and did not had any bandwidth at all to get back to this one.
Thanks for taking care of this issue. I would try to see If I could
confirm that the fix works for ocrdma.

-Regards

On Wed, Aug 3, 2016 at 10:48 AM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Aug 02, 2016 at 10:43:14PM -0400, Doug Ledford wrote:
>> On Tue, 2016-08-02 at 13:26 -0600, Jason Gunthorpe wrote:
>> > On Tue, Aug 02, 2016 at 09:50:14PM +0300, Leon Romanovsky wrote:
>> > >
>> > > >
>> > > > The only reason I hadn't taken this patch before is because Jason
>> > > > said
>> > > > it was totally untested and someone (Devesh in this case) needed
>> > > > to
>> > > > test it to make sure it resolved their problem.  I don't see a
>> > > > test-by
>> > > > line here, so has this happened?
>> > >
>> > > I can't say about Devesh, but we verified it as part of our
>> > > verification phase.
>> > > Do you need anything special except Yishai's SOB?
>> >
>> > I was hoping to see Devesh confirm that the race he was able to
>> > reproduce is in fact closed, unless Mellanox did the reproduction?
>> >
>> > At least your testing shows it doesn't make things worse, so it is
>> > probably OK to go ahead if Devesh does not respond soon.
>> >
>> > Jason
>>
>> I'm not sure what Devesh is off doing now a days, so I went ahead and
>> took this.  With the fact that Mellanox tested it, I'd rather have it
>> in than out.
>
> Thanks
>
>>
>> --
>> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>               GPG KeyID: 0E572FDD
>
>
--
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 for-next 2/2] IB/mlx5: Fix iteration overrun in GSI qps

[Leon Romanovsky]

From: Slava Shwartsman <slavash-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Number of outstanding_pi may overflow and as a result may indicate that
there are no elements in the queue. The effect of doing this is that the
MAD layer will get stuck waiting for completions. The MAD layer will
think that the QP is full - because it didn't receive these completions.

This fix changes it so the outstanding_pi number is increased
with 32-bit wraparound and is not limited to max_send_wr so
that the difference between outstanding_pi and outstanding_ci will
really indicate the number of outstanding completions.

Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Fixes: ea6dc2036224 ('IB/mlx5: Reorder GSI completions')
Signed-off-by: Slava Shwartsman <slavash-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/infiniband/hw/mlx5/gsi.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/gsi.c b/drivers/infiniband/hw/mlx5/gsi.c
index 53e03c8..79e6309 100644
--- a/drivers/infiniband/hw/mlx5/gsi.c
+++ b/drivers/infiniband/hw/mlx5/gsi.c
@@ -69,15 +69,6 @@ static bool mlx5_ib_deth_sqpn_cap(struct mlx5_ib_dev *dev)
 	return MLX5_CAP_GEN(dev->mdev, set_deth_sqpn);
 }
 
-static u32 next_outstanding(struct mlx5_ib_gsi_qp *gsi, u32 index)
-{
-	return ++index % gsi->cap.max_send_wr;
-}
-
-#define for_each_outstanding_wr(gsi, index) \
-	for (index = gsi->outstanding_ci; index != gsi->outstanding_pi; \
-	     index = next_outstanding(gsi, index))
-
 /* Call with gsi->lock locked */
 static void generate_completions(struct mlx5_ib_gsi_qp *gsi)
 {
@@ -85,8 +76,9 @@ static void generate_completions(struct mlx5_ib_gsi_qp *gsi)
 	struct mlx5_ib_gsi_wr *wr;
 	u32 index;
 
-	for_each_outstanding_wr(gsi, index) {
-		wr = &gsi->outstanding_wrs[index];
+	for (index = gsi->outstanding_ci; index != gsi->outstanding_pi;
+	     index++) {
+		wr = &gsi->outstanding_wrs[index % gsi->cap.max_send_wr];
 
 		if (!wr->completed)
 			break;
@@ -430,8 +422,9 @@ static int mlx5_ib_add_outstanding_wr(struct mlx5_ib_gsi_qp *gsi,
 		return -ENOMEM;
 	}
 
-	gsi_wr = &gsi->outstanding_wrs[gsi->outstanding_pi];
-	gsi->outstanding_pi = next_outstanding(gsi, gsi->outstanding_pi);
+	gsi_wr = &gsi->outstanding_wrs[gsi->outstanding_pi %
+				       gsi->cap.max_send_wr];
+	gsi->outstanding_pi++;
 
 	if (!wc) {
 		memset(&gsi_wr->wc, 0, sizeof(gsi_wr->wc));
-- 
2.1.4

--
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 for-next 2/2] IB/mlx5: Fix iteration overrun in GSI qps

[Leon Romanovsky]

From: Slava Shwartsman <slavash@mellanox.com>

Number of outstanding_pi may overflow and as a result may indicate that
there are no elements in the queue. The effect of doing this is that the
MAD layer will get stuck waiting for completions. The MAD layer will
think that the QP is full - because it didn't receive these completions.

This fix changes it so the outstanding_pi number is increased
with 32-bit wraparound and is not limited to max_send_wr so
that the difference between outstanding_pi and outstanding_ci will
really indicate the number of outstanding completions.

Cc: Stable <stable@vger.kernel.org>
Fixes: ea6dc2036224 ('IB/mlx5: Reorder GSI completions')
Signed-off-by: Slava Shwartsman <slavash@mellanox.com>
Signed-off-by: Leon Romanovsky <leon@kernel.org>
---
 drivers/infiniband/hw/mlx5/gsi.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/gsi.c b/drivers/infiniband/hw/mlx5/gsi.c
index 53e03c8..79e6309 100644
--- a/drivers/infiniband/hw/mlx5/gsi.c
+++ b/drivers/infiniband/hw/mlx5/gsi.c
@@ -69,15 +69,6 @@ static bool mlx5_ib_deth_sqpn_cap(struct mlx5_ib_dev *dev)
 	return MLX5_CAP_GEN(dev->mdev, set_deth_sqpn);
 }
 
-static u32 next_outstanding(struct mlx5_ib_gsi_qp *gsi, u32 index)
-{
-	return ++index % gsi->cap.max_send_wr;
-}
-
-#define for_each_outstanding_wr(gsi, index) \
-	for (index = gsi->outstanding_ci; index != gsi->outstanding_pi; \
-	     index = next_outstanding(gsi, index))
-
 /* Call with gsi->lock locked */
 static void generate_completions(struct mlx5_ib_gsi_qp *gsi)
 {
@@ -85,8 +76,9 @@ static void generate_completions(struct mlx5_ib_gsi_qp *gsi)
 	struct mlx5_ib_gsi_wr *wr;
 	u32 index;
 
-	for_each_outstanding_wr(gsi, index) {
-		wr = &gsi->outstanding_wrs[index];
+	for (index = gsi->outstanding_ci; index != gsi->outstanding_pi;
+	     index++) {
+		wr = &gsi->outstanding_wrs[index % gsi->cap.max_send_wr];
 
 		if (!wr->completed)
 			break;
@@ -430,8 +422,9 @@ static int mlx5_ib_add_outstanding_wr(struct mlx5_ib_gsi_qp *gsi,
 		return -ENOMEM;
 	}
 
-	gsi_wr = &gsi->outstanding_wrs[gsi->outstanding_pi];
-	gsi->outstanding_pi = next_outstanding(gsi, gsi->outstanding_pi);
+	gsi_wr = &gsi->outstanding_wrs[gsi->outstanding_pi %
+				       gsi->cap.max_send_wr];
+	gsi->outstanding_pi++;
 
 	if (!wc) {
 		memset(&gsi_wr->wc, 0, sizeof(gsi_wr->wc));
-- 
2.1.4

Re: [PATCH for-next 2/2] IB/mlx5: Fix iteration overrun in GSI qps

[Haggai Eran]

On 7/3/2016 3:28 PM, Leon Romanovsky wrote:
> From: Slava Shwartsman <slavash-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> Number of outstanding_pi may overflow and as a result may indicate that
> there are no elements in the queue. The effect of doing this is that the
> MAD layer will get stuck waiting for completions. The MAD layer will
> think that the QP is full - because it didn't receive these completions.
> 
> This fix changes it so the outstanding_pi number is increased
> with 32-bit wraparound and is not limited to max_send_wr so
> that the difference between outstanding_pi and outstanding_ci will
> really indicate the number of outstanding completions.
> 
> Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Fixes: ea6dc2036224 ('IB/mlx5: Reorder GSI completions')
> Signed-off-by: Slava Shwartsman <slavash-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Reviewed-by: Haggai Eran <haggaie-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Thanks for this fix,

Haggai

--
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 for-next 2/2] IB/mlx5: Fix iteration overrun in GSI qps

[Haggai Eran]

On 7/3/2016 3:28 PM, Leon Romanovsky wrote:
> From: Slava Shwartsman <slavash@mellanox.com>
> 
> Number of outstanding_pi may overflow and as a result may indicate that
> there are no elements in the queue. The effect of doing this is that the
> MAD layer will get stuck waiting for completions. The MAD layer will
> think that the QP is full - because it didn't receive these completions.
> 
> This fix changes it so the outstanding_pi number is increased
> with 32-bit wraparound and is not limited to max_send_wr so
> that the difference between outstanding_pi and outstanding_ci will
> really indicate the number of outstanding completions.
> 
> Cc: Stable <stable@vger.kernel.org>
> Fixes: ea6dc2036224 ('IB/mlx5: Reorder GSI completions')
> Signed-off-by: Slava Shwartsman <slavash@mellanox.com>
> Signed-off-by: Leon Romanovsky <leon@kernel.org>

Reviewed-by: Haggai Eran <haggaie@mellanox.com>

Thanks for this fix,

Haggai

Re: [PATCH for-next 2/2] IB/mlx5: Fix iteration overrun in GSI qps

[Sagi Grimberg]

Looks good,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH for-next 0/2] uverbs and mlx5 fixes for 4.8

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On Sun, 2016-07-03 at 15:28 +0300, Leon Romanovsky wrote:
> Hi Doug,
> 
> Here are two more fixes which are queued for 4.8. They apply cleanly
> on top of your k.o/for-4.8 branch.
> 
> ConnectX-4/Connect-IB fix is marked to be sent to stable.
> 
> These patches are available in the "topic/fixes-ib-next" branch of
> this git repo:
> git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git
> 
> Or for browsing:
> https://git.kernel.org/cgit/linux/kernel/git/leon/linux-rdma.git/log/
> ?h=topic/fixes-ib-next
> 
> Thanks
> 
> Jason Gunthorpe (1):
>   IB/uverbs: Fix race between uverbs_close and remove_one
> 
> Slava Shwartsman (1):
>   IB/mlx5: Fix iteration overrun in GSI qps
> 

I applied patch 2/2, I'm waiting on testing confirmation for 1/2.

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]