Message ID | 20110707125728.GA2469@redhat.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On 07/07/2011 08:57 PM, Michael S. Tsirkin wrote: > Subject: vhost: used ring logging cleanup > > remove extra log bit setting for used ring updates: it's no longer > necessary. Also, use vhost_avail_event instead of duplicating offset > math. > > Signed-off-by: Michael S. Tsirkin<mst@redhat.com> > --- > >> We need set log when updating used flags and avail event. Otherwise guest may >> see stale values after migration and then do not exit or exit unexpectedly. >> >> Signed-off-by: Jason Wang<jasowang@redhat.com> > OK but this means we set the log twice now. > Also, hardcording offset is not as nice as using > vhost_avail_event. So I think the below is needed > on top. Comments? > Right, thanks for the cleanup. > drivers/vhost/vhost.c | 29 +++++++++-------------------- > 1 files changed, 9 insertions(+), 20 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 540591b..c5f96ba 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -946,14 +946,16 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, > > static int vhost_update_used_flags(struct vhost_virtqueue *vq) > { > + void __user *used; > if (put_user(vq->used_flags,&vq->used->flags)< 0) > return -EFAULT; > if (unlikely(vq->log_used)) { > /* Make sure the flag is seen before log. */ > smp_wmb(); > /* Log used flag write. */ > - log_write(vq->log_base, > - vq->log_addr + offsetof(struct vring_used, flags), > + used =&vq->used->flags; > + log_write(vq->log_base, vq->log_addr + > + (used - (void __user *)vq->used), > sizeof vq->used->flags); > if (vq->log_ctx) > eventfd_signal(vq->log_ctx, 1); > @@ -966,13 +968,14 @@ static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event) > if (put_user(vq->avail_idx, vhost_avail_event(vq))) > return -EFAULT; > if (unlikely(vq->log_used)) { > + void __user *used; > /* Make sure the event is seen before log. */ > smp_wmb(); > /* Log avail event write */ > - log_write(vq->log_base, > - vq->log_addr + offsetof(struct vring_used, > - ring[vq->num]), > - sizeof avail_event); > + used = vhost_avail_event(vq); > + log_write(vq->log_base, vq->log_addr + > + (used - (void __user *)vq->used), > + sizeof *vhost_avail_event(vq)); > if (vq->log_ctx) > eventfd_signal(vq->log_ctx, 1); > } > @@ -1474,20 +1477,6 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq) > return false; > } > } > - if (unlikely(vq->log_used)) { > - void __user *used; > - /* Make sure data is seen before log. */ > - smp_wmb(); > - used = vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX) ? > - &vq->used->flags : vhost_avail_event(vq); > - /* Log used flags or event index entry write. Both are 16 bit > - * fields. */ > - log_write(vq->log_base, vq->log_addr + > - (used - (void __user *)vq->used), > - sizeof(u16)); > - if (vq->log_ctx) > - eventfd_signal(vq->log_ctx, 1); > - } > /* They could have slipped one in as we were doing that: make > * sure it's written, then check again. */ > smp_mb(); -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 540591b..c5f96ba 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -946,14 +946,16 @@ int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log, static int vhost_update_used_flags(struct vhost_virtqueue *vq) { + void __user *used; if (put_user(vq->used_flags, &vq->used->flags) < 0) return -EFAULT; if (unlikely(vq->log_used)) { /* Make sure the flag is seen before log. */ smp_wmb(); /* Log used flag write. */ - log_write(vq->log_base, - vq->log_addr + offsetof(struct vring_used, flags), + used = &vq->used->flags; + log_write(vq->log_base, vq->log_addr + + (used - (void __user *)vq->used), sizeof vq->used->flags); if (vq->log_ctx) eventfd_signal(vq->log_ctx, 1); @@ -966,13 +968,14 @@ static int vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event) if (put_user(vq->avail_idx, vhost_avail_event(vq))) return -EFAULT; if (unlikely(vq->log_used)) { + void __user *used; /* Make sure the event is seen before log. */ smp_wmb(); /* Log avail event write */ - log_write(vq->log_base, - vq->log_addr + offsetof(struct vring_used, - ring[vq->num]), - sizeof avail_event); + used = vhost_avail_event(vq); + log_write(vq->log_base, vq->log_addr + + (used - (void __user *)vq->used), + sizeof *vhost_avail_event(vq)); if (vq->log_ctx) eventfd_signal(vq->log_ctx, 1); } @@ -1474,20 +1477,6 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq) return false; } } - if (unlikely(vq->log_used)) { - void __user *used; - /* Make sure data is seen before log. */ - smp_wmb(); - used = vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX) ? - &vq->used->flags : vhost_avail_event(vq); - /* Log used flags or event index entry write. Both are 16 bit - * fields. */ - log_write(vq->log_base, vq->log_addr + - (used - (void __user *)vq->used), - sizeof(u16)); - if (vq->log_ctx) - eventfd_signal(vq->log_ctx, 1); - } /* They could have slipped one in as we were doing that: make * sure it's written, then check again. */ smp_mb();