diff mbox series

[RFC,v4,11/11] vduse: Support binding irq to the specified cpu

Message ID 20210223115048.435-12-xieyongji@bytedance.com (mailing list archive)
State New, archived
Headers show
Series Introduce VDUSE - vDPA Device in Userspace | expand

Commit Message

Yongji Xie Feb. 23, 2021, 11:50 a.m. UTC
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(-)

Comments

Jason Wang March 4, 2021, 7:30 a.m. UTC | #1
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)
Yongji Xie March 4, 2021, 8:19 a.m. UTC | #2
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
Jason Wang March 5, 2021, 3:11 a.m. UTC | #3
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
>
Yongji Xie March 5, 2021, 3:37 a.m. UTC | #4
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
Jason Wang March 5, 2021, 3:44 a.m. UTC | #5
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
>
Yongji Xie March 5, 2021, 6:40 a.m. UTC | #6
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 mbox series

Patch

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)