From patchwork Mon Aug 29 10:04:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leon Romanovsky X-Patchwork-Id: 9303401 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2C0FD601C0 for ; Mon, 29 Aug 2016 10:04:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 136D128693 for ; Mon, 29 Aug 2016 10:04:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0815C28778; Mon, 29 Aug 2016 10:04:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4662828693 for ; Mon, 29 Aug 2016 10:04:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756663AbcH2KEl (ORCPT ); Mon, 29 Aug 2016 06:04:41 -0400 Received: from mail.kernel.org ([198.145.29.136]:59304 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756148AbcH2KEk (ORCPT ); Mon, 29 Aug 2016 06:04:40 -0400 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BBF76201C7; Mon, 29 Aug 2016 10:04:38 +0000 (UTC) Received: from localhost (unknown [213.57.247.249]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B7E08201B9; Mon, 29 Aug 2016 10:04:36 +0000 (UTC) Date: Mon, 29 Aug 2016 13:04:34 +0300 From: Leon Romanovsky To: Sagi Grimberg Cc: dledford@redhat.com, Or Gerlitz , Matan Barak , linux-rdma@vger.kernel.org Subject: Re: [PATCH rdma-rc 4/9] IB/mlx4: Don't return errors from poll_cq Message-ID: <20160829100434.GD594@leon.nu> References: <1472371118-8260-1-git-send-email-leon@kernel.org> <1472371118-8260-5-git-send-email-leon@kernel.org> <82f1a1be-1189-c8c6-b134-d2f582cc7fa0@grimberg.me> <20160829094119.GB594@leon.nu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160829094119.GB594@leon.nu> User-Agent: Mutt/1.5.24 (2015-08-30) X-Virus-Scanned: ClamAV using ClamSMTP Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Aug 29, 2016 at 12:41:19PM +0300, Leon Romanovsky wrote: > 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; > } Or more complete patch (untested), From 8695e5b807610e08de055b04ddd16caa6a5b61f2 Mon Sep 17 00:00:00 2001 From: Leon Romanovsky Date: Mon, 29 Aug 2016 12:42:24 +0300 Subject: [PATCH] IB/mlx4: Use RCU to perform radix tree lookup for SRQ Radix tree lookup can be performed without locking. Suggested-by: Sagi Grimberg Signed-off-by: Leon Romanovsky --- drivers/net/ethernet/mellanox/mlx4/srq.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) -- 2.7.4 > > > > > 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@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/ethernet/mellanox/mlx4/srq.c b/drivers/net/ethernet/mellanox/mlx4/srq.c index 6714662..f44d089 100644 --- a/drivers/net/ethernet/mellanox/mlx4/srq.c +++ b/drivers/net/ethernet/mellanox/mlx4/srq.c @@ -45,15 +45,12 @@ void mlx4_srq_event(struct mlx4_dev *dev, u32 srqn, int event_type) struct mlx4_srq_table *srq_table = &mlx4_priv(dev)->srq_table; struct mlx4_srq *srq; - spin_lock(&srq_table->lock); - + rcu_read_lock(); srq = radix_tree_lookup(&srq_table->tree, srqn & (dev->caps.num_srqs - 1)); + rcu_read_unlock(); if (srq) atomic_inc(&srq->refcount); - - spin_unlock(&srq_table->lock); - - if (!srq) { + else { mlx4_warn(dev, "Async event for bogus SRQ %08x\n", srqn); return; } @@ -301,12 +298,11 @@ struct mlx4_srq *mlx4_srq_lookup(struct mlx4_dev *dev, u32 srqn) { struct mlx4_srq_table *srq_table = &mlx4_priv(dev)->srq_table; 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; }