Re: [PATCH rdma-rc 4/9] IB/mlx4: Don't return errors from poll_cq

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

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

On Sun, Aug 28, 2016 at 07:05:51PM +0300, Sagi Grimberg wrote:
>
> > 		/* SRQ is also in the radix tree */
> > 		msrq = mlx4_srq_lookup(to_mdev(cq->ibcq.device)->dev,
> > 				       srq_num);
> >-		if (unlikely(!msrq)) {
> >-			pr_warn("CQ %06x with entry for unknown SRQN %06x\n",
> >-				cq->mcq.cqn, srq_num);
> >-			return -EINVAL;
> >-		}
> > 	}
>
> BTW, this is completely unrelated to this patch, but the current
> implementation of shared receive queues in Mellanox drivers is
> *very* inefficient. Each completion that relates to a srq the
> mlx4/mlx5 drivers perform a device-wide locked srq lookup which
> is pretty bad... (it kills consumers that want to use more
> then one SRQ)
>
> Other drivers that support kernel SRQs that I've looked at are ocrdma and
> i40iw and they don't seem to have this lock everything approach.
>
> At the very-least we should try to make it a rcu + percpu_ref
> instead of a killer device-wide lock. It'd be even better if
> we use refcounting in the IB core and have the drivers not worry
> about the kernel consumers destroying SRQs while processing IO
> (i.e. when all the related QPs and CQs are destroyed).

This can be as a beginning.
diff --git a/drivers/net/ethernet/mellanox/mlx4/srq.c b/drivers/net/ethernet/mellanox/mlx4/srq.c
index 6714662..e53d366 100644
--- a/drivers/net/ethernet/mellanox/mlx4/srq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/srq.c
@@ -303,10 +303,10 @@ struct mlx4_srq *mlx4_srq_lookup(struct mlx4_dev *dev, u32 srqn)
 	struct mlx4_srq *srq;
 	unsigned long flags;

-	spin_lock_irqsave(&srq_table->lock, flags);
+	rcu_read_lock();
 	srq = radix_tree_lookup(&srq_table->tree,
 				srqn & (dev->caps.num_srqs - 1));
-	spin_unlock_irqrestore(&srq_table->lock, flags);
+	rcu_read_unlock();

 	return srq;
 }

>
> I have some code in the works on this but it's not high on my todo
> list at the moment. Mellanox folks, any thoughts on this?
> --
> 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

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