| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20260203200725.GF3729-mkhalfella@purestorage.com>
Date: Tue, 3 Feb 2026 12:07:25 -0800
From: Mohamed Khalfella <mkhalfella@...estorage.com>
To: Hannes Reinecke <hare@...e.de>
Cc: Justin Tee <justin.tee@...adcom.com>,
Naresh Gottumukkala <nareshgottumukkala83@...il.com>,
Paul Ely <paul.ely@...adcom.com>,
Chaitanya Kulkarni <kch@...dia.com>, Christoph Hellwig <hch@....de>,
Jens Axboe <axboe@...nel.dk>, Keith Busch <kbusch@...nel.org>,
Sagi Grimberg <sagi@...mberg.me>,
Aaron Dailey <adailey@...estorage.com>,
Randy Jennings <randyj@...estorage.com>,
Dhaval Giani <dgiani@...estorage.com>,
linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 09/14] nvme: Implement cross-controller reset
completion
On Tue 2026-02-03 06:22:38 +0100, Hannes Reinecke wrote:
> On 1/30/26 23:34, Mohamed Khalfella wrote:
> > An nvme source controller that issues CCR command expects to receive an
> > NVME_AER_NOTICE_CCR_COMPLETED when pending CCR succeeds or fails. Add
> > sctrl->ccr_work to read NVME_LOG_CCR logpage and wakeup any thread
> > waiting on CCR completion.
> >
> > Signed-off-by: Mohamed Khalfella <mkhalfella@...estorage.com>
> > ---
> > drivers/nvme/host/core.c | 49 +++++++++++++++++++++++++++++++++++++++-
> > drivers/nvme/host/nvme.h | 1 +
> > 2 files changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 13e0775d56b4..0f90feb46369 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -1901,7 +1901,8 @@ EXPORT_SYMBOL_GPL(nvme_set_queue_count);
> >
> > #define NVME_AEN_SUPPORTED \
> > (NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | \
> > - NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_DISC_CHANGE)
> > + NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_CCR_COMPLETE | \
> > + NVME_AEN_CFG_DISC_CHANGE)
> >
> > static void nvme_enable_aen(struct nvme_ctrl *ctrl)
> > {
> > @@ -4866,6 +4867,47 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
> > kfree(log);
> > }
> >
> > +static void nvme_ccr_work(struct work_struct *work)
> > +{
> > + struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, ccr_work);
> > + struct nvme_ccr_entry *ccr;
> > + struct nvme_ccr_log_entry *entry;
> > + struct nvme_ccr_log *log;
> > + unsigned long flags;
> > + int ret, i;
> > +
> > + log = kmalloc(sizeof(*log), GFP_KERNEL);
> > + if (!log)
> > + return;
> > +
> > + ret = nvme_get_log(ctrl, 0, NVME_LOG_CCR, 0x01,
> > + 0x00, log, sizeof(*log), 0);
> > + if (ret)
> > + goto out;
> > +
> > + spin_lock_irqsave(&ctrl->lock, flags);
> > + for (i = 0; i < le16_to_cpu(log->ne); i++) {
> > + entry = &log->entries[i];
> > + if (entry->ccrs == NVME_CCR_STATUS_IN_PROGRESS)
> > + continue;
> > +
> > + list_for_each_entry(ccr, &ctrl->ccr_list, list) {
> > + struct nvme_ctrl *ictrl = ccr->ictrl;
> > +
> > + if (ictrl->cntlid != le16_to_cpu(entry->icid) ||
> > + ictrl->ciu != entry->ciu)
> > + continue;
> > +
> > + /* Complete matching entry */
> > + ccr->ccrs = entry->ccrs;
> > + complete(&ccr->complete);
> > + }
> > + }
> > + spin_unlock_irqrestore(&ctrl->lock, flags);
> > +out:
> > + kfree(log);
> > +}
> > +
> > static void nvme_fw_act_work(struct work_struct *work)
> > {
> > struct nvme_ctrl *ctrl = container_of(work,
> > @@ -4942,6 +4984,9 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
> > case NVME_AER_NOTICE_DISC_CHANGED:
> > ctrl->aen_result = result;
> > break;
> > + case NVME_AER_NOTICE_CCR_COMPLETED:
> > + queue_work(nvme_wq, &ctrl->ccr_work);
> > + break;
> > default:
> > dev_warn(ctrl->device, "async event result %08x\n", result);
> > }
> > @@ -5131,6 +5176,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)
> > nvme_stop_failfast_work(ctrl);
> > flush_work(&ctrl->async_event_work);
> > cancel_work_sync(&ctrl->fw_act_work);
> > + cancel_work_sync(&ctrl->ccr_work);
> > if (ctrl->ops->stop_ctrl)
> > ctrl->ops->stop_ctrl(ctrl);
> > }
> > @@ -5254,6 +5300,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> > ctrl->quirks = quirks;
> > ctrl->numa_node = NUMA_NO_NODE;
> > INIT_WORK(&ctrl->scan_work, nvme_scan_work);
> > + INIT_WORK(&ctrl->ccr_work, nvme_ccr_work);
> > INIT_WORK(&ctrl->async_event_work, nvme_async_event_work);
> > INIT_WORK(&ctrl->fw_act_work, nvme_fw_act_work);
> > INIT_WORK(&ctrl->delete_work, nvme_delete_ctrl_work);
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> > index fa18f580d76a..a7f382e35821 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -366,6 +366,7 @@ struct nvme_ctrl {
> > struct nvme_effects_log *effects;
> > struct xarray cels;
> > struct work_struct scan_work;
> > + struct work_struct ccr_work;
> > struct work_struct async_event_work;
> > struct delayed_work ka_work;
> > struct delayed_work failfast_work;
>
> This confuses me. Why do we call 'complete()' but do not have a
> corresponding 'wait_for_completion()' call?
I think because the two commits were developed separately.
>
> Please merge with the next patch to allow reviewers to have the full
> picture.
I thought it is easier to review this way because this change includes
ctrl->ccr_work. If you feel strongly about it I will merge it into the
previous patch "[PATCH v2 08/14] nvme: Implement cross-controller reset
recovery"?
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke Kernel Storage Architect
> hare@...e.de +49 911 74053 688
> SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Powered by blists - more mailing lists