Re: [PATCH 6/7] xen/9pfs: receive responses

[Boris Ostrovsky]

>
>>> +
>>> +		if (xen_9pfs_queued(prod, cons, XEN_9PFS_RING_SIZE) < sizeof(h)) {
>>> +			notify_remote_via_irq(ring->irq);
>>> +			return;
>>> +		}
>>> +
>>> +		masked_prod = xen_9pfs_mask(prod, XEN_9PFS_RING_SIZE);
>>> +		masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
>>> +
>>> +		xen_9pfs_read_packet(ring->ring.in,
>>> +				masked_prod, &masked_cons,
>>> +				XEN_9PFS_RING_SIZE, &h, sizeof(h));
>>> +
>>> +		req = p9_tag_lookup(priv->client, h.tag);
>>> +		if (!req || req->status != REQ_STATUS_SENT) {
>>> +			dev_warn(&priv->dev->dev, "Wrong req tag=%x\n", h.tag);
>>> +			cons += h.size;
>>> +			mb();
>>> +			ring->intf->in_cons = cons;
>>> +			continue;
>>
>> I don't know what xen_9pfs_read_packet() does so perhaps it's done there
>> but shouldn't the pointers be updated regardless of the 'if' condition?
> This is the error path - the index is increased immediately. In the
> non-error case, we do that right after the next read_packet call, few
> lines below.
>
>
>>> +		}
>>> +
>>> +		memcpy(req->rc, &h, sizeof(h));
>>> +		req->rc->offset = 0;
>>> +
>>> +		masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
>>> +		xen_9pfs_read_packet(ring->ring.in,
>>> +				masked_prod, &masked_cons,
>>> +				XEN_9PFS_RING_SIZE, req->rc->sdata, h.size);
>>> +
>>> +		mb();
>>> +		cons += h.size;
>>> +		ring->intf->in_cons = cons;
>                    Here ^
>


So the second read is reading again from the same pointer in the ring,
but this time it gets the whole packet, including the header. The first
read was just poking at the header. Right?

If that's correct, can you add a comment somewhere? (unless this is
obvious to everyone else but me.)

-boris