Received-SPF: pass (google.com: domain of linux-kernel+bounces-185730-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <1716218462-84587-1-git-send-email-steven.sistare@oracle.com>
 <1716218462-84587-3-git-send-email-steven.sistare@oracle.com>
 <CACGkMEtfjHzdBBwgUWgRHDc5opekvgTuXp-_z4ku6L5cqOeAJA@mail.gmail.com> <b55cccfc-67b0-4f21-9070-127af6fd07a7@oracle.com>
In-Reply-To: <b55cccfc-67b0-4f21-9070-127af6fd07a7@oracle.com>
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 22 May 2024 11:40:49 +0800
Message-ID: <CACGkMEu4WZnqcTPyq566X5aWL-mL1KSR7XWoTKq5AU0mSsB9_A@mail.gmail.com>
Subject: Re: [PATCH V3 2/3] vduse: suspend
To: Steven Sistare <steven.sistare@oracle.com>
Cc: virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, 
	"Michael S. Tsirkin" <mst@redhat.com>, Si-Wei Liu <si-wei.liu@oracle.com>, 
	Eugenio Perez Martin <eperezma@redhat.com>, Xie Yongji <xieyongji@bytedance.com>, 
	Stefano Garzarella <sgarzare@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, May 21, 2024 at 9:39=E2=80=AFPM Steven Sistare
<steven.sistare@oracle.com> wrote:
>
> On 5/20/2024 10:30 PM, Jason Wang wrote:
> > On Mon, May 20, 2024 at 11:21=E2=80=AFPM Steve Sistare
> > <steven.sistare@oracle.com> 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@oracle.com>
> >> ---
> >>   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_us=
er/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 =3D 0; i < dev->vq_num; i++) {
> >> +               struct vduse_virtqueue *vq =3D dev->vqs[i];
> >> +
> >> +               flush_work(&vq->inject);
> >> +               flush_work(&vq->kick);
> >> +       }
> >> +}
> >> +
> >>   static int vduse_vdpa_set_vq_address(struct vdpa_device *vdpa, u16 i=
dx,
> >>                                  u64 desc_area, u64 driver_area,
> >>                                  u64 device_area)
> >> @@ -724,6 +736,17 @@ static int vduse_vdpa_reset(struct vdpa_device *v=
dpa)
> >>          return ret;
> >>   }
> >>
> >> +static int vduse_vdpa_suspend(struct vdpa_device *vdpa)
> >> +{
> >> +       struct vduse_dev *dev =3D 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_suspe=
nd().

>
> 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 =3D -EINVAL;
> >
> >          down_read(&dev->rwsem);
> >          if (!(dev->status & VIRTIO_CONFIG_S_DRIVER_OK))
> >                  goto unlock;
> >
> >          ret =3D 0;
> >          if (irq_effective_cpu =3D=3D 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 =3D vdpa_to_vduse(vdpa);
> >> @@ -806,6 +829,7 @@ static const struct vdpa_config_ops vduse_vdpa_con=
fig_ops =3D {
> >>          .set_vq_affinity        =3D vduse_vdpa_set_vq_affinity,
> >>          .get_vq_affinity        =3D vduse_vdpa_get_vq_affinity,
> >>          .reset                  =3D vduse_vdpa_reset,
> >> +       .suspend                =3D vduse_vdpa_suspend,
> >>          .set_map                =3D vduse_vdpa_set_map,
> >>          .free                   =3D vduse_vdpa_free,
> >>   };
> >> --
> >> 2.39.3
> >>
> >
>