On Tue, Oct 14, 2025 at 5:13 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Thu, Sep 25, 2025 at 06:37:08PM +0800, Jason Wang wrote:
> > @@ -683,7 +707,12 @@ static inline int virtqueue_add_split(struct
> > vring_virtqueue *vq,
> > vq->vq.num_free -= descs_used;
> >
> > /* Update free pointer */
> > - if (indirect)
> > + if (virtqueue_is_in_order(vq)) {
> > + vq->free_head += descs_used;
> > + if (vq->free_head >= vq->split.vring.num)
> > + vq->free_head -= vq->split.vring.num;
> > + vq->split.desc_state[head].total_len = total_len;;
> > + } else if (indirect)
> > vq->free_head = vq->split.desc_extra[head].next;
> > else
> > vq->free_head = i;
>
> So in order is clearly doing something funky with the free_head.
> It's no longer a head of a linked list of free descriptors, is it?
> what is it doing and why?
Since descriptors were consumed in order, it's not yet a list. But
it's still the free head.
How about something like:
/*
* Without IN_ORDER it's the head of free buffer list. With
* IN_ORDER and SPLIT, it's the next available buffer
* index. With IN_ORDER and PACKED, it's unused.
*/
unsigned int free_head;
> Please add code comments to explain
> both where free_list is defined and where it's used.
> For example, virtqueue_vring_attach_packed only inits free_list
> if not in order. So who will init it for in order? And so on.
Packed virtqueue reuses the next_avail_idx, so free_head is unused there.
How about:
static void virtqueue_vring_attach_packed(struct vring_virtqueue *vq,
struct
vring_virtqueue_packed *vring_packed)
{
vq->packed = *vring_packed;
if (virtqueue_is_in_order(vq))
vq->batch_last.id = vq->packed.vring.num;
else {
/*
* Put everything in free lists. Note that
* next_avail_idx is sufficient with IN_ORDER so
* free_head is unused.
*/
vq->free_head = 0 ;
}
}
Thanks
>
> --
> MST
>
