Messages in this thread | | | From | Jason Wang <> | Date | Thu, 7 Dec 2023 16:46:15 +0800 | Subject | Re: [PATCH v3 6/7] vduse: Update the vq_info in ioctl VDUSE_VQ_GET_INFO |
| |
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <lulu@redhat.com> wrote: > > Once the reconnect memory pages are mapped to userspace, the userspace > application will update the "reconnected" bit in the > "struct vduse_dev_reconnect_data". > The kernel will then check this bit. If it is not set to > "VDUSE_NOT_RECONNECT", it means that the application has been > reconnected, and the kernel will synchronize the vq information.
Can you explain why we need such a flag?
> > Signed-off-by: Cindy Lu <lulu@redhat.com> > --- > drivers/vdpa/vdpa_user/vduse_dev.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > index f55f415629de..422f1aedebac 100644 > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > @@ -1193,6 +1193,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > struct vduse_vq_info vq_info; > struct vduse_virtqueue *vq; > u32 index; > + unsigned long vaddr; > + struct vduse_vq_state *vq_reconnect; > + struct vduse_dev_reconnect_data *dev_reconnect; > > ret = -EFAULT; > if (copy_from_user(&vq_info, argp, sizeof(vq_info))) > @@ -1209,6 +1212,12 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > vq_info.device_addr = vq->device_addr; > vq_info.num = vq->num; > > + vaddr = dev->vdpa_reconnect_vaddr; > + dev_reconnect = (struct vduse_dev_reconnect_data *)vaddr; > + > + vaddr = vq->vdpa_reconnect_vaddr; > + vq_reconnect = (struct vduse_vq_state *)vaddr; > + > if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) { > vq_info.packed.last_avail_counter = > vq->state.packed.last_avail_counter; > @@ -1218,9 +1227,22 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd, > vq->state.packed.last_used_counter; > vq_info.packed.last_used_idx = > vq->state.packed.last_used_idx; > - } else > + /*check if the vq is reconnect, if yes then update the info*/ > + if (dev_reconnect->reconnected != VDUSE_NOT_RECONNECT) {
So the dev_reconnect is shared, how to synchronize between them?
> + vq_info.packed.last_avail_idx = > + vq_reconnect->packed.last_avail_idx; > + vq_info.packed.last_avail_counter = > + vq_reconnect->packed.last_avail_counter; > + } > + } else { > vq_info.split.avail_index = > vq->state.split.avail_index; > + /*check if the vq is reconnect, if yes then update the info*/ > + if (dev_reconnect->reconnected != VDUSE_NOT_RECONNECT) {
If this only check is !=, I'd define it as VDUSE_RECONNECT to ease the code logic.
> + vq_info.split.avail_index = > + vq_reconnect->split.avail_index; > + } > + }
It looks like I miss something here
If I read the code correctly, vq_reconnect is written by userspace, so userspace query it again via the get_vq_info()? If this is true, userspace can just ignore the index via GET_VQ_INFO() or is it just for consistency?
Thanks
> > vq_info.ready = vq->ready; > > -- > 2.34.3 >
| |