linux-kernel - Re: [PATCH V3 2/3] vduse: suspend

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 21 May 2024 09:38:52 -0400
From: Steven Sistare <steven.sistare@...cle.com>
To: Jason Wang <jasowang@...hat.com>
Cc: virtualization@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Si-Wei Liu <si-wei.liu@...cle.com>,
        Eugenio Perez Martin <eperezma@...hat.com>,
        Xie Yongji <xieyongji@...edance.com>,
        Stefano Garzarella <sgarzare@...hat.com>
Subject: Re: [PATCH V3 2/3] vduse: suspend

On 5/20/2024 10:30 PM, Jason Wang wrote:
> On Mon, May 20, 2024 at 11:21 PM Steve Sistare
> <steven.sistare@...cle.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@...cle.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_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.

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.

- 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
>>
>