Messages in this thread | | | Subject | Re: [PATCH RFC v6 02/11] vhost: use batched get_vq_desc version | From | Jason Wang <> | Date | Mon, 15 Jun 2020 10:43:42 +0800 |
| |
On 2020/6/11 下午5:06, Michael S. Tsirkin wrote: > On Thu, Jun 11, 2020 at 11:02:57AM +0800, Jason Wang wrote: >> On 2020/6/10 下午7:05, Michael S. Tsirkin wrote: >>>>> +EXPORT_SYMBOL_GPL(vhost_get_vq_desc); >>>>> /* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */ >>>>> void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n) >>>>> { >>>>> + unfetch_descs(vq); >>>>> vq->last_avail_idx -= n; >>>> So unfetch_descs() has decreased last_avail_idx. >>>> Can we fix this by letting unfetch_descs() return the number and then we can >>>> do: >>>> >>>> int d = unfetch_descs(vq); >>>> vq->last_avail_idx -= (n > d) ? n - d: 0; >>>> >>>> Thanks >>> That's intentional I think - we need both. >> >> Yes, but: >> >> >>> Unfetch_descs drops the descriptors in the cache that were >>> *not returned to caller* through get_vq_desc. >>> >>> vhost_discard_vq_desc drops the ones that were returned through get_vq_desc. >>> >>> Did I miss anything? >> We could count some descriptors twice, consider the case e.g we only cache >> on descriptor: >> >> fetch_descs() >> fetch_buf() >> last_avail_idx++; >> >> Then we want do discard it: >> vhost_discard_avail_buf(1) >> unfetch_descs() >> last_avail_idx--; >> last_avail_idx -= 1; >> >> Thanks > > I don't think that happens. vhost_discard_avail_buf(1) is only called > after get vhost_get_avail_buf. vhost_get_avail_buf increments > first_desc. unfetch_descs only counts from first_desc to ndescs. > > If I'm wrong, could you show values of first_desc and ndescs in this > scenario?
You're right, fetch_descriptor could not be called directly from the device code.
Thanks
>
| |