lkml.org 
[lkml]   [2017]   [Mar]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 5/7] xen/9pfs: send requests to the backend
From
Date
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.

-boris

\
 
 \ /
  Last update: 2017-03-08 21:06    [W:0.106 / U:0.484 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site