| Message ID | 20230912030008.3599514-2-lulu@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Support reconnection in vduse | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote: > > Add the function vduse_alloc_reconnnect_info_mem > and vduse_alloc_reconnnect_info_mem > In this 2 function, vduse will get/free (vq_num + 1)*page > Page 0 will be used to save the reconnection information, The > Userspace App will maintain this. Page 1 ~ vq_num + 1 will save > the reconnection information for vqs. Please explain why this is needed instead of only describing how it is implemented. (Code can explain itself). > > Signed-off-by: Cindy Lu <lulu@redhat.com> > --- > drivers/vdpa/vdpa_user/vduse_dev.c | 86 ++++++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > index 26b7e29cb900..4c256fa31fc4 100644 > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > @@ -30,6 +30,10 @@ > #include <uapi/linux/virtio_blk.h> > #include <linux/mod_devicetable.h> > > +#ifdef CONFIG_X86 > +#include <asm/set_memory.h> > +#endif > + > #include "iova_domain.h" > > #define DRV_AUTHOR "Yongji Xie <xieyongji@bytedance.com>" > @@ -41,6 +45,23 @@ > #define VDUSE_IOVA_SIZE (128 * 1024 * 1024) > #define VDUSE_MSG_DEFAULT_TIMEOUT 30 > > +/* struct vdpa_reconnect_info save the page information for reconnection > + * kernel will init these information while alloc the pages > + * and use these information to free the pages > + */ > +struct vdpa_reconnect_info { > + /* Offset (within vm_file) in PAGE_SIZE, > + * this just for check, not using > + */ > + u32 index; > + /* physical address for this page*/ > + phys_addr_t addr; > + /* virtual address for this page*/ > + unsigned long vaddr; If it could be switched by virt_to_phys() why duplicate those fields? > + /* memory size, here always page_size*/ > + phys_addr_t size; If it's always PAGE_SIZE why would we have this? > +}; > + > struct vduse_virtqueue { > u16 index; > u16 num_max; > @@ -57,6 +78,7 @@ struct vduse_virtqueue { > struct vdpa_callback cb; > struct work_struct inject; > struct work_struct kick; > + struct vdpa_reconnect_info reconnect_info; > }; > > struct vduse_dev; > @@ -106,6 +128,7 @@ struct vduse_dev { > u32 vq_align; > struct vduse_umem *umem; > struct mutex mem_lock; > + struct vdpa_reconnect_info reconnect_status; > }; > > struct vduse_dev_msg { > @@ -1030,6 +1053,65 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev, > return ret; > } > > +int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev) > +{ > + struct vdpa_reconnect_info *info; > + struct vduse_virtqueue *vq; > + void *addr; > + > + /*page 0 is use to save status,dpdk will use this to save the information > + *needed in reconnection,kernel don't need to maintain this > + */ > + info = &dev->reconnect_status; > + addr = (void *)get_zeroed_page(GFP_KERNEL); > + if (!addr) > + return -1; -ENOMEM? Thanks
On Mon, Sep 18, 2023 at 4:41 PM Jason Wang <jasowang@redhat.com> wrote: > > On Tue, Sep 12, 2023 at 11:00 AM Cindy Lu <lulu@redhat.com> wrote: > > > > Add the function vduse_alloc_reconnnect_info_mem > > and vduse_alloc_reconnnect_info_mem > > In this 2 function, vduse will get/free (vq_num + 1)*page > > Page 0 will be used to save the reconnection information, The > > Userspace App will maintain this. Page 1 ~ vq_num + 1 will save > > the reconnection information for vqs. > > Please explain why this is needed instead of only describing how it is > implemented. (Code can explain itself). > > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > --- > > drivers/vdpa/vdpa_user/vduse_dev.c | 86 ++++++++++++++++++++++++++++++ > > 1 file changed, 86 insertions(+) > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > > index 26b7e29cb900..4c256fa31fc4 100644 > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > @@ -30,6 +30,10 @@ > > #include <uapi/linux/virtio_blk.h> > > #include <linux/mod_devicetable.h> > > > > +#ifdef CONFIG_X86 > > +#include <asm/set_memory.h> > > +#endif > > + > > #include "iova_domain.h" > > > > #define DRV_AUTHOR "Yongji Xie <xieyongji@bytedance.com>" > > @@ -41,6 +45,23 @@ > > #define VDUSE_IOVA_SIZE (128 * 1024 * 1024) > > #define VDUSE_MSG_DEFAULT_TIMEOUT 30 > > > > +/* struct vdpa_reconnect_info save the page information for reconnection > > + * kernel will init these information while alloc the pages > > + * and use these information to free the pages > > + */ > > +struct vdpa_reconnect_info { > > + /* Offset (within vm_file) in PAGE_SIZE, > > + * this just for check, not using > > + */ > > + u32 index; > > + /* physical address for this page*/ > > + phys_addr_t addr; > > + /* virtual address for this page*/ > > + unsigned long vaddr; > > If it could be switched by virt_to_phys() why duplicate those fields? > yes will remove this part Thanks Cindy > > + /* memory size, here always page_size*/ > > + phys_addr_t size; > > If it's always PAGE_SIZE why would we have this? will remove this Thanks Cindy > > > +}; > > + > > struct vduse_virtqueue { > > u16 index; > > u16 num_max; > > @@ -57,6 +78,7 @@ struct vduse_virtqueue { > > struct vdpa_callback cb; > > struct work_struct inject; > > struct work_struct kick; > > + struct vdpa_reconnect_info reconnect_info; > > }; > > > > struct vduse_dev; > > @@ -106,6 +128,7 @@ struct vduse_dev { > > u32 vq_align; > > struct vduse_umem *umem; > > struct mutex mem_lock; > > + struct vdpa_reconnect_info reconnect_status; > > }; > > > > struct vduse_dev_msg { > > @@ -1030,6 +1053,65 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev, > > return ret; > > } > > > > +int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev) > > +{ > > + struct vdpa_reconnect_info *info; > > + struct vduse_virtqueue *vq; > > + void *addr; > > + > > + /*page 0 is use to save status,dpdk will use this to save the information > > + *needed in reconnection,kernel don't need to maintain this > > + */ > > + info = &dev->reconnect_status; > > + addr = (void *)get_zeroed_page(GFP_KERNEL); > > + if (!addr) > > + return -1; > > -ENOMEM? > sure will change this Thanks Cidny > Thanks >
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c index 26b7e29cb900..4c256fa31fc4 100644 --- a/drivers/vdpa/vdpa_user/vduse_dev.c +++ b/drivers/vdpa/vdpa_user/vduse_dev.c @@ -30,6 +30,10 @@ #include <uapi/linux/virtio_blk.h> #include <linux/mod_devicetable.h> +#ifdef CONFIG_X86 +#include <asm/set_memory.h> +#endif + #include "iova_domain.h" #define DRV_AUTHOR "Yongji Xie <xieyongji@bytedance.com>" @@ -41,6 +45,23 @@ #define VDUSE_IOVA_SIZE (128 * 1024 * 1024) #define VDUSE_MSG_DEFAULT_TIMEOUT 30 +/* struct vdpa_reconnect_info save the page information for reconnection + * kernel will init these information while alloc the pages + * and use these information to free the pages + */ +struct vdpa_reconnect_info { + /* Offset (within vm_file) in PAGE_SIZE, + * this just for check, not using + */ + u32 index; + /* physical address for this page*/ + phys_addr_t addr; + /* virtual address for this page*/ + unsigned long vaddr; + /* memory size, here always page_size*/ + phys_addr_t size; +}; + struct vduse_virtqueue { u16 index; u16 num_max; @@ -57,6 +78,7 @@ struct vduse_virtqueue { struct vdpa_callback cb; struct work_struct inject; struct work_struct kick; + struct vdpa_reconnect_info reconnect_info; }; struct vduse_dev; @@ -106,6 +128,7 @@ struct vduse_dev { u32 vq_align; struct vduse_umem *umem; struct mutex mem_lock; + struct vdpa_reconnect_info reconnect_status; }; struct vduse_dev_msg { @@ -1030,6 +1053,65 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev, return ret; } +int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev) +{ + struct vdpa_reconnect_info *info; + struct vduse_virtqueue *vq; + void *addr; + + /*page 0 is use to save status,dpdk will use this to save the information + *needed in reconnection,kernel don't need to maintain this + */ + info = &dev->reconnect_status; + addr = (void *)get_zeroed_page(GFP_KERNEL); + if (!addr) + return -1; + + info->addr = virt_to_phys(addr); + info->vaddr = (unsigned long)(addr); + info->size = PAGE_SIZE; + /* index is vm Offset in PAGE_SIZE */ + info->index = 0; + + /*page 1~ vq_num + 1 save the reconnect info for vqs*/ + for (int i = 0; i < dev->vq_num + 1; i++) { + vq = &dev->vqs[i]; + info = &vq->reconnect_info; + addr = (void *)get_zeroed_page(GFP_KERNEL); + if (!addr) + return -1; + + info->addr = virt_to_phys(addr); + info->vaddr = (unsigned long)(addr); + info->size = PAGE_SIZE; + info->index = i + 1; + } + + return 0; +} + +int vduse_free_reconnnect_info_mem(struct vduse_dev *dev) +{ + struct vdpa_reconnect_info *info; + struct vduse_virtqueue *vq; + + info = &dev->reconnect_status; + free_page(info->vaddr); + info->size = 0; + info->addr = 0; + info->vaddr = 0; + for (int i = 0; i < dev->vq_num + 1; i++) { + vq = &dev->vqs[i]; + info = &vq->reconnect_info; + free_page(info->vaddr); + info->size = 0; + info->addr = 0; + info->vaddr = 0; + } + + return 0; +} + static long vduse_dev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { @@ -1390,6 +1472,8 @@ static int vduse_destroy_dev(char *name) mutex_unlock(&dev->lock); return -EBUSY; } + vduse_free_reconnnect_info_mem(dev); + dev->connected = true; mutex_unlock(&dev->lock); @@ -1542,6 +1626,8 @@ static int vduse_create_dev(struct vduse_dev_config *config, ret = PTR_ERR(dev->dev); goto err_dev; } + + vduse_alloc_reconnnect_info_mem(dev); __module_get(THIS_MODULE); return 0;
Add the function vduse_alloc_reconnnect_info_mem and vduse_alloc_reconnnect_info_mem In this 2 function, vduse will get/free (vq_num + 1)*page Page 0 will be used to save the reconnection information, The Userspace App will maintain this. Page 1 ~ vq_num + 1 will save the reconnection information for vqs. Signed-off-by: Cindy Lu <lulu@redhat.com> --- drivers/vdpa/vdpa_user/vduse_dev.c | 86 ++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+)