Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754164AbdFNArj (ORCPT ); Tue, 13 Jun 2017 20:47:39 -0400 Received: from mail.kernel.org ([198.145.29.99]:32930 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753947AbdFNArh (ORCPT ); Tue, 13 Jun 2017 20:47:37 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 00A49239A8 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=sstabellini@kernel.org Date: Tue, 13 Jun 2017 17:47:31 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Juergen Gross cc: Stefano Stabellini , xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, boris.ostrovsky@oracle.com, Stefano Stabellini Subject: Re: [PATCH v3 12/18] xen/pvcalls: implement poll command In-Reply-To: Message-ID: References: <1496431915-20774-1-git-send-email-sstabellini@kernel.org> <1496431915-20774-12-git-send-email-sstabellini@kernel.org> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4117 Lines: 122 On Tue, 13 Jun 2017, Juergen Gross wrote: > On 02/06/17 21:31, Stefano Stabellini wrote: > > Implement poll on passive sockets by requesting a delayed response with > > mappass->reqcopy, and reply back when there is data on the passive > > socket. > > > > Poll on active socket is unimplemented as by the spec, as the frontend > > should just wait for events and check the indexes on the indexes page. > > > > Only support one outstanding poll (or accept) request for every passive > > socket at any given time. > > > > Signed-off-by: Stefano Stabellini > > CC: boris.ostrovsky@oracle.com > > CC: jgross@suse.com > > --- > > drivers/xen/pvcalls-back.c | 75 ++++++++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 73 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c > > index f1173f4..82f350d 100644 > > --- a/drivers/xen/pvcalls-back.c > > +++ b/drivers/xen/pvcalls-back.c > > @@ -344,11 +344,33 @@ static void __pvcalls_back_accept(struct work_struct *work) > > static void pvcalls_pass_sk_data_ready(struct sock *sock) > > { > > struct sockpass_mapping *mappass = sock->sk_user_data; > > + struct pvcalls_fedata *priv; > > + struct xen_pvcalls_response *rsp; > > + unsigned long flags; > > + int notify; > > > > if (mappass == NULL) > > return; > > > > - queue_work(mappass->wq, &mappass->register_work); > > + priv = mappass->priv; > > + spin_lock_irqsave(&mappass->copy_lock, flags); > > + if (mappass->reqcopy.cmd == PVCALLS_POLL) { > > + rsp = RING_GET_RESPONSE(&priv->ring, priv->ring.rsp_prod_pvt++); > > + rsp->req_id = mappass->reqcopy.req_id; > > + rsp->u.poll.id = mappass->reqcopy.u.poll.id; > > + rsp->cmd = mappass->reqcopy.cmd; > > + rsp->ret = 0; > > + > > + mappass->reqcopy.cmd = 0; > > + spin_unlock_irqrestore(&mappass->copy_lock, flags); > > + > > + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&priv->ring, notify); > > + if (notify) > > + notify_remote_via_irq(mappass->priv->irq); > > + } else { > > + spin_unlock_irqrestore(&mappass->copy_lock, flags); > > + queue_work(mappass->wq, &mappass->register_work); > > + } > > } > > > > static int pvcalls_back_bind(struct xenbus_device *dev, > > @@ -493,7 +515,56 @@ static int pvcalls_back_accept(struct xenbus_device *dev, > > static int pvcalls_back_poll(struct xenbus_device *dev, > > struct xen_pvcalls_request *req) > > { > > - return 0; > > + struct pvcalls_fedata *priv; > > + struct sockpass_mapping *mappass; > > + struct xen_pvcalls_response *rsp; > > + struct inet_connection_sock *icsk; > > + struct request_sock_queue *queue; > > + unsigned long flags; > > + int ret; > > + bool data; > > + > > + priv = dev_get_drvdata(&dev->dev); > > + > > + mappass = radix_tree_lookup(&priv->socketpass_mappings, req->u.poll.id); > > + if (mappass == NULL) > > + return -EINVAL; > > + > > + /* > > + * Limitation of the current implementation: only support one > > + * concurrent accept or poll call on one socket. > > + */ > > + spin_lock_irqsave(&mappass->copy_lock, flags); > > + if (mappass->reqcopy.cmd != 0) { > > + ret = -EINTR; > > + goto out; > > + } > > + > > + mappass->reqcopy = *req; > > + icsk = inet_csk(mappass->sock->sk); > > + queue = &icsk->icsk_accept_queue; > > + spin_lock(&queue->rskq_lock); > > + data = queue->rskq_accept_head != NULL; > > + spin_unlock(&queue->rskq_lock); > > + if (data) { > > + mappass->reqcopy.cmd = 0; > > + ret = 0; > > + goto out; > > + } > > + spin_unlock_irqrestore(&mappass->copy_lock, flags); I'll also need to change these mappass->reqcopy.cmd accesses to ACCESS_ONCE to be consistent with the changes to the previous patch (need to become atomic) > > + /* Tell the caller we don't need to send back a notification yet */ > > + return -1; > > + > > +out: > > + spin_unlock_irqrestore(&mappass->copy_lock, flags); > > + > > + rsp = RING_GET_RESPONSE(&priv->ring, priv->ring.rsp_prod_pvt++); > > + rsp->req_id = req->req_id; > > + rsp->cmd = req->cmd; > > + rsp->u.poll.id = req->u.poll.id; > > + rsp->ret = ret; > > + return ret; > > return 0; Yes