mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Nilay Shroff <nilay@linux.ibm.com>
To: Cen Zhang <zzzccc427@gmail.com>, Keith Busch <kbusch@kernel.org>,
	Jens Axboe <axboe@kernel.dk>, Christoph Hellwig <hch@lst.de>,
	Sagi Grimberg <sagi@grimberg.me>,
	Steve Wise <larrystevenwise@gmail.com>
Cc: linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
	baijiaju1990@gmail.com
Subject: Re: [PATCH] nvme-rdma: avoid stale device wrapper in remove_one
Date: Mon, 22 Jun 2026 18:12:30 +0530	[thread overview]
Message-ID: <3ba6ae70-9541-455b-932f-3a44f7a576a3@linux.ibm.com> (raw)
In-Reply-To: <20260621135939.2658295-1-zzzccc427@gmail.com>

On 6/21/26 7:29 PM, Cen Zhang wrote:
> nvme_rdma_remove_one() walks nvme_rdma_ctrl_list under
> nvme_rdma_ctrl_mutex, but it identifies matching controllers by reading
> ctrl->device->dev.  The mutex only protects controller list membership.
> ctrl->device is a cached copy of queue 0's nvme_rdma_device, and that
> wrapper is refcounted by queue lifetime.
> 
> The buggy scenario involves two paths, with each column showing the order
> within that path:
> 
> RDMA remove callback:                Controller error recovery:
>    1. enter nvme_rdma_remove_one()    1. run nvme_rdma_error_recovery_work()
>    2. walk nvme_rdma_ctrl_list        2. tear down the admin queue
>    3. read ctrl->device->dev          3. drop the final queue device ref
>                                        4. free the nvme_rdma_device wrapper
> 
> Fix this by caching the ib_device identity in the controller when the
> admin queue is configured.  The remove callback can then compare the
> cached pointer value against the ib_device being removed without
> dereferencing the queue-owned nvme_rdma_device wrapper.  Keep the delete
> workqueue flush conditional on actually matching a controller.
> 

I think, instead of caching ib_device in struct nvme_rdma_ctrl, we can take a
reference on the matching nvme_rdma_device (or ndev) while walking device_list.
As the reference is taken, the rdma device would not be freed until we put the
reference or while we loops through the ctrl list.

Then, loop through the ctrl list and compare ctrl->device directly against that
matching ndev. That would avoid dereferencing ctrl->device->dev while reusing
the existing kref-based lifetime management for nvme_rdma_device, rather than
introducing a separate cached ib_device pointer. At the end, we can drop the
reference to ndev.

Thanks,
--Nilay




  reply	other threads:[~2026-06-22 12:42 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-21 13:59 Cen Zhang
2026-06-22 12:42 ` Nilay Shroff [this message]
2026-06-22 23:12   ` Cen Zhang

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=3ba6ae70-9541-455b-932f-3a44f7a576a3@linux.ibm.com \
    --to=nilay@linux.ibm.com \
    --cc=axboe@kernel.dk \
    --cc=baijiaju1990@gmail.com \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=larrystevenwise@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    --cc=zzzccc427@gmail.com \
    /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

all inboxes | Powered by JetHome®