Subject: Re: [PATCH 6/7] xen/9pfs: receive responses
To: Stefano Stabellini <sstabellini@kernel.org>
References: <alpine.DEB.2.10.1703061147250.17906@sstabellini-ThinkPad-X260>
 <1488830488-18506-1-git-send-email-sstabellini@kernel.org>
 <1488830488-18506-6-git-send-email-sstabellini@kernel.org>
 <a132af66-cbb8-ee5e-cc5b-c82812007eca@oracle.com>
 <alpine.DEB.2.10.1703071658480.8160@sstabellini-ThinkPad-X260>
Cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org,
        Stefano Stabellini <stefano@aporeto.com>, jgross@suse.com,
        Eric Van Hensbergen <ericvh@gmail.com>,
        Ron Minnich <rminnich@sandia.gov>, Latchesar Ionkov <lucho@ionkov.net>,
        v9fs-developer@lists.sourceforge.net
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Message-ID: <b1d7aba4-0256-f02a-1b88-04d1347bc755@oracle.com>
Date: Wed, 8 Mar 2017 09:33:22 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.6.0
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.10.1703071658480.8160@sstabellini-ThinkPad-X260>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1678
Lines: 55


>
>>> +
>>> +		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