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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 8 Mar 2017, Boris Ostrovsky wrote:
> >>> +
> >>> +         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?

That's right. First we read the header, to know how much data to read.


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

Sure.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel