[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5/7] xen/9pfs: send requests to the backend
On Wed, 8 Mar 2017, Boris Ostrovsky wrote: > On 03/08/2017 02:33 PM, Stefano Stabellini wrote: > > On Wed, 8 Mar 2017, Boris Ostrovsky wrote: > >>>>> +} > >>>>> + > >>>>> +static int p9_xen_write_todo(struct xen_9pfs_dataring *ring, RING_IDX > >>>>> size) > >>>>> +{ > >>>>> + RING_IDX cons, prod; > >>>>> + > >>>>> + cons = ring->intf->out_cons; > >>>>> + prod = ring->intf->out_prod; > >>>>> + mb(); > >>>>> + > >>>>> + if (XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons, > >>>>> XEN_9PFS_RING_SIZE) >= size) > >>>>> + return 1; > >>>>> + else > >>>>> + return 0; > >>>>> } > >>>>> > >>>>> static int p9_xen_request(struct p9_client *client, struct p9_req_t > >>>>> *p9_req) > >>>>> { > >>>>> + struct xen_9pfs_front_priv *priv = NULL; > >>>>> + RING_IDX cons, prod, masked_cons, masked_prod; > >>>>> + unsigned long flags; > >>>>> + uint32_t size = p9_req->tc->size; > >>>>> + struct xen_9pfs_dataring *ring; > >>>>> + int num; > >>>>> + > >>>>> + list_for_each_entry(priv, &xen_9pfs_devs, list) { > >>>>> + if (priv->client == client) > >>>>> + break; > >>>>> + } > >>>>> + if (priv == NULL || priv->client != client) > >>>>> + return -EINVAL; > >>>>> + > >>>>> + num = p9_req->tc->tag % priv->num_rings; > >>>>> + ring = &priv->rings[num]; > >>>>> + > >>>>> +again: > >>>>> + while (wait_event_interruptible(ring->wq, > >>>>> + p9_xen_write_todo(ring, size) > 0) != > >>>>> 0); > >>>>> + > >>>>> + spin_lock_irqsave(&ring->lock, flags); > >>>>> + cons = ring->intf->out_cons; > >>>>> + prod = ring->intf->out_prod; > >>>>> + mb(); > >>>>> + > >>>>> + if (XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons, > >>>>> XEN_9PFS_RING_SIZE) < size) { > >>>> This looks like p9_xen_write_todo(). > >>> p9_xen_write_todo is just a wrapper around xen_9pfs_queued to provide > >>> a return value that works well with wait_event_interruptible. > >>> > >>> I would prefer not to call p9_xen_write_todo here, because it's simpler > >>> if we don't read prod and cons twice. > >> I was referring to the whole code fragment after spin_lock_irqsave(), > >> not just the last line. Isn't it exactly !p9_xen_write_todo()? > > Yes, it is true they are almost the same. The difference, and the reason > > for p9_xen_write_todo to exist, is that p9_xen_write_todo is called in > > the wait_event_interruptible loop, as such it needs to read prod and > > cons every time. On the other end, here we want to read them once. Does > > it make sense? > > > I am clearly being particularly dense here but what I was thinking was: > > again: > while (wait_event_interruptible(ring->wq, > p9_xen_write_todo(ring, size) > 0) != 0); > > spin_lock_irqsave(&ring->lock, flags); > if (!p9_xen_write_todo(ring, size)) { > spin_unlock_irqrestore(&ring->lock, flags); > goto again; > } > > There is no extra read of prod/cons. Yes, there are: just after this if statement we would have to read them again to calculate masked_prod and masked_cons. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |