* Re: [PATCH v1 2/2] virtio_fs: add sysfs entries for queue information
[not found] ` <20240825130716.9506-2-mgurtovoy@nvidia.com>
@ 2024-09-05 12:12 ` Michael S. Tsirkin
0 siblings, 0 replies; 3+ messages in thread
From: Michael S. Tsirkin @ 2024-09-05 12:12 UTC (permalink / raw)
To: Max Gurtovoy
Cc: stefanha, virtualization, Miklos Szeredi, Vivek Goyal, kvm,
Jingbo Xu, pgootzen, smalin, larora, ialroy, oren, izach,
Eugenio Pérez
Cc: "Eugenio Pérez" <eperezma@redhat.com>
On Sun, Aug 25, 2024 at 04:07:16PM +0300, Max Gurtovoy wrote:
> Introduce sysfs entries to provide visibility to the multiple queues
> used by the Virtio FS device. This enhancement allows users to query
> information about these queues.
>
> Specifically, add two sysfs entries:
> 1. Queue name: Provides the name of each queue (e.g. hiprio/requests.8).
> 2. CPU list: Shows the list of CPUs that can process requests for each
> queue.
>
> The CPU list feature is inspired by similar functionality in the block
> MQ layer, which provides analogous sysfs entries for block devices.
>
> These new sysfs entries will improve observability and aid in debugging
> and performance tuning of Virtio FS devices.
>
> Reviewed-by: Idan Zach <izach@nvidia.com>
> Reviewed-by: Shai Malin <smalin@nvidia.com>
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
> fs/fuse/virtio_fs.c | 147 +++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 139 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 43f7be1d7887..78f579463cca 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -56,12 +56,14 @@ struct virtio_fs_vq {
> bool connected;
> long in_flight;
> struct completion in_flight_zero; /* No inflight requests */
> + struct kobject *kobj;
> char name[VQ_NAME_LEN];
> } ____cacheline_aligned_in_smp;
>
> /* A virtio-fs device instance */
> struct virtio_fs {
> struct kobject kobj;
> + struct kobject *mqs_kobj;
> struct list_head list; /* on virtio_fs_instances */
> char *tag;
> struct virtio_fs_vq *vqs;
> @@ -200,6 +202,74 @@ static const struct kobj_type virtio_fs_ktype = {
> .default_groups = virtio_fs_groups,
> };
>
> +static struct virtio_fs_vq *virtio_fs_kobj_to_vq(struct virtio_fs *fs,
> + struct kobject *kobj)
> +{
> + int i;
> +
> + for (i = 0; i < fs->nvqs; i++) {
> + if (kobj == fs->vqs[i].kobj)
> + return &fs->vqs[i];
> + }
> + return NULL;
> +}
> +
> +static ssize_t name_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct virtio_fs *fs = container_of(kobj->parent->parent, struct virtio_fs, kobj);
> + struct virtio_fs_vq *fsvq = virtio_fs_kobj_to_vq(fs, kobj);
> +
> + if (!fsvq)
> + return -EINVAL;
> + return sysfs_emit(buf, "%s\n", fsvq->name);
> +}
> +
> +static struct kobj_attribute virtio_fs_vq_name_attr = __ATTR_RO(name);
> +
> +static ssize_t cpu_list_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + struct virtio_fs *fs = container_of(kobj->parent->parent, struct virtio_fs, kobj);
> + struct virtio_fs_vq *fsvq = virtio_fs_kobj_to_vq(fs, kobj);
> + unsigned int cpu, qid;
> + const size_t size = PAGE_SIZE - 1;
> + bool first = true;
> + int ret = 0, pos = 0;
> +
> + if (!fsvq)
> + return -EINVAL;
> +
> + qid = fsvq->vq->index;
> + for (cpu = 0; cpu < nr_cpu_ids; cpu++) {
> + if (qid < VQ_REQUEST || (fs->mq_map[cpu] == qid - VQ_REQUEST)) {
> + if (first)
> + ret = snprintf(buf + pos, size - pos, "%u", cpu);
> + else
> + ret = snprintf(buf + pos, size - pos, ", %u", cpu);
> +
> + if (ret >= size - pos)
> + break;
> + first = false;
> + pos += ret;
> + }
> + }
> + ret = snprintf(buf + pos, size + 1 - pos, "\n");
> + return pos + ret;
> +}
> +
> +static struct kobj_attribute virtio_fs_vq_cpu_list_attr = __ATTR_RO(cpu_list);
> +
> +static struct attribute *virtio_fs_vq_attrs[] = {
> + &virtio_fs_vq_name_attr.attr,
> + &virtio_fs_vq_cpu_list_attr.attr,
> + NULL
> +};
> +
> +static struct attribute_group virtio_fs_vq_attr_group = {
> + .attrs = virtio_fs_vq_attrs,
> +};
> +
> /* Make sure virtiofs_mutex is held */
> static void virtio_fs_put_locked(struct virtio_fs *fs)
> {
> @@ -280,6 +350,50 @@ static void virtio_fs_start_all_queues(struct virtio_fs *fs)
> }
> }
>
> +static void virtio_fs_delete_queues_sysfs(struct virtio_fs *fs)
> +{
> + struct virtio_fs_vq *fsvq;
> + int i;
> +
> + for (i = 0; i < fs->nvqs; i++) {
> + fsvq = &fs->vqs[i];
> + kobject_put(fsvq->kobj);
> + }
> +}
> +
> +static int virtio_fs_add_queues_sysfs(struct virtio_fs *fs)
> +{
> + struct virtio_fs_vq *fsvq;
> + char buff[12];
> + int i, j, ret;
> +
> + for (i = 0; i < fs->nvqs; i++) {
> + fsvq = &fs->vqs[i];
> +
> + sprintf(buff, "%d", i);
> + fsvq->kobj = kobject_create_and_add(buff, fs->mqs_kobj);
> + if (!fs->mqs_kobj) {
> + ret = -ENOMEM;
> + goto out_del;
> + }
> +
> + ret = sysfs_create_group(fsvq->kobj, &virtio_fs_vq_attr_group);
> + if (ret) {
> + kobject_put(fsvq->kobj);
> + goto out_del;
> + }
> + }
> +
> + return 0;
> +
> +out_del:
> + for (j = 0; j < i; j++) {
> + fsvq = &fs->vqs[j];
> + kobject_put(fsvq->kobj);
> + }
> + return ret;
> +}
> +
> /* Add a new instance to the list or return -EEXIST if tag name exists*/
> static int virtio_fs_add_instance(struct virtio_device *vdev,
> struct virtio_fs *fs)
> @@ -303,17 +417,22 @@ static int virtio_fs_add_instance(struct virtio_device *vdev,
> */
> fs->kobj.kset = virtio_fs_kset;
> ret = kobject_add(&fs->kobj, NULL, "%d", vdev->index);
> - if (ret < 0) {
> - mutex_unlock(&virtio_fs_mutex);
> - return ret;
> + if (ret < 0)
> + goto out_unlock;
> +
> + fs->mqs_kobj = kobject_create_and_add("mqs", &fs->kobj);
> + if (!fs->mqs_kobj) {
> + ret = -ENOMEM;
> + goto out_del;
> }
>
> ret = sysfs_create_link(&fs->kobj, &vdev->dev.kobj, "device");
> - if (ret < 0) {
> - kobject_del(&fs->kobj);
> - mutex_unlock(&virtio_fs_mutex);
> - return ret;
> - }
> + if (ret < 0)
> + goto out_put;
> +
> + ret = virtio_fs_add_queues_sysfs(fs);
> + if (ret)
> + goto out_remove;
>
> list_add_tail(&fs->list, &virtio_fs_instances);
>
> @@ -322,6 +441,16 @@ static int virtio_fs_add_instance(struct virtio_device *vdev,
> kobject_uevent(&fs->kobj, KOBJ_ADD);
>
> return 0;
> +
> +out_remove:
> + sysfs_remove_link(&fs->kobj, "device");
> +out_put:
> + kobject_put(fs->mqs_kobj);
> +out_del:
> + kobject_del(&fs->kobj);
> +out_unlock:
> + mutex_unlock(&virtio_fs_mutex);
> + return ret;
> }
>
> /* Return the virtio_fs with a given tag, or NULL */
> @@ -1050,7 +1179,9 @@ static void virtio_fs_remove(struct virtio_device *vdev)
> mutex_lock(&virtio_fs_mutex);
> /* This device is going away. No one should get new reference */
> list_del_init(&fs->list);
> + virtio_fs_delete_queues_sysfs(fs);
> sysfs_remove_link(&fs->kobj, "device");
> + kobject_put(fs->mqs_kobj);
> kobject_del(&fs->kobj);
> virtio_fs_stop_all_queues(fs);
> virtio_fs_drain_all_queues_locked(fs);
> --
> 2.18.1
^ permalink raw reply [flat|nested] 3+ messages in thread