linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Zhu Yanjun <yanjun.zhu@linux.dev>,
	Leon Romanovsky <leon@kernel.org>,
	linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2] rdma_rxe: call comp_handler without holding cq->cq_lock
Date: Wed, 24 Sep 2025 10:21:35 -0300	[thread overview]
Message-ID: <20250924132135.GA2653699@nvidia.com> (raw)
In-Reply-To: <20250822081941.989520-1-philipp.reisner@linbit.com>

On Fri, Aug 22, 2025 at 10:19:41AM +0200, Philipp Reisner wrote:
> Allow the comp_handler callback implementation to call ib_poll_cq().
> A call to ib_poll_cq() calls rxe_poll_cq() with the rdma_rxe driver.
> And rxe_poll_cq() locks cq->cq_lock. That leads to a spinlock deadlock.
> 
> The Mellanox and Intel drivers allow a comp_handler callback
> implementation to call ib_poll_cq().
> 
> Avoid the deadlock by calling the comp_handler callback without
> holding cq->cq_lock.

I spent some time looking at this, and I think the basic statement
above is right. The comp_handler should be able to call poll_cq/etc

rxe holding a lock it used to push a CQE is not correct.

However! The comp_handler is also supposed to be single threaded by
the driver, I don't think ULPs are prepared to handle concurrent calls
to comp_handler.

Other HW drivers run their comp_handlers from an EQ which is both
single threaded and does not exclude poll_cq/etc.

So while removing the cq lock here is correct from the perspective of
allowing poll_cq, I could not find any locking in rxe that made
do_complete() be single threaded.

Please send a v2, either explain how the do_complete is single
threaded in a comment above the comp_handler call, or make it be
single threaded.

Thanks,
Jason

  parent reply	other threads:[~2025-09-24 13:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22  8:19 [PATCH V2] rdma_rxe: call comp_handler without holding cq->cq_lock Philipp Reisner
2025-09-08 14:24 ` Leon Romanovsky
2025-09-09 14:48   ` Philipp Reisner
2025-09-09 15:31     ` Jason Gunthorpe
2025-09-09 16:00       ` Philipp Reisner
2025-09-10 10:27         ` Leon Romanovsky
2025-09-24 13:21 ` Jason Gunthorpe [this message]
2025-09-25  6:02   ` Philipp Reisner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250924132135.GA2653699@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=philipp.reisner@linbit.com \
    --cc=yanjun.zhu@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).