On 11/30/2015 04:22 PM, Michael S. Tsirkin wrote:
> On Wed, Nov 25, 2015 at 03:11:28PM +0800, Jason Wang wrote:
>> Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
>> ---
>> drivers/vhost/vhost.c | 26 +++++++++++++++++---------
>> drivers/vhost/vhost.h | 1 +
>> 2 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 163b365..b86c5aa 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -1633,10 +1633,25 @@ void vhost_add_used_and_signal_n(struct vhost_dev *dev,
>> }
>> EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n);
>>
>> +bool vhost_vq_more_avail(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> +{
>> + __virtio16 avail_idx;
>> + int r;
>> +
>> + r = __get_user(avail_idx, &vq->avail->idx);
>> + if (r) {
>> + vq_err(vq, "Failed to check avail idx at %p: %d\n",
>> + &vq->avail->idx, r);
>> + return false;
> In patch 3 you are calling this under preempt disable,
> so this actually can fail and it isn't a VQ error.
>
Yes.
>> + }
>> +
>> + return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
>> +}
>> +EXPORT_SYMBOL_GPL(vhost_vq_more_avail);
>> +
>> /* OK, now we need to know about added descriptors. */
>> bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> {
>> - __virtio16 avail_idx;
>> int r;
>>
>> if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
>> @@ -1660,14 +1675,7 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>> /* They could have slipped one in as we were doing that: make
>> * sure it's written, then check again. */
>> smp_mb();
>> - r = __get_user(avail_idx, &vq->avail->idx);
>> - if (r) {
>> - vq_err(vq, "Failed to check avail idx at %p: %d\n",
>> - &vq->avail->idx, r);
>> - return false;
>> - }
>> -
>> - return vhost16_to_cpu(vq, avail_idx) != vq->avail_idx;
>> + return vhost_vq_more_avail(dev, vq);
>> }
>> EXPORT_SYMBOL_GPL(vhost_enable_notify);
>>
> This path does need an error though.
> It's probably easier to just leave this call site alone.
Ok, will leave this function as is and remove the vq_err() in
vhost_vq_more_avail().
Thanks
>
>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>> index 43284ad..2f3c57c 100644
>> --- a/drivers/vhost/vhost.h
>> +++ b/drivers/vhost/vhost.h
>> @@ -159,6 +159,7 @@ void vhost_add_used_and_signal_n(struct vhost_dev *, struct vhost_virtqueue *,
>> struct vring_used_elem *heads, unsigned count);
>> void vhost_signal(struct vhost_dev *, struct vhost_virtqueue *);
>> void vhost_disable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>> +bool vhost_vq_more_avail(struct vhost_dev *, struct vhost_virtqueue *);
>> bool vhost_enable_notify(struct vhost_dev *, struct vhost_virtqueue *);
>>
>> int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
>> --
>> 2.5.0
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
