On Sun, Jan 22, 2017 at 04:16:11PM +0800, Jason Wang wrote:
>
>
> On 2017年01月20日 21:40, Stefan Hajnoczi wrote:
> > On Fri, Jan 20, 2017 at 11:23:13AM +0800, Jason Wang wrote:
> > >
> > > On 2017年01月19日 18:43, Stefan Hajnoczi wrote:
> > > > Propagate the error when vhost_vq_init_access() fails and set
> > > > vq->private_data to NULL.
> > > >
> > > > Signed-off-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
> > > > ---
> > > > drivers/vhost/vsock.c | 13 +++++++++----
> > > > 1 file changed, 9 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> > > > index bbbf5885..ce5e63d 100644
> > > > --- a/drivers/vhost/vsock.c
> > > > +++ b/drivers/vhost/vsock.c
> > > > @@ -373,6 +373,7 @@ static void vhost_vsock_handle_rx_kick(struct vhost_work *work)
> > > > static int vhost_vsock_start(struct vhost_vsock *vsock)
> > > > {
> > > > + struct vhost_virtqueue *vq;
> > > > size_t i;
> > > > int ret;
> > > > @@ -383,19 +384,20 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
> > > > goto err;
> > > > for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
> > > > - struct vhost_virtqueue *vq = &vsock->vqs[i];
> > > > + vq = &vsock->vqs[i];
> > > > mutex_lock(&vq->mutex);
> > > > if (!vhost_vq_access_ok(vq)) {
> > > > ret = -EFAULT;
> > > > - mutex_unlock(&vq->mutex);
> > > > goto err_vq;
> > > > }
> > > > if (!vq->private_data) {
> > > > vq->private_data = vsock;
> > > > - vhost_vq_init_access(vq);
> > > > + ret = vhost_vq_init_access(vq);
> > > > + if (ret)
> > > > + goto err_vq;
> > > > }
> > > > mutex_unlock(&vq->mutex);
> > > > @@ -405,8 +407,11 @@ static int vhost_vsock_start(struct vhost_vsock *vsock)
> > > > return 0;
> > > > err_vq:
> > > > + vq->private_data = NULL;
> > > Looks like this will be done twice?
> > Here we still hold the vq mutex. If we release the mutex then a
> > virtqueue kick could be processed.
> >
> > Stefan
>
> Ok, then it's better to assign vq->private_data after vhost_vq_init_acess()
> succeed.
No, that won't work because vhost_vq_init_acess() requires that
vq->private_data is non-NULL.
Stefan
Attachment:
signature.asc
Description: PGP signature
