diff mbox series

[v3,03/11] vdpa: Add set_irq_affinity callback in vdpa_config_ops

Message ID 20230228094110.37-4-xieyongji@bytedance.com
State New
Headers
Series VDUSE: Improve performance |

Commit Message

Yongji Xie Feb. 28, 2023, 9:41 a.m. UTC 
  This introduces set_irq_affinity callback in
vdpa_config_ops so that vdpa device driver can
get the interrupt affinity hint from the virtio
device driver. The interrupt affinity hint would
be needed by the interrupt affinity spreading
mechanism.

Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
---
 drivers/virtio/virtio_vdpa.c | 4 ++++
 include/linux/vdpa.h         | 9 +++++++++
 2 files changed, 13 insertions(+)

Comments

Jason Wang March 16, 2023, 4:02 a.m. UTC | #1 
On Tue, Feb 28, 2023 at 5:42 PM Xie Yongji <xieyongji@bytedance.com> wrote:
>
> This introduces set_irq_affinity callback in
> vdpa_config_ops so that vdpa device driver can
> get the interrupt affinity hint from the virtio
> device driver. The interrupt affinity hint would
> be needed by the interrupt affinity spreading
> mechanism.
>
> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> ---
>  drivers/virtio/virtio_vdpa.c | 4 ++++
>  include/linux/vdpa.h         | 9 +++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index f72696b4c1c2..9eee8afabda8 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -282,9 +282,13 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
>         struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev);
>         struct vdpa_device *vdpa = vd_get_vdpa(vdev);
>         const struct vdpa_config_ops *ops = vdpa->config;
> +       struct irq_affinity default_affd = { 0 };
>         struct vdpa_callback cb;
>         int i, err, queue_idx = 0;
>
> +       if (ops->set_irq_affinity)
> +               ops->set_irq_affinity(vdpa, desc ? desc : &default_affd);
> +
>         for (i = 0; i < nvqs; ++i) {
>                 if (!names[i]) {
>                         vqs[i] = NULL;
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index d61f369f9cd6..10bd22387276 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -259,6 +259,13 @@ struct vdpa_map_file {
>   *                             @vdev: vdpa device
>   *                             @idx: virtqueue index
>   *                             Returns the irq affinity mask
> + * @set_irq_affinity:          Pass the irq affinity hint (best effort)

Note that this could easily confuse the users. I wonder if we can
unify it with set_irq_affinity. Looking at vduse's implementation, it
should be possible.

(E.g set_vq_affinity implemented by virtio-pci are using irq affinity actually).

Thanks

> + *                             from the virtio device driver to vdpa
> + *                             driver (optional).
> + *                             Needed by the interrupt affinity spreading
> + *                             mechanism.
> + *                             @vdev: vdpa device
> + *                             @desc: irq affinity hint
>   * @set_group_asid:            Set address space identifier for a
>   *                             virtqueue group (optional)
>   *                             @vdev: vdpa device
> @@ -353,6 +360,8 @@ struct vdpa_config_ops {
>                                const struct cpumask *cpu_mask);
>         const struct cpumask *(*get_vq_affinity)(struct vdpa_device *vdev,
>                                                  u16 idx);
> +       void (*set_irq_affinity)(struct vdpa_device *vdev,
> +                                struct irq_affinity *desc);
>
>         /* DMA ops */
>         int (*set_map)(struct vdpa_device *vdev, unsigned int asid,
> --
> 2.20.1
>
Yongji Xie March 17, 2023, 7:44 a.m. UTC | #2 
On Thu, Mar 16, 2023 at 12:03 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Feb 28, 2023 at 5:42 PM Xie Yongji <xieyongji@bytedance.com> wrote:
> >
> > This introduces set_irq_affinity callback in
> > vdpa_config_ops so that vdpa device driver can
> > get the interrupt affinity hint from the virtio
> > device driver. The interrupt affinity hint would
> > be needed by the interrupt affinity spreading
> > mechanism.
> >
> > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > ---
> >  drivers/virtio/virtio_vdpa.c | 4 ++++
> >  include/linux/vdpa.h         | 9 +++++++++
> >  2 files changed, 13 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > index f72696b4c1c2..9eee8afabda8 100644
> > --- a/drivers/virtio/virtio_vdpa.c
> > +++ b/drivers/virtio/virtio_vdpa.c
> > @@ -282,9 +282,13 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> >         struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev);
> >         struct vdpa_device *vdpa = vd_get_vdpa(vdev);
> >         const struct vdpa_config_ops *ops = vdpa->config;
> > +       struct irq_affinity default_affd = { 0 };
> >         struct vdpa_callback cb;
> >         int i, err, queue_idx = 0;
> >
> > +       if (ops->set_irq_affinity)
> > +               ops->set_irq_affinity(vdpa, desc ? desc : &default_affd);
> > +
> >         for (i = 0; i < nvqs; ++i) {
> >                 if (!names[i]) {
> >                         vqs[i] = NULL;
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > index d61f369f9cd6..10bd22387276 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -259,6 +259,13 @@ struct vdpa_map_file {
> >   *                             @vdev: vdpa device
> >   *                             @idx: virtqueue index
> >   *                             Returns the irq affinity mask
> > + * @set_irq_affinity:          Pass the irq affinity hint (best effort)
>
> Note that this could easily confuse the users. I wonder if we can
> unify it with set_irq_affinity. Looking at vduse's implementation, it
> should be possible.
>

Do you mean unify set_irq_affinity() with set_vq_affinity()? Actually
I didn't get how to achieve that. The set_vq_affinity() callback is
called by virtio_config_ops.set_vq_affinity() but the set_irq_affinity
is called by virtio_config_ops.find_vqs(), I don't know where to call
the unified callback.

Thanks,
Yongji
Jason Wang March 20, 2023, 9:31 a.m. UTC | #3 
On Fri, Mar 17, 2023 at 3:45 PM Yongji Xie <xieyongji@bytedance.com> wrote:
>
> On Thu, Mar 16, 2023 at 12:03 PM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Tue, Feb 28, 2023 at 5:42 PM Xie Yongji <xieyongji@bytedance.com> wrote:
> > >
> > > This introduces set_irq_affinity callback in
> > > vdpa_config_ops so that vdpa device driver can
> > > get the interrupt affinity hint from the virtio
> > > device driver. The interrupt affinity hint would
> > > be needed by the interrupt affinity spreading
> > > mechanism.
> > >
> > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > > ---
> > >  drivers/virtio/virtio_vdpa.c | 4 ++++
> > >  include/linux/vdpa.h         | 9 +++++++++
> > >  2 files changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > index f72696b4c1c2..9eee8afabda8 100644
> > > --- a/drivers/virtio/virtio_vdpa.c
> > > +++ b/drivers/virtio/virtio_vdpa.c
> > > @@ -282,9 +282,13 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> > >         struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev);
> > >         struct vdpa_device *vdpa = vd_get_vdpa(vdev);
> > >         const struct vdpa_config_ops *ops = vdpa->config;
> > > +       struct irq_affinity default_affd = { 0 };
> > >         struct vdpa_callback cb;
> > >         int i, err, queue_idx = 0;
> > >
> > > +       if (ops->set_irq_affinity)
> > > +               ops->set_irq_affinity(vdpa, desc ? desc : &default_affd);
> > > +
> > >         for (i = 0; i < nvqs; ++i) {
> > >                 if (!names[i]) {
> > >                         vqs[i] = NULL;
> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > index d61f369f9cd6..10bd22387276 100644
> > > --- a/include/linux/vdpa.h
> > > +++ b/include/linux/vdpa.h
> > > @@ -259,6 +259,13 @@ struct vdpa_map_file {
> > >   *                             @vdev: vdpa device
> > >   *                             @idx: virtqueue index
> > >   *                             Returns the irq affinity mask
> > > + * @set_irq_affinity:          Pass the irq affinity hint (best effort)
> >
> > Note that this could easily confuse the users. I wonder if we can
> > unify it with set_irq_affinity. Looking at vduse's implementation, it
> > should be possible.
> >
>
> Do you mean unify set_irq_affinity() with set_vq_affinity()? Actually
> I didn't get how to achieve that. The set_vq_affinity() callback is
> called by virtio_config_ops.set_vq_affinity() but the set_irq_affinity
> is called by virtio_config_ops.find_vqs(), I don't know where to call
> the unified callback.

I meant, can we stick a single per vq affinity config ops then use
that in virtio-vpda's find_vqs() by something like:

masks = create_affinity_masks(dev->vq_num, desc);
for (i = 0; i < dev->vq_num; i++)
        config->set_vq_affinity()
...

?

Thanks

>
> Thanks,
> Yongji
>
Yongji Xie March 20, 2023, 11:17 a.m. UTC | #4 
On Mon, Mar 20, 2023 at 5:31 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Fri, Mar 17, 2023 at 3:45 PM Yongji Xie <xieyongji@bytedance.com> wrote:
> >
> > On Thu, Mar 16, 2023 at 12:03 PM Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > On Tue, Feb 28, 2023 at 5:42 PM Xie Yongji <xieyongji@bytedance.com> wrote:
> > > >
> > > > This introduces set_irq_affinity callback in
> > > > vdpa_config_ops so that vdpa device driver can
> > > > get the interrupt affinity hint from the virtio
> > > > device driver. The interrupt affinity hint would
> > > > be needed by the interrupt affinity spreading
> > > > mechanism.
> > > >
> > > > Signed-off-by: Xie Yongji <xieyongji@bytedance.com>
> > > > ---
> > > >  drivers/virtio/virtio_vdpa.c | 4 ++++
> > > >  include/linux/vdpa.h         | 9 +++++++++
> > > >  2 files changed, 13 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > > index f72696b4c1c2..9eee8afabda8 100644
> > > > --- a/drivers/virtio/virtio_vdpa.c
> > > > +++ b/drivers/virtio/virtio_vdpa.c
> > > > @@ -282,9 +282,13 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> > > >         struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev);
> > > >         struct vdpa_device *vdpa = vd_get_vdpa(vdev);
> > > >         const struct vdpa_config_ops *ops = vdpa->config;
> > > > +       struct irq_affinity default_affd = { 0 };
> > > >         struct vdpa_callback cb;
> > > >         int i, err, queue_idx = 0;
> > > >
> > > > +       if (ops->set_irq_affinity)
> > > > +               ops->set_irq_affinity(vdpa, desc ? desc : &default_affd);
> > > > +
> > > >         for (i = 0; i < nvqs; ++i) {
> > > >                 if (!names[i]) {
> > > >                         vqs[i] = NULL;
> > > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > > index d61f369f9cd6..10bd22387276 100644
> > > > --- a/include/linux/vdpa.h
> > > > +++ b/include/linux/vdpa.h
> > > > @@ -259,6 +259,13 @@ struct vdpa_map_file {
> > > >   *                             @vdev: vdpa device
> > > >   *                             @idx: virtqueue index
> > > >   *                             Returns the irq affinity mask
> > > > + * @set_irq_affinity:          Pass the irq affinity hint (best effort)
> > >
> > > Note that this could easily confuse the users. I wonder if we can
> > > unify it with set_irq_affinity. Looking at vduse's implementation, it
> > > should be possible.
> > >
> >
> > Do you mean unify set_irq_affinity() with set_vq_affinity()? Actually
> > I didn't get how to achieve that. The set_vq_affinity() callback is
> > called by virtio_config_ops.set_vq_affinity() but the set_irq_affinity
> > is called by virtio_config_ops.find_vqs(), I don't know where to call
> > the unified callback.
>
> I meant, can we stick a single per vq affinity config ops then use
> that in virtio-vpda's find_vqs() by something like:
>
> masks = create_affinity_masks(dev->vq_num, desc);
> for (i = 0; i < dev->vq_num; i++)
>         config->set_vq_affinity()

OK, I see. Will do it in v4.

Thanks,
Yongji
diff mbox series

Patch

diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index f72696b4c1c2..9eee8afabda8 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -282,9 +282,13 @@  static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
 	struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev);
 	struct vdpa_device *vdpa = vd_get_vdpa(vdev);
 	const struct vdpa_config_ops *ops = vdpa->config;
+	struct irq_affinity default_affd = { 0 };
 	struct vdpa_callback cb;
 	int i, err, queue_idx = 0;
 
+	if (ops->set_irq_affinity)
+		ops->set_irq_affinity(vdpa, desc ? desc : &default_affd);
+
 	for (i = 0; i < nvqs; ++i) {
 		if (!names[i]) {
 			vqs[i] = NULL;
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index d61f369f9cd6..10bd22387276 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -259,6 +259,13 @@  struct vdpa_map_file {
  *				@vdev: vdpa device
  *				@idx: virtqueue index
  *				Returns the irq affinity mask
+ * @set_irq_affinity:		Pass the irq affinity hint (best effort)
+ *				from the virtio device driver to vdpa
+ *				driver (optional).
+ *				Needed by the interrupt affinity spreading
+ *				mechanism.
+ *				@vdev: vdpa device
+ *				@desc: irq affinity hint
  * @set_group_asid:		Set address space identifier for a
  *				virtqueue group (optional)
  *				@vdev: vdpa device
@@ -353,6 +360,8 @@  struct vdpa_config_ops {
 			       const struct cpumask *cpu_mask);
 	const struct cpumask *(*get_vq_affinity)(struct vdpa_device *vdev,
 						 u16 idx);
+	void (*set_irq_affinity)(struct vdpa_device *vdev,
+				 struct irq_affinity *desc);
 
 	/* DMA ops */
 	int (*set_map)(struct vdpa_device *vdev, unsigned int asid,