search for: vhost_discard_avail_buf

.... > > 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

.... > > 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

...able_notify(&net->dev, > @@ -808,7 +823,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) > goto done; > } else if (unlikely(err != -ENOSPC)) { > vhost_tx_batch(net, nvq, sock, &msg); > - vhost_discard_vq_desc(vq, 1); > + vhost_discard_avail_bufs(vq, &buf, 1); > vhost_net_enable_vq(net, vq); > break; > } > @@ -829,7 +844,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) > /* TODO: Check specific error and bomb out unless ENOBUFS? */ > err = sock->ops->sendmsg(so...

...@@ -2375,7 +2379,15 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq, > > return ret; > } > -EXPORT_SYMBOL_GPL(vhost_get_vq_desc); > +EXPORT_SYMBOL_GPL(vhost_get_avail_buf); > + > +/* Reverse the effect of vhost_get_avail_buf. Useful for error handling. */ > +void vhost_discard_avail_bufs(struct vhost_virtqueue *vq, > + struct vhost_buf *buf, unsigned count) > +{ > + vhost_discard_vq_desc(vq, count); > +} > +EXPORT_SYMBOL_GPL(vhost_discard_avail_bufs); > > static int __vhost_add_used_n(struct vhost_virtqueue *vq, > struct vring_used_ele...

...@@ -2375,7 +2379,15 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq, > > return ret; > } > -EXPORT_SYMBOL_GPL(vhost_get_vq_desc); > +EXPORT_SYMBOL_GPL(vhost_get_avail_buf); > + > +/* Reverse the effect of vhost_get_avail_buf. Useful for error handling. */ > +void vhost_discard_avail_bufs(struct vhost_virtqueue *vq, > + struct vhost_buf *buf, unsigned count) > +{ > + vhost_discard_vq_desc(vq, count); > +} > +EXPORT_SYMBOL_GPL(vhost_discard_avail_bufs); > > static int __vhost_add_used_n(struct vhost_virtqueue *vq, > struct vring_used_ele...

...} else if (unlikely(vhost_enable_notify(&net->dev, @@ -808,7 +823,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) goto done; } else if (unlikely(err != -ENOSPC)) { vhost_tx_batch(net, nvq, sock, &msg); - vhost_discard_vq_desc(vq, 1); + vhost_discard_avail_bufs(vq, &buf, 1); vhost_net_enable_vq(net, vq); break; } @@ -829,7 +844,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock) /* TODO: Check specific error and bomb out unless ENOBUFS? */ err = sock->ops->sendmsg(sock, &msg, len); if (unlikely...

...>first_desc; i < vq->ndescs; ++i) @@ -2375,7 +2379,15 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq, return ret; } -EXPORT_SYMBOL_GPL(vhost_get_vq_desc); +EXPORT_SYMBOL_GPL(vhost_get_avail_buf); + +/* Reverse the effect of vhost_get_avail_buf. Useful for error handling. */ +void vhost_discard_avail_bufs(struct vhost_virtqueue *vq, + struct vhost_buf *buf, unsigned count) +{ + vhost_discard_vq_desc(vq, count); +} +EXPORT_SYMBOL_GPL(vhost_discard_avail_bufs); static int __vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads, @@ -2459,6 +2471,26 @@ int vhost_...

...vhost_get_vq_desc(struct vhost_virtqueue *vq, > > return ret; > > } > > -EXPORT_SYMBOL_GPL(vhost_get_vq_desc); > > +EXPORT_SYMBOL_GPL(vhost_get_avail_buf); > > + > > +/* Reverse the effect of vhost_get_avail_buf. Useful for error handling. */ > > +void vhost_discard_avail_bufs(struct vhost_virtqueue *vq, > > + struct vhost_buf *buf, unsigned count) > > +{ > > + vhost_discard_vq_desc(vq, count); > > +} > > +EXPORT_SYMBOL_GPL(vhost_discard_avail_bufs); > > static int __vhost_add_used_n(struct vhost_virtqueue *vq, > >...

...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...

We let the specifics of the ring format seep through to vhost API callers - mostly because there was only one format so it was hard to imagine what an independent API would look like. Now that there's an alternative in form of the packed ring, it's easier to see the issues, and fixing them is perhaps the cleanest way to add support for more formats. This patchset does this by indtroducing

This intentionally leaves "fixup" changes separate - hopefully that is enough to fix vhost-net crashes reported here, but it helps me keep track of what changed. I will naturally squash them later when we are done. This adds infrastructure required for supporting multiple ring formats. The idea is as follows: we convert descriptors to an independent format first, and process that

This intentionally leaves "fixup" changes separate - hopefully that is enough to fix vhost-net crashes reported here, but it helps me keep track of what changed. I will naturally squash them later when we are done. This adds infrastructure required for supporting multiple ring formats. The idea is as follows: we convert descriptors to an independent format first, and process that

This adds infrastructure required for supporting multiple ring formats. The idea is as follows: we convert descriptors to an independent format first, and process that converting to iov later. Used ring is similar: we fetch into an independent struct first, convert that to IOV later. The point is that we have a tight loop that fetches descriptors, which is good for cache utilization. This will

This adds infrastructure required for supporting multiple ring formats. The idea is as follows: we convert descriptors to an independent format first, and process that converting to iov later. Used ring is similar: we fetch into an independent struct first, convert that to IOV later. The point is that we have a tight loop that fetches descriptors, which is good for cache utilization. This will

...t vhost_virtqueue *, int n); int vhost_get_avail_buf(struct vhost_virtqueue *, struct vhost_buf *buf, struct iovec iov[], unsigned int iov_count, unsigned int *out_num, unsigned int *in_num, @@ -209,13 +204,6 @@ int vhost_get_avail_buf(struct vhost_virtqueue *, struct vhost_buf *buf, void vhost_discard_avail_bufs(struct vhost_virtqueue *, struct vhost_buf *, unsigned count); int vhost_vq_init_access(struct vhost_virtqueue *); -int vhost_add_used(struct vhost_virtqueue *, unsigned int head, int len); -int vhost_add_used_n(struct vhost_virtqueue *, struct vring_used_elem *heads, - unsigned c...

This still causes corruption issues for people so don't try to use in production please. Posting to expedite debugging. This adds infrastructure required for supporting multiple ring formats. The idea is as follows: we convert descriptors to an independent format first, and process that converting to iov later. Used ring is similar: we fetch into an independent struct first, convert that to

This still causes corruption issues for people so don't try to use in production please. Posting to expedite debugging. This adds infrastructure required for supporting multiple ring formats. The idea is as follows: we convert descriptors to an independent format first, and process that converting to iov later. Used ring is similar: we fetch into an independent struct first, convert that to

On 2020/6/8 ??8:52, Michael S. Tsirkin wrote: > As testing shows no performance change, switch to that now. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > Signed-off-by: Eugenio P?rez <eperezma at redhat.com> > Link: https://lore.kernel.org/r/20200401183118.8334-3-eperezma at redhat.com > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> >

On 2020/6/8 ??8:52, Michael S. Tsirkin wrote: > As testing shows no performance change, switch to that now. > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > Signed-off-by: Eugenio P?rez <eperezma at redhat.com> > Link: https://lore.kernel.org/r/20200401183118.8334-3-eperezma at redhat.com > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> >