search for: fetch_buf

Return code of fetch_buf is confusing, so callers resort to tricks to get to sane values. Let's switch to something standard: 0 empty, >0 non-empty, <0 error. Signed-off-by: Michael S. Tsirkin <mst at redhat.com> --- drivers/vhost/vhost.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8...

On 2020/6/2 ??9:06, Michael S. Tsirkin wrote: > Return code of fetch_buf is confusing, so callers resort to > tricks to get to sane values. Let's switch to something standard: > 0 empty, >0 non-empty, <0 error. > > Signed-off-by: Michael S. Tsirkin<mst at redhat.com> > --- > drivers/vhost/vhost.c | 24 ++++++++++++++++-------- > 1...

...> + return 1; > > > > > > > + > > > > > > > + for (ret = 1; > > > > > > > + ret > 0 && vq->ndescs <= vhost_vq_num_batch_descs(vq); > > > > > > > + ret = fetch_buf(vq)) > > > > > > > + ; > > > > > > > > > > > > (Expanding comment in V6): > > > > > > > > > > > > We get an infinite loop this way: > > > > > > * vq->ndescs == 0, so we ca...

...gt; > > > + if (vq->ndescs) > > > > + return 1; > > > > + > > > > + for (ret = 1; > > > > + ret > 0 && vq->ndescs <= vhost_vq_num_batch_descs(vq); > > > > + ret = fetch_buf(vq)) > > > > + ; > > > > > > (Expanding comment in V6): > > > > > > We get an infinite loop this way: > > > * vq->ndescs == 0, so we call fetch_buf() here > > > * fetch_buf gets less than vhost_vq_num_batch_descs(vq)...

...t16_to_cpu(vq, desc->flags) & VHOST_DESC_FLAGS; > h->id = id; > > return 0; > @@ -2450,7 +2454,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, > return 0; > } > > -static int fetch_descs(struct vhost_virtqueue *vq) > +static int fetch_buf(struct vhost_virtqueue *vq) > { > struct vring_desc desc; > unsigned int i, head, found = 0; > @@ -2462,7 +2466,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) > /* Check it isn't doing very strange things with descriptor numbers. */ > last_avail_idx = v...

...t16_to_cpu(vq, desc->flags) & VHOST_DESC_FLAGS; > h->id = id; > > return 0; > @@ -2450,7 +2454,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, > return 0; > } > > -static int fetch_descs(struct vhost_virtqueue *vq) > +static int fetch_buf(struct vhost_virtqueue *vq) > { > struct vring_desc desc; > unsigned int i, head, found = 0; > @@ -2462,7 +2466,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) > /* Check it isn't doing very strange things with descriptor numbers. */ > last_avail_idx = v...

...st.h | 12 ------------ > 2 files changed, 8 insertions(+), 40 deletions(-) > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > index 72ee55c810c4..e6931b760b61 100644 > --- a/drivers/vhost/vhost.c > +++ b/drivers/vhost/vhost.c > @@ -2299,12 +2299,12 @@ static int fetch_buf(struct vhost_virtqueue *vq) > return 1; > } > > -/* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */ > +/* Revert the effect of fetch_buf. Useful for error handling. */ > +static > void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n) &gt...

...vq->ndescs = 0; > > + } > > + > > + if (vq->ndescs) > > + return 1; > > + > > + for (ret = 1; > > + ret > 0 && vq->ndescs <= vhost_vq_num_batch_descs(vq); > > + ret = fetch_buf(vq)) > > + ; > > (Expanding comment in V6): > > We get an infinite loop this way: > * vq->ndescs == 0, so we call fetch_buf() here > * fetch_buf gets less than vhost_vq_num_batch_descs(vq); descriptors. ret = 1 > * This loop calls again fetch_buf, but v...

