Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1700420
| From | Stefano Stabellini <sstabellini@kernel.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2 02/13] xen/pvcalls: connect to the backend |
| Date | 2017-08-01 00:00 +0200 |
| Message-ID | <u9qTg-35o-23@gated-at.bofh.it> (permalink) |
| References | (1 earlier) <u7fyV-8aW-5@gated-at.bofh.it> <u7fyY-8aW-71@gated-at.bofh.it> <u7uHF-SQ-57@gated-at.bofh.it> <u7EQF-7nN-1@gated-at.bofh.it> <u7SAh-7F9-11@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Thu, 27 Jul 2017, Boris Ostrovsky wrote:
> >>> static int pvcalls_front_probe(struct xenbus_device *dev,
> >>> const struct xenbus_device_id *id)
> >>> {
> >>> + int ret = -EFAULT, evtchn, ref = -1, i;
> >>> + unsigned int max_page_order, function_calls, len;
> >>> + char *versions;
> >>> + grant_ref_t gref_head = 0;
> >>> + struct xenbus_transaction xbt;
> >>> + struct pvcalls_bedata *bedata = NULL;
> >>> + struct xen_pvcalls_sring *sring;
> >>> +
> >>> + if (pvcalls_front_dev != NULL) {
> >>> + dev_err(&dev->dev, "only one PV Calls connection
> >>> supported\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + versions = xenbus_read(XBT_NIL, dev->otherend, "versions", &len);
> >>> + if (!len)
> >>> + return -EINVAL;
> >>> + if (strcmp(versions, "1")) {
> >>> + kfree(versions);
> >>> + return -EINVAL;
> >>> + }
> >>> + kfree(versions);
> >>> + ret = xenbus_scanf(XBT_NIL, dev->otherend,
> >>> + "max-page-order", "%u", &max_page_order);
> >>> + if (ret <= 0)
> >>> + return -ENODEV;
> >>> + if (max_page_order < RING_ORDER)
> >>> + return -ENODEV;
> >>> + ret = xenbus_scanf(XBT_NIL, dev->otherend,
> >>> + "function-calls", "%u", &function_calls);
> >>> + if (ret <= 0 || function_calls != 1)
> >>> + return -ENODEV;
> >>> + pr_info("%s max-page-order is %u\n", __func__, max_page_order);
> >>> +
> >>> + bedata = kzalloc(sizeof(struct pvcalls_bedata), GFP_KERNEL);
> >>> + if (!bedata)
> >>> + return -ENOMEM;
> >>> +
> >>> + init_waitqueue_head(&bedata->inflight_req);
> >>> + for (i = 0; i < PVCALLS_NR_REQ_PER_RING; i++)
> >>> + bedata->rsp[i].req_id = PVCALLS_INVALID_ID;
> >>> +
> >>> + sring = (struct xen_pvcalls_sring *) __get_free_page(GFP_KERNEL |
> >>> + __GFP_ZERO);
> >>> + if (!sring)
> >>> + goto error;
> >>> + SHARED_RING_INIT(sring);
> >>> + FRONT_RING_INIT(&bedata->ring, sring, XEN_PAGE_SIZE);
> >>> +
> >>> + ret = xenbus_alloc_evtchn(dev, &evtchn);
> >>> + if (ret)
> >>> + goto error;
> >>> +
> >>> + bedata->irq = bind_evtchn_to_irqhandler(evtchn,
> >>> + pvcalls_front_event_handler,
> >>> + 0, "pvcalls-frontend", dev);
> >>> + if (bedata->irq < 0) {
> >>> + ret = bedata->irq;
> >>> + goto error;
> >>> + }
> >>> +
> >>> + ret = gnttab_alloc_grant_references(1, &gref_head);
> >>> + if (ret < 0)
> >>> + goto error;
> >>> + bedata->ref = ref = gnttab_claim_grant_reference(&gref_head);
> >> Is ref really needed?
> > No, I'll remove it
> >
> >
> >>> + if (ref < 0)
> >>> + goto error;
> >>> + gnttab_grant_foreign_access_ref(ref, dev->otherend_id,
> >>> + virt_to_gfn((void *)sring), 0);
> >>> +
> >>> + again:
> >>> + ret = xenbus_transaction_start(&xbt);
> >>> + if (ret) {
> >>> + xenbus_dev_fatal(dev, ret, "starting transaction");
> >>> + goto error;
> >>> + }
> >>> + ret = xenbus_printf(xbt, dev->nodename, "version", "%u", 1);
> >>> + if (ret)
> >>> + goto error_xenbus;
> >>> + ret = xenbus_printf(xbt, dev->nodename, "ring-ref", "%d", ref);
> >>> + if (ret)
> >>> + goto error_xenbus;
> >>> + ret = xenbus_printf(xbt, dev->nodename, "port", "%u",
> >>> + evtchn);
> >>> + if (ret)
> >>> + goto error_xenbus;
> >>> + ret = xenbus_transaction_end(xbt, 0);
> >>> + if (ret) {
> >>> + if (ret == -EAGAIN)
> >>> + goto again;
> >>> + xenbus_dev_fatal(dev, ret, "completing transaction");
> >>> + goto error;
> >>> + }
> >>> +
> >>> + INIT_LIST_HEAD(&bedata->socket_mappings);
> >>> + INIT_LIST_HEAD(&bedata->socketpass_mappings);
> >>> + spin_lock_init(&bedata->pvcallss_lock);
> >>> + dev_set_drvdata(&dev->dev, bedata);
> >>> + pvcalls_front_dev = dev;
> >>> + xenbus_switch_state(dev, XenbusStateInitialised);
> >>> +
> >>> return 0;
> >>> +
> >>> + error_xenbus:
> >>> + xenbus_transaction_end(xbt, 1);
> >>> + xenbus_dev_fatal(dev, ret, "writing xenstore");
> >>> + error:
> >>> + pvcalls_front_remove(dev);
> >> I think patch 12 (where you implement cleanup) could be moved before this one.
> > I'll move the patch
> >
> >
> >> I also think you are leaking bedata on error paths.
> > bedata is freed by pvcalls_front_remove (kfree(bedata)), why do you say
> > so?
>
> bedata there is read from dev_get_drvdata() and here you assign drvdata
> at the very end.
>
> Come think of it, pvcalls_front_remove() should probably first check
> whether bedata is valid. Or drvdata should be assigned right away in
> this routine, before any 'got error/error_xenbus'.
Yes, I'll do that
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v2 00/13] introduce the Xen PV Calls frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
[PATCH v2 13/13] xen: introduce a Kconfig option to enable the pvcalls frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
[PATCH v2 10/13] xen/pvcalls: implement poll command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 10/13] xen/pvcalls: implement poll command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-27 01:30 +0200
Re: [PATCH v2 10/13] xen/pvcalls: implement poll command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 02:30 +0200
[PATCH v2 06/13] xen/pvcalls: implement listen command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
[PATCH v2 07/13] xen/pvcalls: implement accept command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 07/13] xen/pvcalls: implement accept command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 20:00 +0200
Re: [PATCH v2 07/13] xen/pvcalls: implement accept command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 01:30 +0200
[PATCH v2 01/13] xen/pvcalls: introduce the pvcalls xenbus frontend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
[PATCH v2 12/13] xen/pvcalls: implement frontend disconnect Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
[PATCH v2 05/13] xen/pvcalls: implement bind command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 05/13] xen/pvcalls: implement bind command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 17:00 +0200
Re: [PATCH v2 05/13] xen/pvcalls: implement bind command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 02:00 +0200
Re: [PATCH v2 05/13] xen/pvcalls: implement bind command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-27 16:50 +0200
Re: [PATCH v2 05/13] xen/pvcalls: implement bind command Stefano Stabellini <sstabellini@kernel.org> - 2017-08-01 00:20 +0200
[PATCH v2 04/13] xen/pvcalls: implement connect command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 04/13] xen/pvcalls: implement connect command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 17:00 +0200
Re: [PATCH v2 04/13] xen/pvcalls: implement connect command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 01:30 +0200
[PATCH v2 09/13] xen/pvcalls: implement recvmsg Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 09/13] xen/pvcalls: implement recvmsg Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 23:30 +0200
Re: [Xen-devel] [PATCH v2 09/13] xen/pvcalls: implement recvmsg Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 23:40 +0200
Re: [Xen-devel] [PATCH v2 09/13] xen/pvcalls: implement recvmsg Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 02:10 +0200
Re: [Xen-devel] [PATCH v2 09/13] xen/pvcalls: implement recvmsg Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-27 17:00 +0200
Re: [Xen-devel] [PATCH v2 09/13] xen/pvcalls: implement recvmsg Stefano Stabellini <sstabellini@kernel.org> - 2017-08-01 00:30 +0200
[PATCH v2 11/13] xen/pvcalls: implement release command Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 11/13] xen/pvcalls: implement release command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-27 20:40 +0200
Re: [PATCH v2 11/13] xen/pvcalls: implement release command Stefano Stabellini <sstabellini@kernel.org> - 2017-08-01 00:40 +0200
Re: [PATCH v2 11/13] xen/pvcalls: implement release command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-08-01 17:30 +0200
Re: [PATCH v2 11/13] xen/pvcalls: implement release command Juergen Gross <jgross@suse.com> - 2017-08-01 17:40 +0200
Re: [PATCH v2 11/13] xen/pvcalls: implement release command Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-08-01 18:10 +0200
[PATCH v2 02/13] xen/pvcalls: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 02/13] xen/pvcalls: connect to the backend Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 15:40 +0200
Re: [PATCH v2 02/13] xen/pvcalls: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 02:30 +0200
Re: [PATCH v2 02/13] xen/pvcalls: connect to the backend Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-27 17:10 +0200
Re: [PATCH v2 02/13] xen/pvcalls: connect to the backend Stefano Stabellini <sstabellini@kernel.org> - 2017-08-01 00:00 +0200
[PATCH v2 03/13] xen/pvcalls: implement socket command and handle events Stefano Stabellini <sstabellini@kernel.org> - 2017-07-25 23:30 +0200
Re: [PATCH v2 03/13] xen/pvcalls: implement socket command and handle events Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-07-26 16:30 +0200
Re: [PATCH v2 03/13] xen/pvcalls: implement socket command and handle events Stefano Stabellini <sstabellini@kernel.org> - 2017-07-27 01:20 +0200
csiph-web