From: Philipp Reisner <philipp.reisner@linbit.com>
To: Jason Gunthorpe <jgg@nvidia.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: Thu, 25 Sep 2025 08:02:38 +0200 [thread overview]
Message-ID: <CADGDV=XkfuNhATA4GkQm1VBVaG+JkFYmXojaVSnGo=rco7bUyQ@mail.gmail.com> (raw)
In-Reply-To: <20250924132135.GA2653699@nvidia.com>
Hi Jason,
On Wed, Sep 24, 2025 at 3:21 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> 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 for following up. Sure, I will send a new version of it, it will
be v3, as this is already the discussion on v2.
Best regards,
Philipp
prev parent reply other threads:[~2025-09-25 6:02 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
2025-09-25 6:02 ` Philipp Reisner [this message]
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='CADGDV=XkfuNhATA4GkQm1VBVaG+JkFYmXojaVSnGo=rco7bUyQ@mail.gmail.com' \
--to=philipp.reisner@linbit.com \
--cc=jgg@nvidia.com \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--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).