Re: [PATCH RFC] nvmet-tcp: add new workqueue to surpress lockdep warning

[Guoqing Jiang <guoqing.jiang@linux.dev>]

On 9/8/23 16:08, Yi Zhang wrote:
> On Thu, Sep 7, 2023 at 4:12 PM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Hi Yi,
>>
>> On 9/7/23 14:41, Yi Zhang wrote:
>>> Tested-by: Yi Zhang <yi.zhang@redhat.com>
>>>
>>> Confirmed below issue was fixed by this patch:
>> Thanks a lot for the test!
>>
>> And I had got another similar lockdep warning about nvmet-rdma.
> Yeah, I've filed this one last year, but still not get one solution.
> https://lore.kernel.org/linux-rdma/13441b9b-cc13-f0e0-bd46-f14983dadd49@grimberg.me/

Hmm, it is a false alarm given the conn_id in cma_cm_event_handler is 
just created
by cma_ib_new_conn_id, so  conn_id should have different handler_mutex 
with listen_id
(which is the parent of conn_id)  though they share the same 
event_handler. And the
cm_id (listen_id) in the chain

nvmet_rdma_free_queue -> nvmet_rdma_destroy_queue_ib -> 
rdma_destroy_id(queue->cm_id)

should be created by rdma_create_idin nvmet_rdma_enable_port.

And the similar thing also had happened to rtrs.

https://lore.kernel.org/linux-rdma/CAMGffEm22sP-oKK0D9=vOw77nbS05iwG7MC3DTVB0CyzVFhtXg@mail.gmail.com/#t

>> Besides make similar change like this patch, another option might be
>> check queue state before
>> flush workqueue. Thoughts?
>>
>> --- a/drivers/nvme/target/rdma.c
>> +++ b/drivers/nvme/target/rdma.c
>> @@ -1582,7 +1582,8 @@ static int nvmet_rdma_queue_connect(struct
>> rdma_cm_id *cm_id,
>>                   goto put_device;
>>           }
>>
>> -       if (queue->host_qid == 0) {
>> +       if (queue->state == NVMET_RDMA_Q_LIVE &&
>> +           queue->host_qid == 0) {
>>                   /* Let inflight controller teardown complete */
>>                   flush_workqueue(nvmet_wq);
>>           }

I guess the above can suppress the false alarm as well or similar change 
as in this RFC.

BTW, I suggest we need to move forward from the lockdep's report by 
either suppress it
or ignore it to save people's time.

Thanks,
Guoqing