[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: [PATCH v3] packed-ring: fix example code
Driver can't just check whether USED bit equals to the used wrap counter when checking whether a descriptor is a used descriptor, because driver also needs to check whether the descriptor has been made available. Below is an example: Assuming ring size is 4, ring's initial state will be: +----+----+----+----+ | 00 | 00 | 00 | 00 | +----+----+----+----+ 00 means AVAIL=0 USED=0, 01 means AVAIL=0 USED=1 10 means AVAIL=1 USED=0, 11 means AVAIL=1 USED=1 After driver made two descriptor chains available and each chain consists of two descriptors, the ring could be: +----+-----------+----+-----------+ | 10 | 10 (id=0) | 10 | 10 (id=1) | +----+-----------+----+-----------+ After device processed all the available descriptors and made them used (e.g. in order), the ring could be: +-----------+----+-----------+----+ | 11 (id=0) | 10 | 11 (id=1) | 10 | +-----------+----+-----------+----+ After driver processed all the used descriptors and made one descriptor (not chained, just one descriptor) available, the ring could be: +-----------+----+----+----+ | 01 (id=0) | 10 | 11 | 10 | +-----------+----+----+----+ After device made that descriptor used, the ring will be: +-----------+----+----+----+ | 00 (id=0) | 10 | 11 | 10 | +-----------+----+----+----+ If driver just checks whether USED bit equals to the used wrap counter when checking whether a descriptor is a used descriptor, after processing the first descriptor (whose AVAIL and USED bits are both 0), and advancing vq->next_used pointer, it will then also treat the next descriptor, i.e. the second descriptor (whose AVAIL and USED bits are 1 and 0 respectively) as a used descriptor which is wrong. Fixes: https://github.com/oasis-tcs/virtio-spec/issues/29 Signed-off-by: Tiwei Bie <tiwei.bie@intel.com> --- v2: - Add "Fixes" tag; - Refine commit log; v3: - Compare with vq->used_wrap_count (MST); - Add comments (MST); - Refine commit log; packed-ring.tex | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/packed-ring.tex b/packed-ring.tex index f24f49b..c8cd83b 100644 --- a/packed-ring.tex +++ b/packed-ring.tex @@ -687,16 +687,29 @@ vq->driver_event.flags = RING_EVENT_FLAGS_DISABLE; for (;;) { struct pvirtq_desc *d = vq->desc[vq->next_used]; + /* + * Check that + * 1. Descriptor has been made available. + * Note: there are many other ways to check this, e.g. + * track the number of outstanding available descriptors or buffers + * and check that it's not 0. + * 2. Descriptor has been used by device. + */ flags = d->flags; + bool avail = flags & VIRTQ_DESC_F_AVAIL; bool used = flags & VIRTQ_DESC_F_USED; - - if (used != vq->used_wrap_count) { + if (avail != vq->used_wrap_count || used != vq->used_wrap_count) { vq->driver_event.flags = RING_EVENT_FLAGS_ENABLE; memory_barrier(); + /* + * Re-test in case another thread submitted more descriptors + * and/or device used more descriptors before driver enabled events. + */ flags = d->flags; + bool avail = flags & VIRTQ_DESC_F_AVAIL; bool used = flags & VIRTQ_DESC_F_USED; - if (used != vq->used_wrap_count) { + if (avail != vq->used_wrap_count || used != vq->used_wrap_count) { break; } -- 2.17.1
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]