diff mbox series

[RFC,v3,11/11] vduse: Introduce a workqueue for irq injection

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

Commit Message

Xie Yongji Jan. 19, 2021, 5:07 a.m. UTC
This patch introduces a dedicated workqueue for irq injection
so that we are able to do some performance tuning for it.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 drivers/vdpa/vdpa_user/eventfd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Jason Wang Jan. 26, 2021, 8:17 a.m. UTC | #1
On 2021/1/19 下午1:07, Xie Yongji wrote:
> This patch introduces a dedicated workqueue for irq injection
> so that we are able to do some performance tuning for it.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>


If we want the split like this.

It might be better to:

1) implement a simple irq injection on the ioctl context in patch 8
2) add the dedicated workqueue injection in this patch

Since my understanding is that

1) the function looks more isolated for readers
2) the difference between sysctl vs workqueue should be more obvious 
than system wq vs dedicated wq
3) a chance to describe why workqueue is needed in the commit log in 
this patch

Thanks


> ---
>   drivers/vdpa/vdpa_user/eventfd.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_user/eventfd.c b/drivers/vdpa/vdpa_user/eventfd.c
> index dbffddb08908..caf7d8d68ac0 100644
> --- a/drivers/vdpa/vdpa_user/eventfd.c
> +++ b/drivers/vdpa/vdpa_user/eventfd.c
> @@ -18,6 +18,7 @@
>   #include "eventfd.h"
>   
>   static struct workqueue_struct *vduse_irqfd_cleanup_wq;
> +static struct workqueue_struct *vduse_irq_wq;
>   
>   static void vduse_virqfd_shutdown(struct work_struct *work)
>   {
> @@ -57,7 +58,7 @@ static int vduse_virqfd_wakeup(wait_queue_entry_t *wait, unsigned int mode,
>   	__poll_t flags = key_to_poll(key);
>   
>   	if (flags & EPOLLIN)
> -		schedule_work(&virqfd->inject);
> +		queue_work(vduse_irq_wq, &virqfd->inject);
>   
>   	if (flags & EPOLLHUP) {
>   		spin_lock(&vq->irq_lock);
> @@ -165,11 +166,18 @@ int vduse_virqfd_init(void)
>   	if (!vduse_irqfd_cleanup_wq)
>   		return -ENOMEM;
>   
> +	vduse_irq_wq = alloc_workqueue("vduse-irq", WQ_SYSFS | WQ_UNBOUND, 0);
> +	if (!vduse_irq_wq) {
> +		destroy_workqueue(vduse_irqfd_cleanup_wq);
> +		return -ENOMEM;
> +	}
> +
>   	return 0;
>   }
>   
>   void vduse_virqfd_exit(void)
>   {
> +	destroy_workqueue(vduse_irq_wq);
>   	destroy_workqueue(vduse_irqfd_cleanup_wq);
>   }
>
Xie Yongji Jan. 27, 2021, 9 a.m. UTC | #2
On Tue, Jan 26, 2021 at 4:17 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2021/1/19 下午1:07, Xie Yongji wrote:
> > This patch introduces a dedicated workqueue for irq injection
> > so that we are able to do some performance tuning for it.
> >
> > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
>
>
> If we want the split like this.
>
> It might be better to:
>
> 1) implement a simple irq injection on the ioctl context in patch 8
> 2) add the dedicated workqueue injection in this patch
>
> Since my understanding is that
>
> 1) the function looks more isolated for readers
> 2) the difference between sysctl vs workqueue should be more obvious
> than system wq vs dedicated wq
> 3) a chance to describe why workqueue is needed in the commit log in
> this patch
>

OK, I will try to do it in v4.

Thanks,
Yongji
diff mbox series

Patch

diff --git a/drivers/vdpa/vdpa_user/eventfd.c b/drivers/vdpa/vdpa_user/eventfd.c
index dbffddb08908..caf7d8d68ac0 100644
--- a/drivers/vdpa/vdpa_user/eventfd.c
+++ b/drivers/vdpa/vdpa_user/eventfd.c
@@ -18,6 +18,7 @@ 
 #include "eventfd.h"
 
 static struct workqueue_struct *vduse_irqfd_cleanup_wq;
+static struct workqueue_struct *vduse_irq_wq;
 
 static void vduse_virqfd_shutdown(struct work_struct *work)
 {
@@ -57,7 +58,7 @@  static int vduse_virqfd_wakeup(wait_queue_entry_t *wait, unsigned int mode,
 	__poll_t flags = key_to_poll(key);
 
 	if (flags & EPOLLIN)
-		schedule_work(&virqfd->inject);
+		queue_work(vduse_irq_wq, &virqfd->inject);
 
 	if (flags & EPOLLHUP) {
 		spin_lock(&vq->irq_lock);
@@ -165,11 +166,18 @@  int vduse_virqfd_init(void)
 	if (!vduse_irqfd_cleanup_wq)
 		return -ENOMEM;
 
+	vduse_irq_wq = alloc_workqueue("vduse-irq", WQ_SYSFS | WQ_UNBOUND, 0);
+	if (!vduse_irq_wq) {
+		destroy_workqueue(vduse_irqfd_cleanup_wq);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
 void vduse_virqfd_exit(void)
 {
+	destroy_workqueue(vduse_irq_wq);
 	destroy_workqueue(vduse_irqfd_cleanup_wq);
 }