All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.