Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
On 5/14/19 7:54 AM, Pankaj Gupta wrote:
> + if (!list_empty(>req_list)) {
> + req_buf = list_first_entry(>req_list,
> + struct virtio_pmem_request, list);
> + req_buf->wq_buf_avail = true;
> + wake_up(_buf->wq_buf);
> + list_del(_buf->list);
Yes, this change is the right one, thank you!
> + /*
> + * If virtqueue_add_sgs returns -ENOSPC then req_vq virtual
> + * queue does not have free descriptor. We add the request
> + * to req_list and wait for host_ack to wake us up when free
> + * slots are available.
> + */
> + while ((err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req,
> + GFP_ATOMIC)) == -ENOSPC) {
> +
> + dev_err(>dev, "failed to send command to virtio pmem" \
> + "device, no free slots in the virtqueue\n");
> + req->wq_buf_avail = false;
> + list_add_tail(>list, >req_list);
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +
> + /* A host response results in "host_ack" getting called */
> + wait_event(req->wq_buf, req->wq_buf_avail);
> + spin_lock_irqsave(>pmem_lock, flags);
> + }
> + err1 = virtqueue_kick(vpmem->req_vq);
> + spin_unlock_irqrestore(>pmem_lock, flags);
> +
> + /*
> + * virtqueue_add_sgs failed with error different than -ENOSPC, we can't
> + * do anything about that.
> + */
> + if (err || !err1) {
> + dev_info(>dev, "failed to send command to virtio pmem
> device\n");
> + err = -EIO;
> + } else {
> + /* A host repsonse results in "host_ack" getting called */
> + wait_event(req->host_acked, req->done);
> + err = req->ret;
> +I confirm that the failures I was facing with the `-ENOSPC` error path are
> not present in v9.
Best,
Jakub Staron
Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
On Wed, May 15, 2019 at 10:46:00PM +0200, David Hildenbrand wrote:
> > + vpmem->vdev = vdev;
> > + vdev->priv = vpmem;
> > + err = init_vq(vpmem);
> > + if (err) {
> > + dev_err(>dev, "failed to initialize virtio pmem vq's\n");
> > + goto out_err;
> > + }
> > +
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + start, >start);
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + size, >size);
> > +
> > + res.start = vpmem->start;
> > + res.end = vpmem->start + vpmem->size-1;
>
> nit: " - 1;"
>
> > + vpmem->nd_desc.provider_name = "virtio-pmem";
> > + vpmem->nd_desc.module = THIS_MODULE;
> > +
> > + vpmem->nvdimm_bus = nvdimm_bus_register(>dev,
> > + >nd_desc);
> > + if (!vpmem->nvdimm_bus) {
> > + dev_err(>dev, "failed to register device with
> > nvdimm_bus\n");
> > + err = -ENXIO;
> > + goto out_vq;
> > + }
> > +
> > + dev_set_drvdata(>dev, vpmem->nvdimm_bus);
> > +
> > + ndr_desc.res =
> > + ndr_desc.numa_node = nid;
> > + ndr_desc.flush = async_pmem_flush;
> > + set_bit(ND_REGION_PAGEMAP, _desc.flags);
> > + set_bit(ND_REGION_ASYNC, _desc.flags);
> > + nd_region = nvdimm_pmem_region_create(vpmem->nvdimm_bus, _desc);
> > + if (!nd_region) {
> > + dev_err(>dev, "failed to create nvdimm region\n");
> > + err = -ENXIO;
> > + goto out_nd;
> > + }
> > + nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
> > + return 0;
> > +out_nd:
> > + nvdimm_bus_unregister(vpmem->nvdimm_bus);
> > +out_vq:
> > + vdev->config->del_vqs(vdev);
> > +out_err:
> > + return err;
> > +}
> > +
> > +static void virtio_pmem_remove(struct virtio_device *vdev)
> > +{
> > + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(>dev);
> > +
> > + nvdimm_bus_unregister(nvdimm_bus);
> > + vdev->config->del_vqs(vdev);
> > + vdev->config->reset(vdev);
> > +}
> > +
> > +static struct virtio_driver virtio_pmem_driver = {
> > + .driver.name= KBUILD_MODNAME,
> > + .driver.owner = THIS_MODULE,
> > + .id_table = id_table,
> > + .probe = virtio_pmem_probe,
> > + .remove = virtio_pmem_remove,
> > +};
> > +
> > +module_virtio_driver(virtio_pmem_driver);
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +MODULE_DESCRIPTION("Virtio pmem driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
> > new file mode 100644
> > index ..ab1da877575d
> > --- /dev/null
> > +++ b/drivers/nvdimm/virtio_pmem.h
> > @@ -0,0 +1,60 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * virtio_pmem.h: virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and provides a virtio based flushing
> > + * interface.
> > + **/
> > +
> > +#ifndef _LINUX_VIRTIO_PMEM_H
> > +#define _LINUX_VIRTIO_PMEM_H
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +struct virtio_pmem_request {
> > + /* Host return status corresponding to flush request */
> > + int ret;
> > +
> > + /* command name*/
> > + char name[16];
>
> So ... why are we sending string commands and expect native-endianess
> integers and don't define a proper request/response structure + request
> types in include/uapi/linux/virtio_pmem.h like
passing names could be ok.
I missed the fact we return a native endian int.
Pls fix that.
>
> struct virtio_pmem_resp {
> __virtio32 ret;
> }
>
> #define VIRTIO_PMEM_REQ_TYPE_FLUSH1
> struct virtio_pmem_req {
> __virtio16 type;
> }
>
> ... and this way we also define a proper endianess format for exchange
> and keep it extensible
>
> @MST, what's your take on this?
Extensions can always use feature bits so I don't think
it's a problem.
>
> --
>
> Thanks,
>
> David / dhildenb
Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
> >> + vpmem->vdev = vdev;
> >> + vdev->priv = vpmem;
> >> + err = init_vq(vpmem);
> >> + if (err) {
> >> + dev_err(>dev, "failed to initialize virtio pmem vq's\n");
> >> + goto out_err;
> >> + }
> >> +
> >> + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> >> + start, >start);
> >> + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> >> + size, >size);
> >> +
> >> + res.start = vpmem->start;
> >> + res.end = vpmem->start + vpmem->size-1;
> >
> > nit: " - 1;"
> >
> >> + vpmem->nd_desc.provider_name = "virtio-pmem";
> >> + vpmem->nd_desc.module = THIS_MODULE;
> >> +
> >> + vpmem->nvdimm_bus = nvdimm_bus_register(>dev,
> >> + >nd_desc);
> >> + if (!vpmem->nvdimm_bus) {
> >> + dev_err(>dev, "failed to register device with
> >> nvdimm_bus\n");
> >> + err = -ENXIO;
> >> + goto out_vq;
> >> + }
> >> +
> >> + dev_set_drvdata(>dev, vpmem->nvdimm_bus);
> >> +
> >> + ndr_desc.res =
> >> + ndr_desc.numa_node = nid;
> >> + ndr_desc.flush = async_pmem_flush;
> >> + set_bit(ND_REGION_PAGEMAP, _desc.flags);
> >> + set_bit(ND_REGION_ASYNC, _desc.flags);
> >> + nd_region = nvdimm_pmem_region_create(vpmem->nvdimm_bus, _desc);
> >> + if (!nd_region) {
> >> + dev_err(>dev, "failed to create nvdimm region\n");
> >> + err = -ENXIO;
> >> + goto out_nd;
> >> + }
> >> + nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
> >> + return 0;
> >> +out_nd:
> >> + nvdimm_bus_unregister(vpmem->nvdimm_bus);
> >> +out_vq:
> >> + vdev->config->del_vqs(vdev);
> >> +out_err:
> >> + return err;
> >> +}
> >> +
> >> +static void virtio_pmem_remove(struct virtio_device *vdev)
> >> +{
> >> + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(>dev);
> >> +
> >> + nvdimm_bus_unregister(nvdimm_bus);
> >> + vdev->config->del_vqs(vdev);
> >> + vdev->config->reset(vdev);
> >> +}
> >> +
> >> +static struct virtio_driver virtio_pmem_driver = {
> >> + .driver.name= KBUILD_MODNAME,
> >> + .driver.owner = THIS_MODULE,
> >> + .id_table = id_table,
> >> + .probe = virtio_pmem_probe,
> >> + .remove = virtio_pmem_remove,
> >> +};
> >> +
> >> +module_virtio_driver(virtio_pmem_driver);
> >> +MODULE_DEVICE_TABLE(virtio, id_table);
> >> +MODULE_DESCRIPTION("Virtio pmem driver");
> >> +MODULE_LICENSE("GPL");
> >> diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
> >> new file mode 100644
> >> index ..ab1da877575d
> >> --- /dev/null
> >> +++ b/drivers/nvdimm/virtio_pmem.h
> >> @@ -0,0 +1,60 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + * virtio_pmem.h: virtio pmem Driver
> >> + *
> >> + * Discovers persistent memory range information
> >> + * from host and provides a virtio based flushing
> >> + * interface.
> >> + **/
> >> +
> >> +#ifndef _LINUX_VIRTIO_PMEM_H
> >> +#define _LINUX_VIRTIO_PMEM_H
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +struct virtio_pmem_request {
> >> + /* Host return status corresponding to flush request */
> >> + int ret;
> >> +
> >> + /* command name*/
> >> + char name[16];
> >
> > So ... why are we sending string commands and expect native-endianess
> > integers and don't define a proper request/response structure + request
> > types in include/uapi/linux/virtio_pmem.h like
> >
> > struct virtio_pmem_resp {
> > __virtio32 ret;
> > }
>
> FWIW, I wonder if we should even properly translate return values and
> define types like
>
> VIRTIO_PMEM_RESP_TYPE_OK 0
> VIRTIO_PMEM_RESP_TYPE_EIO 1
Don't think these are required as only failure and success
return types easy to understand.
Thanks,
Pankaj
>
> ..
>
> >
> > #define VIRTIO_PMEM_REQ_TYPE_FLUSH 1
> > struct virtio_pmem_req {
> > __virtio16 type;
> > }
> >
> > ... and this way we also define a proper endianess format for exchange
> > and keep it extensible
> >
> > @MST, what's your take on this?
> >
> >
>
>
> --
>
> Thanks,
>
> David / dhildenb
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
>
> > + vpmem->vdev = vdev;
> > + vdev->priv = vpmem;
> > + err = init_vq(vpmem);
> > + if (err) {
> > + dev_err(>dev, "failed to initialize virtio pmem vq's\n");
> > + goto out_err;
> > + }
> > +
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + start, >start);
> > + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> > + size, >size);
> > +
> > + res.start = vpmem->start;
> > + res.end = vpmem->start + vpmem->size-1;
>
> nit: " - 1;"
Sure.
>
> > + vpmem->nd_desc.provider_name = "virtio-pmem";
> > + vpmem->nd_desc.module = THIS_MODULE;
> > +
> > + vpmem->nvdimm_bus = nvdimm_bus_register(>dev,
> > + >nd_desc);
> > + if (!vpmem->nvdimm_bus) {
> > + dev_err(>dev, "failed to register device with
> > nvdimm_bus\n");
> > + err = -ENXIO;
> > + goto out_vq;
> > + }
> > +
> > + dev_set_drvdata(>dev, vpmem->nvdimm_bus);
> > +
> > + ndr_desc.res =
> > + ndr_desc.numa_node = nid;
> > + ndr_desc.flush = async_pmem_flush;
> > + set_bit(ND_REGION_PAGEMAP, _desc.flags);
> > + set_bit(ND_REGION_ASYNC, _desc.flags);
> > + nd_region = nvdimm_pmem_region_create(vpmem->nvdimm_bus, _desc);
> > + if (!nd_region) {
> > + dev_err(>dev, "failed to create nvdimm region\n");
> > + err = -ENXIO;
> > + goto out_nd;
> > + }
> > + nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
> > + return 0;
> > +out_nd:
> > + nvdimm_bus_unregister(vpmem->nvdimm_bus);
> > +out_vq:
> > + vdev->config->del_vqs(vdev);
> > +out_err:
> > + return err;
> > +}
> > +
> > +static void virtio_pmem_remove(struct virtio_device *vdev)
> > +{
> > + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(>dev);
> > +
> > + nvdimm_bus_unregister(nvdimm_bus);
> > + vdev->config->del_vqs(vdev);
> > + vdev->config->reset(vdev);
> > +}
> > +
> > +static struct virtio_driver virtio_pmem_driver = {
> > + .driver.name= KBUILD_MODNAME,
> > + .driver.owner = THIS_MODULE,
> > + .id_table = id_table,
> > + .probe = virtio_pmem_probe,
> > + .remove = virtio_pmem_remove,
> > +};
> > +
> > +module_virtio_driver(virtio_pmem_driver);
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +MODULE_DESCRIPTION("Virtio pmem driver");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
> > new file mode 100644
> > index ..ab1da877575d
> > --- /dev/null
> > +++ b/drivers/nvdimm/virtio_pmem.h
> > @@ -0,0 +1,60 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * virtio_pmem.h: virtio pmem Driver
> > + *
> > + * Discovers persistent memory range information
> > + * from host and provides a virtio based flushing
> > + * interface.
> > + **/
> > +
> > +#ifndef _LINUX_VIRTIO_PMEM_H
> > +#define _LINUX_VIRTIO_PMEM_H
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +struct virtio_pmem_request {
> > + /* Host return status corresponding to flush request */
> > + int ret;
> > +
> > + /* command name*/
> > + char name[16];
>
> So ... why are we sending string commands and expect native-endianess
> integers and don't define a proper request/response structure + request
> types in include/uapi/linux/virtio_pmem.h like
>
> struct virtio_pmem_resp {
> __virtio32 ret;
> }
>
> #define VIRTIO_PMEM_REQ_TYPE_FLUSH1
> struct virtio_pmem_req {
> __virtio16 type;
> }
>
> ... and this way we also define a proper endianess format for exchange
> and keep it extensible
Done the suggested change.
Thank you,
Pankaj
>
> @MST, what's your take on this?
>
>
> --
>
> Thanks,
>
> David / dhildenb
>
Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
On 15.05.19 22:46, David Hildenbrand wrote:
>> +vpmem->vdev = vdev;
>> +vdev->priv = vpmem;
>> +err = init_vq(vpmem);
>> +if (err) {
>> +dev_err(>dev, "failed to initialize virtio pmem vq's\n");
>> +goto out_err;
>> +}
>> +
>> +virtio_cread(vpmem->vdev, struct virtio_pmem_config,
>> +start, >start);
>> +virtio_cread(vpmem->vdev, struct virtio_pmem_config,
>> +size, >size);
>> +
>> +res.start = vpmem->start;
>> +res.end = vpmem->start + vpmem->size-1;
>
> nit: " - 1;"
>
>> +vpmem->nd_desc.provider_name = "virtio-pmem";
>> +vpmem->nd_desc.module = THIS_MODULE;
>> +
>> +vpmem->nvdimm_bus = nvdimm_bus_register(>dev,
>> +>nd_desc);
>> +if (!vpmem->nvdimm_bus) {
>> +dev_err(>dev, "failed to register device with
>> nvdimm_bus\n");
>> +err = -ENXIO;
>> +goto out_vq;
>> +}
>> +
>> +dev_set_drvdata(>dev, vpmem->nvdimm_bus);
>> +
>> +ndr_desc.res =
>> +ndr_desc.numa_node = nid;
>> +ndr_desc.flush = async_pmem_flush;
>> +set_bit(ND_REGION_PAGEMAP, _desc.flags);
>> +set_bit(ND_REGION_ASYNC, _desc.flags);
>> +nd_region = nvdimm_pmem_region_create(vpmem->nvdimm_bus, _desc);
>> +if (!nd_region) {
>> +dev_err(>dev, "failed to create nvdimm region\n");
>> +err = -ENXIO;
>> +goto out_nd;
>> +}
>> +nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
>> +return 0;
>> +out_nd:
>> +nvdimm_bus_unregister(vpmem->nvdimm_bus);
>> +out_vq:
>> +vdev->config->del_vqs(vdev);
>> +out_err:
>> +return err;
>> +}
>> +
>> +static void virtio_pmem_remove(struct virtio_device *vdev)
>> +{
>> +struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(>dev);
>> +
>> +nvdimm_bus_unregister(nvdimm_bus);
>> +vdev->config->del_vqs(vdev);
>> +vdev->config->reset(vdev);
>> +}
>> +
>> +static struct virtio_driver virtio_pmem_driver = {
>> +.driver.name= KBUILD_MODNAME,
>> +.driver.owner = THIS_MODULE,
>> +.id_table = id_table,
>> +.probe = virtio_pmem_probe,
>> +.remove = virtio_pmem_remove,
>> +};
>> +
>> +module_virtio_driver(virtio_pmem_driver);
>> +MODULE_DEVICE_TABLE(virtio, id_table);
>> +MODULE_DESCRIPTION("Virtio pmem driver");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
>> new file mode 100644
>> index ..ab1da877575d
>> --- /dev/null
>> +++ b/drivers/nvdimm/virtio_pmem.h
>> @@ -0,0 +1,60 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * virtio_pmem.h: virtio pmem Driver
>> + *
>> + * Discovers persistent memory range information
>> + * from host and provides a virtio based flushing
>> + * interface.
>> + **/
>> +
>> +#ifndef _LINUX_VIRTIO_PMEM_H
>> +#define _LINUX_VIRTIO_PMEM_H
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +struct virtio_pmem_request {
>> +/* Host return status corresponding to flush request */
>> +int ret;
>> +
>> +/* command name*/
>> +char name[16];
>
> So ... why are we sending string commands and expect native-endianess
> integers and don't define a proper request/response structure + request
> types in include/uapi/linux/virtio_pmem.h like
>
> struct virtio_pmem_resp {
> __virtio32 ret;
> }
FWIW, I wonder if we should even properly translate return values and
define types like
VIRTIO_PMEM_RESP_TYPE_OK0
VIRTIO_PMEM_RESP_TYPE_EIO 1
..
>
> #define VIRTIO_PMEM_REQ_TYPE_FLUSH1
> struct virtio_pmem_req {
> __virtio16 type;
> }
>
> ... and this way we also define a proper endianess format for exchange
> and keep it extensible
>
> @MST, what's your take on this?
>
>
--
Thanks,
David / dhildenb
Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
> + vpmem->vdev = vdev;
> + vdev->priv = vpmem;
> + err = init_vq(vpmem);
> + if (err) {
> + dev_err(>dev, "failed to initialize virtio pmem vq's\n");
> + goto out_err;
> + }
> +
> + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> + start, >start);
> + virtio_cread(vpmem->vdev, struct virtio_pmem_config,
> + size, >size);
> +
> + res.start = vpmem->start;
> + res.end = vpmem->start + vpmem->size-1;
nit: " - 1;"
> + vpmem->nd_desc.provider_name = "virtio-pmem";
> + vpmem->nd_desc.module = THIS_MODULE;
> +
> + vpmem->nvdimm_bus = nvdimm_bus_register(>dev,
> + >nd_desc);
> + if (!vpmem->nvdimm_bus) {
> + dev_err(>dev, "failed to register device with
> nvdimm_bus\n");
> + err = -ENXIO;
> + goto out_vq;
> + }
> +
> + dev_set_drvdata(>dev, vpmem->nvdimm_bus);
> +
> + ndr_desc.res =
> + ndr_desc.numa_node = nid;
> + ndr_desc.flush = async_pmem_flush;
> + set_bit(ND_REGION_PAGEMAP, _desc.flags);
> + set_bit(ND_REGION_ASYNC, _desc.flags);
> + nd_region = nvdimm_pmem_region_create(vpmem->nvdimm_bus, _desc);
> + if (!nd_region) {
> + dev_err(>dev, "failed to create nvdimm region\n");
> + err = -ENXIO;
> + goto out_nd;
> + }
> + nd_region->provider_data = dev_to_virtio(nd_region->dev.parent->parent);
> + return 0;
> +out_nd:
> + nvdimm_bus_unregister(vpmem->nvdimm_bus);
> +out_vq:
> + vdev->config->del_vqs(vdev);
> +out_err:
> + return err;
> +}
> +
> +static void virtio_pmem_remove(struct virtio_device *vdev)
> +{
> + struct nvdimm_bus *nvdimm_bus = dev_get_drvdata(>dev);
> +
> + nvdimm_bus_unregister(nvdimm_bus);
> + vdev->config->del_vqs(vdev);
> + vdev->config->reset(vdev);
> +}
> +
> +static struct virtio_driver virtio_pmem_driver = {
> + .driver.name= KBUILD_MODNAME,
> + .driver.owner = THIS_MODULE,
> + .id_table = id_table,
> + .probe = virtio_pmem_probe,
> + .remove = virtio_pmem_remove,
> +};
> +
> +module_virtio_driver(virtio_pmem_driver);
> +MODULE_DEVICE_TABLE(virtio, id_table);
> +MODULE_DESCRIPTION("Virtio pmem driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/nvdimm/virtio_pmem.h b/drivers/nvdimm/virtio_pmem.h
> new file mode 100644
> index ..ab1da877575d
> --- /dev/null
> +++ b/drivers/nvdimm/virtio_pmem.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * virtio_pmem.h: virtio pmem Driver
> + *
> + * Discovers persistent memory range information
> + * from host and provides a virtio based flushing
> + * interface.
> + **/
> +
> +#ifndef _LINUX_VIRTIO_PMEM_H
> +#define _LINUX_VIRTIO_PMEM_H
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct virtio_pmem_request {
> + /* Host return status corresponding to flush request */
> + int ret;
> +
> + /* command name*/
> + char name[16];
So ... why are we sending string commands and expect native-endianess
integers and don't define a proper request/response structure + request
types in include/uapi/linux/virtio_pmem.h like
struct virtio_pmem_resp {
__virtio32 ret;
}
#define VIRTIO_PMEM_REQ_TYPE_FLUSH 1
struct virtio_pmem_req {
__virtio16 type;
}
... and this way we also define a proper endianess format for exchange
and keep it extensible
@MST, what's your take on this?
--
Thanks,
David / dhildenb
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 2/7] virtio-pmem: Add virtio pmem driver
On 5/14/19 7:54 AM, Pankaj Gupta wrote: > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index 35897649c24f..94bad084ebab 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -42,6 +42,17 @@ config VIRTIO_PCI_LEGACY > > If unsure, say Y. > > +config VIRTIO_PMEM > + tristate "Support for virtio pmem driver" > + depends on VIRTIO > + depends on LIBNVDIMM > + help > + This driver provides access to virtio-pmem devices, storage devices > + that are mapped into the physical address space - similar to NVDIMMs > + - with a virtio-based flushing interface. > + > + If unsure, say M. from Documentation/process/coding-style.rst: "Lines under a ``config`` definition are indented with one tab, while help text is indented an additional two spaces." > + > config VIRTIO_BALLOON > tristate "Virtio balloon driver" > depends on VIRTIO thanks. -- ~Randy
