Messages in this thread | | | Subject | Re: [PATCH 06/18] xen/pvcalls: handle commands from the frontend | From | Boris Ostrovsky <> | Date | Mon, 15 May 2017 22:06:07 -0400 |
| |
On 05/15/2017 04:35 PM, Stefano Stabellini wrote: > When the other end notifies us that there are commands to be read > (pvcalls_back_event), wake up the backend thread to parse the command. > > The command ring works like most other Xen rings, so use the usual > ring macros to read and write to it. The functions implementing the > commands are empty stubs for now. > > Signed-off-by: Stefano Stabellini <stefano@aporeto.com> > CC: boris.ostrovsky@oracle.com > CC: jgross@suse.com > --- > drivers/xen/pvcalls-back.c | 115 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 115 insertions(+) > > diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c > index 876e577..2b2a49a 100644 > --- a/drivers/xen/pvcalls-back.c > +++ b/drivers/xen/pvcalls-back.c > @@ -62,12 +62,127 @@ static void pvcalls_back_ioworker(struct work_struct *work) > { > } > > +static int pvcalls_back_socket(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_connect(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_release(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_bind(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_listen(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_accept(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_poll(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + return 0; > +} > + > +static int pvcalls_back_handle_cmd(struct xenbus_device *dev, > + struct xen_pvcalls_request *req) > +{ > + int ret = 0; > + > + switch (req->cmd) { > + case PVCALLS_SOCKET: > + ret = pvcalls_back_socket(dev, req); > + break; > + case PVCALLS_CONNECT: > + ret = pvcalls_back_connect(dev, req); > + break; > + case PVCALLS_RELEASE: > + ret = pvcalls_back_release(dev, req); > + break; > + case PVCALLS_BIND: > + ret = pvcalls_back_bind(dev, req); > + break; > + case PVCALLS_LISTEN: > + ret = pvcalls_back_listen(dev, req); > + break; > + case PVCALLS_ACCEPT: > + ret = pvcalls_back_accept(dev, req); > + break; > + case PVCALLS_POLL: > + ret = pvcalls_back_poll(dev, req); > + break; > + default: > + ret = -ENOTSUPP; > + break; > + } > + return ret; > +} > + > static void pvcalls_back_work(struct work_struct *work) > { > + struct pvcalls_back_priv *priv = container_of(work, > + struct pvcalls_back_priv, register_work); > + int notify, notify_all = 0, more = 1; > + struct xen_pvcalls_request req; > + struct xenbus_device *dev = priv->dev; > + > + atomic_set(&priv->work, 1); > + > + while (more || !atomic_dec_and_test(&priv->work)) { > + while (RING_HAS_UNCONSUMED_REQUESTS(&priv->ring)) { > + RING_COPY_REQUEST(&priv->ring, > + priv->ring.req_cons++, > + &req); > + > + if (pvcalls_back_handle_cmd(dev, &req) > 0) {
Can you make handlers make "traditional" returns, i.e. <0 on error and 0 on success? Or do you really need to distinguish 0 from >0?
> + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY( > + &priv->ring, notify); > + notify_all += notify; > + } > + } > + > + if (notify_all) > + notify_remote_via_irq(priv->irq); > + > + RING_FINAL_CHECK_FOR_REQUESTS(&priv->ring, more); > + } > } > > static irqreturn_t pvcalls_back_event(int irq, void *dev_id) > { > + struct xenbus_device *dev = dev_id; > + struct pvcalls_back_priv *priv = NULL; > + > + if (dev == NULL) > + return IRQ_HANDLED; > + > + priv = dev_get_drvdata(&dev->dev); > + if (priv == NULL) > + return IRQ_HANDLED;
These two aren't errors?
> + > + atomic_inc(&priv->work);
Is this really needed? We have a new entry on the ring, so the outer loop in pvcalls_back_work() will pick this up (by setting 'more').
-boris
> + queue_work(priv->wq, &priv->register_work); > + > return IRQ_HANDLED; > } > >
| |