...->flags) & VHOST_DESC_FLAGS; > > h->id = id; > > return 0; > > @@ -2450,7 +2454,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, > > return 0; > > } > > -static int fetch_descs(struct vhost_virtqueue *vq) > > +static int fetch_buf(struct vhost_virtqueue *vq) > > { > > struct vring_desc desc; > > unsigned int i, head, found = 0; > > @@ -2462,7 +2466,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) > > /* Check it isn't doing very strange things with descriptor numbers. */ &...

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

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

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

...scs = vq->first_desc = 0; > break; > case VHOST_GET_VRING_BASE: > s.index = idx; > @@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, > return 0; > } > > -static int fetch_descs(struct vhost_virtqueue *vq) > +static int fetch_buf(struct vhost_virtqueue *vq) > { > unsigned int i, head, found = 0; > struct vhost_desc *last; > @@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) > /* Check it isn't doing very strange things with descriptor numbers. */ > last_avail_idx =...

...scs = vq->first_desc = 0; > break; > case VHOST_GET_VRING_BASE: > s.index = idx; > @@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, > return 0; > } > > -static int fetch_descs(struct vhost_virtqueue *vq) > +static int fetch_buf(struct vhost_virtqueue *vq) > { > unsigned int i, head, found = 0; > struct vhost_desc *last; > @@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) > /* Check it isn't doing very strange things with descriptor numbers. */ > last_avail_idx =...

...---------------- drivers/vhost/vhost.h | 12 ------------ 2 files changed, 8 insertions(+), 40 deletions(-) diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c index 72ee55c810c4..e6931b760b61 100644 --- a/drivers/vhost/vhost.c +++ b/drivers/vhost/vhost.c @@ -2299,12 +2299,12 @@ static int fetch_buf(struct vhost_virtqueue *vq) return 1; } -/* Reverse the effect of vhost_get_vq_desc. Useful for error handling. */ +/* Revert the effect of fetch_buf. Useful for error handling. */ +static void vhost_discard_vq_desc(struct vhost_virtqueue *vq, int n) { vq->last_avail_idx -= n; } -EXPOR...

...nges from v2: - fixed indirect descriptor batching reported by Jason Wang Changes from v1: - typo fixes Michael S. Tsirkin (13): vhost: option to fetch descriptors through an independent struct vhost: use batched version by default vhost: batching fetches vhost: cleanup fetch_buf return code handling vhost/net: pass net specific struct pointer vhost: reorder functions vhost: format-independent API for used buffers vhost/net: convert to new API: heads->bufs vhost/net: avoid iov length math vhost/test: convert to the buf API vhost/scsi: switch to buf APIs v...

...gt;avail_idx = vq->last_avail_idx; + vq->ndescs = vq->first_desc = 0; break; case VHOST_GET_VRING_BASE: s.index = idx; @@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, return 0; } -static int fetch_descs(struct vhost_virtqueue *vq) +static int fetch_buf(struct vhost_virtqueue *vq) { unsigned int i, head, found = 0; struct vhost_desc *last; @@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vq->last_avail_idx; - if (vq-&...

...gt;avail_idx = vq->last_avail_idx; + vq->ndescs = vq->first_desc = 0; break; case VHOST_GET_VRING_BASE: s.index = idx; @@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, return 0; } -static int fetch_descs(struct vhost_virtqueue *vq) +static int fetch_buf(struct vhost_virtqueue *vq) { unsigned int i, head, found = 0; struct vhost_desc *last; @@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vq->last_avail_idx; - if (vq-&...

...gt;avail_idx = vq->last_avail_idx; + vq->ndescs = vq->first_desc = 0; break; case VHOST_GET_VRING_BASE: s.index = idx; @@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue *vq, return 0; } -static int fetch_descs(struct vhost_virtqueue *vq) +static int fetch_buf(struct vhost_virtqueue *vq) { unsigned int i, head, found = 0; struct vhost_desc *last; @@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq) /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vq->last_avail_idx; - if (vq-&...