[v3,4/4] nvme: add csi, ms and nuse to sysfs
Commit Message
libnvme is using the sysfs for enumarating the nvme resources. Though
there are few missing attritbutes in the sysfs. For these libnvme issues
commands during discovering.
As the kernel already knows all these attributes and we would like to
avoid libnvme to issue commands all the time, expose these missing
attributes.
The nuse value is updated on request because the nuse is a volatile
value. Since any user can read the sysfs attribute, a very simple rate
limit is added (update max every 5 seconds). A more sophisticated update
strategy can be added later if there is actually a need for it.
Signed-off-by: Daniel Wagner <dwagner@suse.de>
---
drivers/nvme/host/core.c | 28 ++++++++++++++++++++++++++++
drivers/nvme/host/nvme.h | 2 ++
drivers/nvme/host/sysfs.c | 31 +++++++++++++++++++++++++++++++
3 files changed, 61 insertions(+)
Comments
On Wed, Dec 06, 2023 at 09:12:44AM +0100, Daniel Wagner wrote:
> libnvme is using the sysfs for enumarating the nvme resources. Though
> there are few missing attritbutes in the sysfs. For these libnvme issues
> commands during discovering.
>
> As the kernel already knows all these attributes and we would like to
> avoid libnvme to issue commands all the time, expose these missing
> attributes.
>
> The nuse value is updated on request because the nuse is a volatile
> value. Since any user can read the sysfs attribute, a very simple rate
> limit is added (update max every 5 seconds). A more sophisticated update
> strategy can be added later if there is actually a need for it.
>
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
> drivers/nvme/host/core.c | 28 ++++++++++++++++++++++++++++
> drivers/nvme/host/nvme.h | 2 ++
> drivers/nvme/host/sysfs.c | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 61 insertions(+)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index caa52c2f57c8..e7dd64ee1653 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1663,6 +1663,33 @@ static void nvme_ns_release(struct nvme_ns *ns)
> nvme_put_ns(ns);
> }
>
> +int nvme_ns_update_nuse(struct nvme_ns_head *head)
> +{
> + static DEFINE_RATELIMIT_STATE(_rs, 5 * HZ, 1);
> + struct nvme_id_ns *id;
> + struct nvme_ns *ns;
> + int srcu_idx, ret = -EWOULDBLOCK;
> +
> + if (__ratelimit(&_rs))
> + return 0;
Can you add a comment on the ratelimiting here?
> +
> + srcu_idx = srcu_read_lock(&head->srcu);
> + ns = nvme_find_path(head);
> + if (!ns)
> + goto out_unlock;
> +
> + ret = nvme_identify_ns(ns->ctrl, head->ns_id, &id);
> + if (ret)
> + goto out_unlock;
> +
> + head->nuse = le64_to_cpu(id->nuse);
This looks like the wrong thing to do for the non-multipath nodes,
which should be able to go straight to the ns.
I'd move this to sysfs.c, and then do a similar trick to say
nvme_send_pr_command to directly use the ns for the non-multipath
nodes, and do what you're doing here for the multipath nodes.
> static struct attribute *nvme_ns_id_attrs[] = {
And I duess the _id is not correct now, I'd just drop it.
@@ -1663,6 +1663,33 @@ static void nvme_ns_release(struct nvme_ns *ns)
nvme_put_ns(ns);
}
+int nvme_ns_update_nuse(struct nvme_ns_head *head)
+{
+ static DEFINE_RATELIMIT_STATE(_rs, 5 * HZ, 1);
+ struct nvme_id_ns *id;
+ struct nvme_ns *ns;
+ int srcu_idx, ret = -EWOULDBLOCK;
+
+ if (__ratelimit(&_rs))
+ return 0;
+
+ srcu_idx = srcu_read_lock(&head->srcu);
+ ns = nvme_find_path(head);
+ if (!ns)
+ goto out_unlock;
+
+ ret = nvme_identify_ns(ns->ctrl, head->ns_id, &id);
+ if (ret)
+ goto out_unlock;
+
+ head->nuse = le64_to_cpu(id->nuse);
+ kfree(id);
+
+out_unlock:
+ srcu_read_unlock(&head->srcu, srcu_idx);
+ return ret;
+}
+
static int nvme_open(struct gendisk *disk, blk_mode_t mode)
{
return nvme_ns_open(disk->private_data);
@@ -2068,6 +2095,7 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
blk_mq_freeze_queue(ns->disk->queue);
lbaf = nvme_lbaf_index(id->flbas);
ns->head->lba_shift = id->lbaf[lbaf].ds;
+ ns->head->nuse = le64_to_cpu(id->nuse);
nvme_set_queue_limits(ns->ctrl, ns->queue);
ret = nvme_configure_metadata(ns, id);
@@ -456,6 +456,7 @@ struct nvme_ns_head {
u16 pi_size;
u16 sgs;
u32 sws;
+ u64 nuse;
u8 pi_type;
u8 guard_type;
#ifdef CONFIG_BLK_DEV_ZONED
@@ -867,6 +868,7 @@ int nvme_ns_chr_uring_cmd(struct io_uring_cmd *ioucmd,
unsigned int issue_flags);
int nvme_ns_head_chr_uring_cmd(struct io_uring_cmd *ioucmd,
unsigned int issue_flags);
+int nvme_ns_update_nuse(struct nvme_ns_head *head);
int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo);
int nvme_dev_uring_cmd(struct io_uring_cmd *ioucmd, unsigned int issue_flags);
@@ -114,12 +114,43 @@ static ssize_t nsid_show(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RO(nsid);
+static ssize_t csi_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "%d\n", dev_to_ns_head(dev)->ids.csi);
+}
+static DEVICE_ATTR_RO(csi);
+
+static ssize_t metadata_bytes_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "%d\n", dev_to_ns_head(dev)->ms);
+}
+static DEVICE_ATTR_RO(metadata_bytes);
+
+static ssize_t nuse_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct nvme_ns_head *head = dev_to_ns_head(dev);
+ int ret;
+
+ ret = nvme_ns_update_nuse(head);
+ if (ret)
+ return ret;
+
+ return sysfs_emit(buf, "%llu\n", head->nuse);
+}
+static DEVICE_ATTR_RO(nuse);
+
static struct attribute *nvme_ns_id_attrs[] = {
&dev_attr_wwid.attr,
&dev_attr_uuid.attr,
&dev_attr_nguid.attr,
&dev_attr_eui.attr,
+ &dev_attr_csi.attr,
&dev_attr_nsid.attr,
+ &dev_attr_metadata_bytes.attr,
+ &dev_attr_nuse.attr,
#ifdef CONFIG_NVME_MULTIPATH
&dev_attr_ana_grpid.attr,
&dev_attr_ana_state.attr,