| Message ID | 20210223115048.435-12-xieyongji@bytedance.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Introduce VDUSE - vDPA Device in Userspace | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 2021/2/23 7:50 下午, Xie Yongji wrote: > Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support > injecting virtqueue's interrupt to the specified cpu. How userspace know which CPU is this irq for? It looks to me we need to do it at different level. E.g introduce some API in sys to allow admin to tune for that. But I think we can do that in antoher patch on top of this series. Thanks > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com> > --- > drivers/vdpa/vdpa_user/vduse_dev.c | 22 +++++++++++++++++----- > include/uapi/linux/vduse.h | 7 ++++++- > 2 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > index f5adeb9ee027..df3d467fff40 100644 > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > @@ -923,14 +923,27 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > break; > } > case VDUSE_INJECT_VQ_IRQ: { > + struct vduse_vq_irq irq; > struct vduse_virtqueue *vq; > > + ret = -EFAULT; > + if (copy_from_user(&irq, argp, sizeof(irq))) > + break; > + > ret = -EINVAL; > - if (arg >= dev->vq_num) > + if (irq.index >= dev->vq_num) > + break; > + > + if (irq.cpu != -1 && (irq.cpu >= nr_cpu_ids || > + !cpu_online(irq.cpu))) > break; > > - vq = &dev->vqs[arg]; > - queue_work(vduse_irq_wq, &vq->inject); > + ret = 0; > + vq = &dev->vqs[irq.index]; > + if (irq.cpu == -1) > + queue_work(vduse_irq_wq, &vq->inject); > + else > + queue_work_on(irq.cpu, vduse_irq_wq, &vq->inject); > break; > } > case VDUSE_INJECT_CONFIG_IRQ: > @@ -1342,8 +1355,7 @@ static int vduse_init(void) > if (ret) > goto err_chardev; > > - vduse_irq_wq = alloc_workqueue("vduse-irq", > - WQ_HIGHPRI | WQ_SYSFS | WQ_UNBOUND, 0); > + vduse_irq_wq = alloc_workqueue("vduse-irq", WQ_HIGHPRI, 0); > if (!vduse_irq_wq) > goto err_wq; > > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h > index 9070cd512cb4..9c70fd842ce5 100644 > --- a/include/uapi/linux/vduse.h > +++ b/include/uapi/linux/vduse.h > @@ -116,6 +116,11 @@ struct vduse_vq_eventfd { > int fd; /* eventfd, -1 means de-assigning the eventfd */ > }; > > +struct vduse_vq_irq { > + __u32 index; /* virtqueue index */ > + int cpu; /* bind irq to the specified cpu, -1 means running on the current cpu */ > +}; > + > #define VDUSE_BASE 0x81 > > /* Create a vduse device which is represented by a char device (/dev/vduse/<name>) */ > @@ -131,7 +136,7 @@ struct vduse_vq_eventfd { > #define VDUSE_VQ_SETUP_KICKFD _IOW(VDUSE_BASE, 0x04, struct vduse_vq_eventfd) > > /* Inject an interrupt for specific virtqueue */ > -#define VDUSE_INJECT_VQ_IRQ _IO(VDUSE_BASE, 0x05) > +#define VDUSE_INJECT_VQ_IRQ _IOW(VDUSE_BASE, 0x05, struct vduse_vq_irq) > > /* Inject a config interrupt */ > #define VDUSE_INJECT_CONFIG_IRQ _IO(VDUSE_BASE, 0x06)
On Thu, Mar 4, 2021 at 3:30 PM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/2/23 7:50 下午, Xie Yongji wrote: > > Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support > > injecting virtqueue's interrupt to the specified cpu. > > > How userspace know which CPU is this irq for? It looks to me we need to > do it at different level. > > E.g introduce some API in sys to allow admin to tune for that. > > But I think we can do that in antoher patch on top of this series. > OK. I will think more about it. Thanks, Yongji
On 2021/3/4 4:19 下午, Yongji Xie wrote: > On Thu, Mar 4, 2021 at 3:30 PM Jason Wang <jasowang@redhat.com> wrote: >> >> On 2021/2/23 7:50 下午, Xie Yongji wrote: >>> Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support >>> injecting virtqueue's interrupt to the specified cpu. >> >> How userspace know which CPU is this irq for? It looks to me we need to >> do it at different level. >> >> E.g introduce some API in sys to allow admin to tune for that. >> >> But I think we can do that in antoher patch on top of this series. >> > OK. I will think more about it. It should be soemthing like /sys/class/vduse/$dev_name/vq/0/irq_affinity. Also need to make sure eventfd could not be reused. Thanks > > Thanks, > Yongji >
On Fri, Mar 5, 2021 at 11:11 AM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/4 4:19 下午, Yongji Xie wrote: > > On Thu, Mar 4, 2021 at 3:30 PM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On 2021/2/23 7:50 下午, Xie Yongji wrote: > >>> Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support > >>> injecting virtqueue's interrupt to the specified cpu. > >> > >> How userspace know which CPU is this irq for? It looks to me we need to > >> do it at different level. > >> > >> E.g introduce some API in sys to allow admin to tune for that. > >> > >> But I think we can do that in antoher patch on top of this series. > >> > > OK. I will think more about it. > > > It should be soemthing like > /sys/class/vduse/$dev_name/vq/0/irq_affinity. Also need to make sure > eventfd could not be reused. > Looks like we doesn't use eventfd now. Do you mean we need to use eventfd in this case? Thanks, Yongji
On 2021/3/5 11:37 上午, Yongji Xie wrote: > On Fri, Mar 5, 2021 at 11:11 AM Jason Wang <jasowang@redhat.com> wrote: >> >> On 2021/3/4 4:19 下午, Yongji Xie wrote: >>> On Thu, Mar 4, 2021 at 3:30 PM Jason Wang <jasowang@redhat.com> wrote: >>>> On 2021/2/23 7:50 下午, Xie Yongji wrote: >>>>> Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support >>>>> injecting virtqueue's interrupt to the specified cpu. >>>> How userspace know which CPU is this irq for? It looks to me we need to >>>> do it at different level. >>>> >>>> E.g introduce some API in sys to allow admin to tune for that. >>>> >>>> But I think we can do that in antoher patch on top of this series. >>>> >>> OK. I will think more about it. >> >> It should be soemthing like >> /sys/class/vduse/$dev_name/vq/0/irq_affinity. Also need to make sure >> eventfd could not be reused. >> > Looks like we doesn't use eventfd now. Do you mean we need to use > eventfd in this case? No, I meant if we're using eventfd, do we allow a single eventfd to be used for injecting irq for more than one virtqueue? (If not, I guess it should be ok). Thanks > > Thanks, > Yongji >
On Fri, Mar 5, 2021 at 11:44 AM Jason Wang <jasowang@redhat.com> wrote: > > > On 2021/3/5 11:37 上午, Yongji Xie wrote: > > On Fri, Mar 5, 2021 at 11:11 AM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On 2021/3/4 4:19 下午, Yongji Xie wrote: > >>> On Thu, Mar 4, 2021 at 3:30 PM Jason Wang <jasowang@redhat.com> wrote: > >>>> On 2021/2/23 7:50 下午, Xie Yongji wrote: > >>>>> Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support > >>>>> injecting virtqueue's interrupt to the specified cpu. > >>>> How userspace know which CPU is this irq for? It looks to me we need to > >>>> do it at different level. > >>>> > >>>> E.g introduce some API in sys to allow admin to tune for that. > >>>> > >>>> But I think we can do that in antoher patch on top of this series. > >>>> > >>> OK. I will think more about it. > >> > >> It should be soemthing like > >> /sys/class/vduse/$dev_name/vq/0/irq_affinity. Also need to make sure > >> eventfd could not be reused. > >> > > Looks like we doesn't use eventfd now. Do you mean we need to use > > eventfd in this case? > > > No, I meant if we're using eventfd, do we allow a single eventfd to be > used for injecting irq for more than one virtqueue? (If not, I guess it > should be ok). > OK, I see. I think we don't allow that now. Thanks, Yongji
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c index f5adeb9ee027..df3d467fff40 100644 --- a/drivers/vdpa/vdpa_user/vduse_dev.c +++ b/drivers/vdpa/vdpa_user/vduse_dev.c @@ -923,14 +923,27 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, break; } case VDUSE_INJECT_VQ_IRQ: { + struct vduse_vq_irq irq; struct vduse_virtqueue *vq; + ret = -EFAULT; + if (copy_from_user(&irq, argp, sizeof(irq))) + break; + ret = -EINVAL; - if (arg >= dev->vq_num) + if (irq.index >= dev->vq_num) + break; + + if (irq.cpu != -1 && (irq.cpu >= nr_cpu_ids || + !cpu_online(irq.cpu))) break; - vq = &dev->vqs[arg]; - queue_work(vduse_irq_wq, &vq->inject); + ret = 0; + vq = &dev->vqs[irq.index]; + if (irq.cpu == -1) + queue_work(vduse_irq_wq, &vq->inject); + else + queue_work_on(irq.cpu, vduse_irq_wq, &vq->inject); break; } case VDUSE_INJECT_CONFIG_IRQ: @@ -1342,8 +1355,7 @@ static int vduse_init(void) if (ret) goto err_chardev; - vduse_irq_wq = alloc_workqueue("vduse-irq", - WQ_HIGHPRI | WQ_SYSFS | WQ_UNBOUND, 0); + vduse_irq_wq = alloc_workqueue("vduse-irq", WQ_HIGHPRI, 0); if (!vduse_irq_wq) goto err_wq; diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h index 9070cd512cb4..9c70fd842ce5 100644 --- a/include/uapi/linux/vduse.h +++ b/include/uapi/linux/vduse.h @@ -116,6 +116,11 @@ struct vduse_vq_eventfd { int fd; /* eventfd, -1 means de-assigning the eventfd */ }; +struct vduse_vq_irq { + __u32 index; /* virtqueue index */ + int cpu; /* bind irq to the specified cpu, -1 means running on the current cpu */ +}; + #define VDUSE_BASE 0x81 /* Create a vduse device which is represented by a char device (/dev/vduse/<name>) */ @@ -131,7 +136,7 @@ struct vduse_vq_eventfd { #define VDUSE_VQ_SETUP_KICKFD _IOW(VDUSE_BASE, 0x04, struct vduse_vq_eventfd) /* Inject an interrupt for specific virtqueue */ -#define VDUSE_INJECT_VQ_IRQ _IO(VDUSE_BASE, 0x05) +#define VDUSE_INJECT_VQ_IRQ _IOW(VDUSE_BASE, 0x05, struct vduse_vq_irq) /* Inject a config interrupt */ #define VDUSE_INJECT_CONFIG_IRQ _IO(VDUSE_BASE, 0x06)
Add a parameter for the ioctl VDUSE_INJECT_VQ_IRQ to support injecting virtqueue's interrupt to the specified cpu. Signed-off-by: Xie Yongji <xieyongji@bytedance.com> --- drivers/vdpa/vdpa_user/vduse_dev.c | 22 +++++++++++++++++----- include/uapi/linux/vduse.h | 7 ++++++- 2 files changed, 23 insertions(+), 6 deletions(-)