Re: [PATCH V3 2/3] vduse: suspend

[Jason Wang]

On Tue, May 21, 2024 at 9:39 PM Steven Sistare
<steven.sistare@xxxxxxxxxx> wrote:
>
> On 5/20/2024 10:30 PM, Jason Wang wrote:
> > On Mon, May 20, 2024 at 11:21 PM Steve Sistare
> > <steven.sistare@xxxxxxxxxx> wrote:
> >>
> >> Support the suspend operation.  There is little to do, except flush to
> >> guarantee no workers are running when suspend returns.
> >>
> >> Signed-off-by: Steve Sistare <steven.sistare@xxxxxxxxxx>
> >> ---
> >>   drivers/vdpa/vdpa_user/vduse_dev.c | 24 ++++++++++++++++++++++++
> >>   1 file changed, 24 insertions(+)
> >>
> >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> >> index 73c89701fc9d..7dc46f771f12 100644
> >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> >> @@ -472,6 +472,18 @@ static void vduse_dev_reset(struct vduse_dev *dev)
> >>          up_write(&dev->rwsem);
> >>   }
> >>
> >> +static void vduse_flush_work(struct vduse_dev *dev)
> >> +{
> >> +       flush_work(&dev->inject);
> >> +
> >> +       for (int i = 0; i < dev->vq_num; i++) {
> >> +               struct vduse_virtqueue *vq = dev->vqs[i];
> >> +
> >> +               flush_work(&vq->inject);
> >> +               flush_work(&vq->kick);
> >> +       }
> >> +}
> >> +
> >>   static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 idx,
> >>                                  u64 desc_area, u64 driver_area,
> >>                                  u64 device_area)
> >> @@ -724,6 +736,17 @@ static int vduse_vdpa_reset(struct vdpa_device *vdpa)
> >>          return ret;
> >>   }
> >>
> >> +static int vduse_vdpa_suspend(struct vdpa_device *vdpa)
> >> +{
> >> +       struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> >> +
> >> +       down_write(&dev->rwsem);
> >> +       vduse_flush_work(dev);
> >> +       up_write(&dev->rwsem);
> >
> > Can this forbid the new work to be scheduled?
>
> Are you suggesting I return an error below if the dev is suspended?
> I can do that.

I mean the irq injection work can still be scheduled after vduse_vdpa_suspend().

>
> However, I now suspect this implementation of vduse_vdpa_suspend is not
> complete in other ways, so I withdraw this patch pending future work.
> Thanks for looking at it.

Ok.

Thanks

>
> - Steve
>
> > static int vduse_dev_queue_irq_work(struct vduse_dev *dev,
> >                                      struct work_struct *irq_work,
> >                                      int irq_effective_cpu)
> > {
> >          int ret = -EINVAL;
> >
> >          down_read(&dev->rwsem);
> >          if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
> >                  goto unlock;
> >
> >          ret = 0;
> >          if (irq_effective_cpu == IRQ_UNBOUND)
> >                  queue_work(vduse_irq_wq, irq_work);
> >          else
> >                  queue_work_on(irq_effective_cpu,
> >                        vduse_irq_bound_wq, irq_work);
> > unlock:
> >          up_read(&dev->rwsem);
> >
> >          return ret;
> > }
> >
> > Thanks
> >
> >> +
> >> +       return 0;
> >> +}
> >> +
> >>   static u32 vduse_vdpa_get_generation(struct vdpa_device *vdpa)
> >>   {
> >>          struct vduse_dev *dev = vdpa_to_vduse(vdpa);
> >> @@ -806,6 +829,7 @@ static const struct vdpa_config_ops vduse_vdpa_config_ops = {
> >>          .set_vq_affinity        = vduse_vdpa_set_vq_affinity,
> >>          .get_vq_affinity        = vduse_vdpa_get_vq_affinity,
> >>          .reset                  = vduse_vdpa_reset,
> >> +       .suspend                = vduse_vdpa_suspend,
> >>          .set_map                = vduse_vdpa_set_map,
> >>          .free                   = vduse_vdpa_free,
> >>   };
> >> --
> >> 2.39.3
> >>
> >
>