[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eeab9242-6968-4d44-b765-d4876b353b62@grimberg.me>
Date: Sat, 27 Dec 2025 11:39:55 +0200
From: Sagi Grimberg <sagi@...mberg.me>
To: Mohamed Khalfella <mkhalfella@...estorage.com>
Cc: Chaitanya Kulkarni <kch@...dia.com>, Christoph Hellwig <hch@....de>,
Jens Axboe <axboe@...nel.dk>, Keith Busch <kbusch@...nel.org>,
Aaron Dailey <adailey@...estorage.com>,
Randy Jennings <randyj@...estorage.com>, John Meneghini
<jmeneghi@...hat.com>, Hannes Reinecke <hare@...e.de>,
linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 03/14] nvmet: Implement CCR nvme command
>>> +void nvmet_execute_cross_ctrl_reset(struct nvmet_req *req)
>>> +{
>>> + struct nvmet_ctrl *ictrl, *ctrl = req->sq->ctrl;
>>> + struct nvme_command *cmd = req->cmd;
>>> + struct nvmet_ccr *ccr, *new_ccr;
>>> + int ccr_active, ccr_total;
>>> + u16 cntlid, status = 0;
>>> +
>>> + cntlid = le16_to_cpu(cmd->ccr.icid);
>>> + if (ctrl->cntlid == cntlid) {
>>> + req->error_loc =
>>> + offsetof(struct nvme_cross_ctrl_reset_cmd, icid);
>>> + status = NVME_SC_INVALID_FIELD | NVME_STATUS_DNR;
>>> + goto out;
>>> + }
>>> +
>>> + ictrl = nvmet_ctrl_find_get_ccr(ctrl->subsys, ctrl->hostnqn,
>> What does the 'i' stand for?
> 'i' stands for impacted controller. Also, if you see sctrl the 's'
> stands for source controller. These terms are from TP8028.
Can you perhaps add a comment on this?
>>> + new_ccr->ciu = cmd->ccr.ciu;
>>> + new_ccr->icid = cntlid;
>>> + new_ccr->ctrl = ictrl;
>>> + list_add_tail(&new_ccr->entry, &ctrl->ccrs);
>>> + mutex_unlock(&ctrl->lock);
>>> +
>>> + nvmet_ctrl_fatal_error(ictrl);
>> Don't you need to wait for it to complete?
>> e.g. flush_work(&ictrl->fatal_err_work);
>>
>> Or is that done async? will need to look downstream...
> No, we do not need to wait for ictrl->fatal_err_work to complete. An AEN
> will be sent when ictrl exits. It is okay if AEN is sent before CCR
> request is completed. The initiator should expect this behavior and deal
> with it.
Yes, saw that in a later patch (didn't get to do a full review yet)
Powered by blists - more mailing lists