[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 02/13] xen/pvcalls: connect to the backend
>>> 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'. -boris > > >>> + return ret; >>> } >>> _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |