Re: [Xen-devel] [PATCH v5 11/13] xen/pvcalls: implement poll command

[Boris Ostrovsky]

On 10/23/2017 07:06 PM, Stefano Stabellini wrote:
> On Tue, 17 Oct 2017, Boris Ostrovsky wrote:
>>> +static unsigned int pvcalls_front_poll_passive(struct file *file,
>>> +  struct pvcalls_bedata *bedata,
>>> +  struct sock_mapping *map,
>>> +  poll_table *wait)
>>> +{
>>> +   int notify, req_id, ret;
>>> +   struct xen_pvcalls_request *req;
>>> +
>>> +   if (test_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
>>> +(void *)>passive.flags)) {
>>> +   uint32_t req_id = READ_ONCE(map->passive.inflight_req_id);
>>> +
>>> +   if (req_id != PVCALLS_INVALID_ID &&
>>> +   READ_ONCE(bedata->rsp[req_id].req_id) == req_id)
>>> +   return POLLIN | POLLRDNORM;
>>
>> Same READ_ONCE() question as for an earlier patch.
> Same answer :-)


Reviewed-by: Boris Ostrovsky 

>
>
>>> +
>>> +   poll_wait(file, >passive.inflight_accept_req, wait);
>>> +   return 0;
>>> +   }
>>> +


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

Re: [Xen-devel] [PATCH v5 11/13] xen/pvcalls: implement poll command

[Stefano Stabellini]

On Tue, 17 Oct 2017, Boris Ostrovsky wrote:
> > +static unsigned int pvcalls_front_poll_passive(struct file *file,
> > +  struct pvcalls_bedata *bedata,
> > +  struct sock_mapping *map,
> > +  poll_table *wait)
> > +{
> > +   int notify, req_id, ret;
> > +   struct xen_pvcalls_request *req;
> > +
> > +   if (test_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
> > +(void *)>passive.flags)) {
> > +   uint32_t req_id = READ_ONCE(map->passive.inflight_req_id);
> > +
> > +   if (req_id != PVCALLS_INVALID_ID &&
> > +   READ_ONCE(bedata->rsp[req_id].req_id) == req_id)
> > +   return POLLIN | POLLRDNORM;
> 
> 
> Same READ_ONCE() question as for an earlier patch.

Same answer :-)


> > +
> > +   poll_wait(file, >passive.inflight_accept_req, wait);
> > +   return 0;
> > +   }
> > +
> 

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

Re: [Xen-devel] [PATCH v5 11/13] xen/pvcalls: implement poll command

[Boris Ostrovsky]

>  
> +static unsigned int pvcalls_front_poll_passive(struct file *file,
> +struct pvcalls_bedata *bedata,
> +struct sock_mapping *map,
> +poll_table *wait)
> +{
> + int notify, req_id, ret;
> + struct xen_pvcalls_request *req;
> +
> + if (test_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
> +  (void *)>passive.flags)) {
> + uint32_t req_id = READ_ONCE(map->passive.inflight_req_id);
> +
> + if (req_id != PVCALLS_INVALID_ID &&
> + READ_ONCE(bedata->rsp[req_id].req_id) == req_id)
> + return POLLIN | POLLRDNORM;


Same READ_ONCE() question as for an earlier patch.

-boris

> +
> + poll_wait(file, >passive.inflight_accept_req, wait);
> + return 0;
> + }
> +


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

[Xen-devel] [PATCH v5 11/13] xen/pvcalls: implement poll command

[Stefano Stabellini]

For active sockets, check the indexes and use the inflight_conn_req
waitqueue to wait.

For passive sockets if an accept is outstanding
(PVCALLS_FLAG_ACCEPT_INFLIGHT), check if it has been answered by looking
at bedata->rsp[req_id]. If so, return POLLIN.  Otherwise use the
inflight_accept_req waitqueue.

If no accepts are inflight, send PVCALLS_POLL to the backend. If we have
outstanding POLL requests awaiting for a response use the inflight_req
waitqueue: inflight_req is awaken when a new response is received; on
wakeup we check whether the POLL response is arrived by looking at the
PVCALLS_FLAG_POLL_RET flag. We set the flag from
pvcalls_front_event_handler, if the response was for a POLL command.

In pvcalls_front_event_handler, get the struct sock_mapping from the
poll id (we previously converted struct sock_mapping* to uint64_t and
used it as id).

Signed-off-by: Stefano Stabellini 
CC: boris.ostrov...@oracle.com
CC: jgr...@suse.com
---
 drivers/xen/pvcalls-front.c | 144 +---
 drivers/xen/pvcalls-front.h |   3 +
 2 files changed, 138 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 161f88b..aca2b32 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -84,6 +84,8 @@ struct sock_mapping {
 * Only one poll operation can be inflight for a given socket.
 */
 #define PVCALLS_FLAG_ACCEPT_INFLIGHT 0
+#define PVCALLS_FLAG_POLL_INFLIGHT   1
+#define PVCALLS_FLAG_POLL_RET2
uint8_t flags;
uint32_t inflight_req_id;
struct sock_mapping *accept_map;
@@ -155,15 +157,32 @@ static irqreturn_t pvcalls_front_event_handler(int irq, 
void *dev_id)
rsp = RING_GET_RESPONSE(>ring, bedata->ring.rsp_cons);
 
req_id = rsp->req_id;
-   dst = (uint8_t *)>rsp[req_id] + sizeof(rsp->req_id);
-   src = (uint8_t *)rsp + sizeof(rsp->req_id);
-   memcpy(dst, src, sizeof(*rsp) - sizeof(rsp->req_id));
-   /*
-* First copy the rest of the data, then req_id. It is
-* paired with the barrier when accessing bedata->rsp.
-*/
-   smp_wmb();
-   bedata->rsp[req_id].req_id = rsp->req_id;
+   if (rsp->cmd == PVCALLS_POLL) {
+   struct sock_mapping *map = (struct sock_mapping *)
+  rsp->u.poll.id;
+
+   clear_bit(PVCALLS_FLAG_POLL_INFLIGHT,
+ (void *)>passive.flags);
+   /*
+* clear INFLIGHT, then set RET. It pairs with
+* the checks at the beginning of
+* pvcalls_front_poll_passive.
+*/
+   smp_wmb();
+   set_bit(PVCALLS_FLAG_POLL_RET,
+   (void *)>passive.flags);
+   } else {
+   dst = (uint8_t *)>rsp[req_id] +
+ sizeof(rsp->req_id);
+   src = (uint8_t *)rsp + sizeof(rsp->req_id);
+   memcpy(dst, src, sizeof(*rsp) - sizeof(rsp->req_id));
+   /*
+* First copy the rest of the data, then req_id. It is
+* paired with the barrier when accessing bedata->rsp.
+*/
+   smp_wmb();
+   bedata->rsp[req_id].req_id = req_id;
+   }
 
done = 1;
bedata->ring.rsp_cons++;
@@ -842,6 +861,113 @@ int pvcalls_front_accept(struct socket *sock, struct 
socket *newsock, int flags)
return ret;
 }
 
+static unsigned int pvcalls_front_poll_passive(struct file *file,
+  struct pvcalls_bedata *bedata,
+  struct sock_mapping *map,
+  poll_table *wait)
+{
+   int notify, req_id, ret;
+   struct xen_pvcalls_request *req;
+
+   if (test_bit(PVCALLS_FLAG_ACCEPT_INFLIGHT,
+(void *)>passive.flags)) {
+   uint32_t req_id = READ_ONCE(map->passive.inflight_req_id);
+
+   if (req_id != PVCALLS_INVALID_ID &&
+   READ_ONCE(bedata->rsp[req_id].req_id) == req_id)
+   return POLLIN | POLLRDNORM;
+
+   poll_wait(file, >passive.inflight_accept_req, wait);
+   return 0;
+   }
+
+   if (test_and_clear_bit(PVCALLS_FLAG_POLL_RET,
+  (void *)>passive.flags))
+   return POLLIN | POLLRDNORM;
+
+   /*
+* First check RET, then INFLIGHT. No barriers necessary to
+* ensure execution ordering because of the