From: Himanshu Madhani <himanshu.madhani@oracle.com>
To: Keith Busch <kbusch@kernel.org>
Cc: axboe@kernel.dk, sagi@grimberg.me,
"Keith Busch" <keith.busch@wdc.com>,
"Johannes Thumshirn" <johannes.thumshirn@wdc.com>,
"Daniel Wagner" <dwagner@suse.de>,
linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
"Hannes Reinecke" <hare@suse.de>,
"Martin K . Petersen" <martin.petersen@oracle.com>,
"Javier González" <javier.gonz@samsung.com>,
hch@lst.de, "Matias Bjørling" <matias.bjorling@wdc.com>
Subject: Re: [PATCHv4 4/5] nvme: support for multi-command set effects
Date: Tue, 30 Jun 2020 10:51:40 -0500 [thread overview]
Message-ID: <6EDBAF48-6130-4D33-8200-4905CD15244E@oracle.com> (raw)
In-Reply-To: <20200629190641.1986462-5-kbusch@kernel.org>
> On Jun 29, 2020, at 2:06 PM, Keith Busch <kbusch@kernel.org> wrote:
>
> From: Keith Busch <keith.busch@wdc.com>
>
> The Commands Supported and Effects log page was extended with a CSI
> field that enables the host to query the log page for each command set
> supported. Retrieve this log page for each command set that an attached
> namespace supports, and save a pointer to that log in the namespace head.
>
> Reviewed-by: Javier González <javier.gonz@samsung.com>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Reviewed-by: Matias Bjørling <matias.bjorling@wdc.com>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Daniel Wagner <dwagner@suse.de>
> Signed-off-by: Keith Busch <keith.busch@wdc.com>
> ---
> drivers/nvme/host/core.c | 79 ++++++++++++++++++++++++++---------
> drivers/nvme/host/hwmon.c | 2 +-
> drivers/nvme/host/lightnvm.c | 4 +-
> drivers/nvme/host/multipath.c | 2 +-
> drivers/nvme/host/nvme.h | 10 ++++-
> include/linux/nvme.h | 4 +-
> 6 files changed, 76 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index b7d12eb42fd8..a4a61d7e793d 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1370,8 +1370,8 @@ static u32 nvme_passthru_start(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> u32 effects = 0;
>
> if (ns) {
> - if (ctrl->effects)
> - effects = le32_to_cpu(ctrl->effects->iocs[opcode]);
> + if (ns->head->effects)
> + effects = le32_to_cpu(ns->head->effects->iocs[opcode]);
> if (effects & ~(NVME_CMD_EFFECTS_CSUPP | NVME_CMD_EFFECTS_LBCC))
> dev_warn(ctrl->device,
> "IO command:%02x has unhandled effects:%08x\n",
> @@ -2850,7 +2850,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
> return ret;
> }
>
> -int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
> +int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
> void *log, size_t size, u64 offset)
> {
> struct nvme_command c = { };
> @@ -2864,27 +2864,55 @@ int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
> c.get_log_page.numdu = cpu_to_le16(dwlen >> 16);
> c.get_log_page.lpol = cpu_to_le32(lower_32_bits(offset));
> c.get_log_page.lpou = cpu_to_le32(upper_32_bits(offset));
> + c.get_log_page.csi = csi;
>
> return nvme_submit_sync_cmd(ctrl->admin_q, &c, log, size);
> }
>
> -static int nvme_get_effects_log(struct nvme_ctrl *ctrl)
> +struct nvme_cel *nvme_find_cel(struct nvme_ctrl *ctrl, u8 csi)
> {
> + struct nvme_cel *cel, *ret = NULL;
> +
> + spin_lock(&ctrl->lock);
> + list_for_each_entry(cel, &ctrl->cels, entry) {
> + if (cel->csi == csi) {
> + ret = cel;
> + break;
> + }
> + }
> + spin_unlock(&ctrl->lock);
> +
> + return ret;
> +}
> +
> +static int nvme_get_effects_log(struct nvme_ctrl *ctrl, u8 csi,
> + struct nvme_effects_log **log)
> +{
> + struct nvme_cel *cel = nvme_find_cel(ctrl, csi);
> int ret;
>
> - if (!ctrl->effects)
> - ctrl->effects = kzalloc(sizeof(*ctrl->effects), GFP_KERNEL);
> + if (cel)
> + goto out;
>
> - if (!ctrl->effects)
> - return 0;
> + cel = kzalloc(sizeof(*cel), GFP_KERNEL);
> + if (!cel)
> + return -ENOMEM;
>
> - ret = nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_CMD_EFFECTS, 0,
> - ctrl->effects, sizeof(*ctrl->effects), 0);
> + ret = nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_CMD_EFFECTS, 0, csi,
> + &cel->log, sizeof(cel->log), 0);
> if (ret) {
> - kfree(ctrl->effects);
> - ctrl->effects = NULL;
> + kfree(cel);
> + return ret;
> }
> - return ret;
> +
> + cel->csi = csi;
> +
> + spin_lock(&ctrl->lock);
> + list_add_tail(&cel->entry, &ctrl->cels);
> + spin_unlock(&ctrl->lock);
> +out:
> + *log = &cel->log;
> + return 0;
> }
>
> /*
> @@ -2917,7 +2945,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
> }
>
> if (id->lpa & NVME_CTRL_LPA_CMD_EFFECTS_LOG) {
> - ret = nvme_get_effects_log(ctrl);
> + ret = nvme_get_effects_log(ctrl, NVME_CSI_NVM, &ctrl->effects);
> if (ret < 0)
> goto out_free;
> }
> @@ -3550,6 +3578,13 @@ static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
> goto out_cleanup_srcu;
> }
>
> + if (head->ids.csi) {
> + ret = nvme_get_effects_log(ctrl, head->ids.csi, &head->effects);
> + if (ret)
> + goto out_cleanup_srcu;
> + } else
> + head->effects = ctrl->effects;
> +
> ret = nvme_mpath_alloc_disk(ctrl, head);
> if (ret)
> goto out_cleanup_srcu;
> @@ -3890,8 +3925,8 @@ static void nvme_clear_changed_ns_log(struct nvme_ctrl *ctrl)
> * raced with us in reading the log page, which could cause us to miss
> * updates.
> */
> - error = nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_CHANGED_NS, 0, log,
> - log_size, 0);
> + error = nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_CHANGED_NS, 0,
> + NVME_CSI_NVM, log, log_size, 0);
> if (error)
> dev_warn(ctrl->device,
> "reading changed ns log failed: %d\n", error);
> @@ -4035,8 +4070,8 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
> if (!log)
> return;
>
> - if (nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_FW_SLOT, 0, log,
> - sizeof(*log), 0))
> + if (nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_FW_SLOT, 0, NVME_CSI_NVM,
> + log, sizeof(*log), 0))
> dev_warn(ctrl->device, "Get FW SLOT INFO log error\n");
> kfree(log);
> }
> @@ -4173,11 +4208,16 @@ static void nvme_free_ctrl(struct device *dev)
> struct nvme_ctrl *ctrl =
> container_of(dev, struct nvme_ctrl, ctrl_device);
> struct nvme_subsystem *subsys = ctrl->subsys;
> + struct nvme_cel *cel, *next;
>
> if (subsys && ctrl->instance != subsys->instance)
> ida_simple_remove(&nvme_instance_ida, ctrl->instance);
>
> - kfree(ctrl->effects);
> + list_for_each_entry_safe(cel, next, &ctrl->cels, entry) {
> + list_del(&cel->entry);
> + kfree(cel);
> + }
> +
> nvme_mpath_uninit(ctrl);
> __free_page(ctrl->discard_page);
>
> @@ -4208,6 +4248,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> spin_lock_init(&ctrl->lock);
> mutex_init(&ctrl->scan_lock);
> INIT_LIST_HEAD(&ctrl->namespaces);
> + INIT_LIST_HEAD(&ctrl->cels);
> init_rwsem(&ctrl->namespaces_rwsem);
> ctrl->dev = dev;
> ctrl->ops = ops;
> diff --git a/drivers/nvme/host/hwmon.c b/drivers/nvme/host/hwmon.c
> index 2e6477ed420f..23ba8bf678ae 100644
> --- a/drivers/nvme/host/hwmon.c
> +++ b/drivers/nvme/host/hwmon.c
> @@ -62,7 +62,7 @@ static int nvme_hwmon_get_smart_log(struct nvme_hwmon_data *data)
> int ret;
>
> ret = nvme_get_log(data->ctrl, NVME_NSID_ALL, NVME_LOG_SMART, 0,
> - &data->log, sizeof(data->log), 0);
> + NVME_CSI_NVM, &data->log, sizeof(data->log), 0);
>
> return ret <= 0 ? ret : -EIO;
> }
> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
> index 69608755d415..8e562d0f2c30 100644
> --- a/drivers/nvme/host/lightnvm.c
> +++ b/drivers/nvme/host/lightnvm.c
> @@ -593,8 +593,8 @@ static int nvme_nvm_get_chk_meta(struct nvm_dev *ndev,
> dev_meta_off = dev_meta;
>
> ret = nvme_get_log(ctrl, ns->head->ns_id,
> - NVME_NVM_LOG_REPORT_CHUNK, 0, dev_meta, len,
> - offset);
> + NVME_NVM_LOG_REPORT_CHUNK, 0, NVME_CSI_NVM,
> + dev_meta, len, offset);
> if (ret) {
> dev_err(ctrl->device, "Get REPORT CHUNK log error\n");
> break;
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index 18d084ed497e..b551e9884430 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -529,7 +529,7 @@ static int nvme_read_ana_log(struct nvme_ctrl *ctrl)
> int error;
>
> mutex_lock(&ctrl->ana_lock);
> - error = nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_ANA, 0,
> + error = nvme_get_log(ctrl, NVME_NSID_ALL, NVME_LOG_ANA, 0, NVME_CSI_NVM,
> ctrl->ana_log_buf, ctrl->ana_log_size, 0);
> if (error) {
> dev_warn(ctrl->device, "Failed to get ANA log: %d\n", error);
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 4aa321b16eaa..dd24a94c42ff 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -191,6 +191,12 @@ struct nvme_fault_inject {
> #endif
> };
>
> +struct nvme_cel {
> + struct list_head entry;
> + struct nvme_effects_log log;
> + u8 csi;
> +};
> +
> struct nvme_ctrl {
> bool comp_seen;
> enum nvme_ctrl_state state;
> @@ -257,6 +263,7 @@ struct nvme_ctrl {
> unsigned long quirks;
> struct nvme_id_power_state psd[32];
> struct nvme_effects_log *effects;
> + struct list_head cels;
> struct work_struct scan_work;
> struct work_struct async_event_work;
> struct delayed_work ka_work;
> @@ -359,6 +366,7 @@ struct nvme_ns_head {
> struct kref ref;
> bool shared;
> int instance;
> + struct nvme_effects_log *effects;
> #ifdef CONFIG_NVME_MULTIPATH
> struct gendisk *disk;
> struct bio_list requeue_list;
> @@ -561,7 +569,7 @@ int nvme_reset_ctrl_sync(struct nvme_ctrl *ctrl);
> int nvme_try_sched_reset(struct nvme_ctrl *ctrl);
> int nvme_delete_ctrl(struct nvme_ctrl *ctrl);
>
> -int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp,
> +int nvme_get_log(struct nvme_ctrl *ctrl, u32 nsid, u8 log_page, u8 lsp, u8 csi,
> void *log, size_t size, u64 offset);
>
> extern const struct attribute_group *nvme_ns_id_attr_groups[];
> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index 81ffe5247505..95cd03e240a1 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -1101,7 +1101,9 @@ struct nvme_get_log_page_command {
> };
> __le64 lpo;
> };
> - __u32 rsvd14[2];
> + __u8 rsvd14[3];
> + __u8 csi;
> + __u32 rsvd15;
> };
>
> struct nvme_directive_cmd {
> --
> 2.24.1
>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
--
Himanshu Madhani Oracle Linux Engineering
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-06-30 15:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-29 19:06 [PATCHv4 0/5] nvme support for zoned namespace command set Keith Busch
2020-06-29 19:06 ` [PATCHv4 1/5] block: add capacity field to zone descriptors Keith Busch
2020-06-30 15:34 ` Himanshu Madhani
2020-06-29 19:06 ` [PATCHv4 2/5] null_blk: introduce zone capacity for zoned device Keith Busch
2020-06-30 15:34 ` Himanshu Madhani
2020-06-29 19:06 ` [PATCHv4 3/5] nvme: implement I/O Command Sets Command Set support Keith Busch
2020-06-30 6:01 ` Hannes Reinecke
2020-06-30 15:45 ` Himanshu Madhani
2020-06-29 19:06 ` [PATCHv4 4/5] nvme: support for multi-command set effects Keith Busch
2020-06-30 15:51 ` Himanshu Madhani [this message]
2020-06-29 19:06 ` [PATCHv4 5/5] nvme: support for zoned namespaces Keith Busch
2020-06-30 16:36 ` Himanshu Madhani
2020-07-01 6:37 ` [PATCHv4 0/5] nvme support for zoned namespace command set Christoph Hellwig
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=6EDBAF48-6130-4D33-8200-4905CD15244E@oracle.com \
--to=himanshu.madhani@oracle.com \
--cc=axboe@kernel.dk \
--cc=dwagner@suse.de \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=javier.gonz@samsung.com \
--cc=johannes.thumshirn@wdc.com \
--cc=kbusch@kernel.org \
--cc=keith.busch@wdc.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=martin.petersen@oracle.com \
--cc=matias.bjorling@wdc.com \
--cc=sagi@grimberg.me \
/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).