On Tue, Dec 7, 2021 at 10:45 AM Mike Christie <michael.christie at oracle.com> wrote:> > From: Andrey Ryabinin <arbn at yandex-team.com> > > vhost_vsock_flush() calls vhost_work_dev_flush(vsock->vqs[i].poll.dev) > before vhost_work_dev_flush(&vsock->dev). This seems pointless > as vsock->vqs[i].poll.dev is the same as &vsock->dev and several flushes > in a row doesn't do anything useful, one is just enough. > > Signed-off-by: Andrey Ryabinin <arbn at yandex-team.com> > Reviewed-by: Stefano Garzarella <sgarzare at redhat.com> > Signed-off-by: Mike Christie <michael.christie at oracle.com> > --- > drivers/vhost/vsock.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c > index 2339587bcd31..1f38160b249d 100644 > --- a/drivers/vhost/vsock.c > +++ b/drivers/vhost/vsock.c > @@ -703,11 +703,6 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file) > > static void vhost_vsock_flush(struct vhost_vsock *vsock) > { > - int i; > - > - for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) > - if (vsock->vqs[i].handle_kick) > - vhost_work_dev_flush(vsock->vqs[i].poll.dev); > vhost_work_dev_flush(&vsock->dev); > }Is this better to be consistent with vhost-net so that we can simply remove the vhost_vsock_flush() wrapper here. Thanks> > -- > 2.25.1 >
michael.christie at oracle.com
2021-Dec-08 17:35 UTC
[PATCH 5/7] vhost_vsock: simplify vhost_vsock_flush()
On 12/7/21 9:53 PM, Jason Wang wrote:> On Tue, Dec 7, 2021 at 10:45 AM Mike Christie > <michael.christie at oracle.com> wrote: >> >> From: Andrey Ryabinin <arbn at yandex-team.com> >> >> vhost_vsock_flush() calls vhost_work_dev_flush(vsock->vqs[i].poll.dev) >> before vhost_work_dev_flush(&vsock->dev). This seems pointless >> as vsock->vqs[i].poll.dev is the same as &vsock->dev and several flushes >> in a row doesn't do anything useful, one is just enough. >> >> Signed-off-by: Andrey Ryabinin <arbn at yandex-team.com> >> Reviewed-by: Stefano Garzarella <sgarzare at redhat.com> >> Signed-off-by: Mike Christie <michael.christie at oracle.com> >> --- >> drivers/vhost/vsock.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c >> index 2339587bcd31..1f38160b249d 100644 >> --- a/drivers/vhost/vsock.c >> +++ b/drivers/vhost/vsock.c >> @@ -703,11 +703,6 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file) >> >> static void vhost_vsock_flush(struct vhost_vsock *vsock) >> { >> - int i; >> - >> - for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) >> - if (vsock->vqs[i].handle_kick) >> - vhost_work_dev_flush(vsock->vqs[i].poll.dev); >> vhost_work_dev_flush(&vsock->dev); >> } > > Is this better to be consistent with vhost-net so that we can simply > remove the vhost_vsock_flush() wrapper here. >I didn't understand that comment. Did you mean consistent as they both have vhost_vsock/net_flush functions or as in they prefer to not have one line wrappers? Before and after this patchset, both net and vsock have a vhost_vsock/net_flush function, so maybe you didn't mean that. I think the wrapper is not very useful and could be removed. However, I thought vsock preferred wrappers because we have vhost_vsock_free which is just a wrapper around kfree. I also noticed test.c is a fan of one line wrappers, but I see net and scsi do not do that.