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

On Sun, Aug 28, 2016 at 07:05:51PM +0300, Sagi Grimberg wrote:

quoted

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