From owner-freebsd-xen@freebsd.org Thu Jan 5 21:52:51 2017 Return-Path: Delivered-To: freebsd-xen@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B4D4FCA17EC; Thu, 5 Jan 2017 21:52:51 +0000 (UTC) (envelope-from prvs=117803d5e3=jwest@ezwind.net) Received: from ezwind.net (booby.ezwind.net [199.188.211.150]) by mx1.freebsd.org (Postfix) with ESMTP id 7D1CC111D; Thu, 5 Jan 2017 21:52:51 +0000 (UTC) (envelope-from prvs=117803d5e3=jwest@ezwind.net) X-MDAV-Result: clean X-MDAV-Processed: ezwind.net, Thu, 05 Jan 2017 15:52:06 -0600 X-Spam-Processed: ezwind.net, Thu, 05 Jan 2017 15:52:05 -0600 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on Booby X-Spam-Level: X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,NO_RELAYS shortcircuit=no autolearn=unavailable autolearn_force=no version=3.4.1 Received: from OwnerTHINK by ezwind.net (MDaemon PRO v16.0.4) with ESMTPA id 19-md50000016524.msg; Thu, 05 Jan 2017 15:52:04 -0600 X-MDRemoteIP: 68.184.8.34 X-MDHelo: OwnerTHINK X-MDArrival-Date: Thu, 05 Jan 2017 15:52:04 -0600 X-Authenticated-Sender: jwest@ezwind.net X-Return-Path: prvs=117803d5e3=jwest@ezwind.net X-Envelope-From: jwest@ezwind.net From: "Jay West" To: "'Hoyer-Reuther, Christian'" , =?iso-8859-1?Q?'Roger_Pau_Monn=E9'?= Cc: , References: <001f01d250a6$05ca4160$115ec420$@ezwind.net> <003501d250af$032dd580$09898080$@ezwind.net> <41E487BC91654544B2B8F31096F2D9D4D1B060A925@ex1> <20161212103148.5prkggi3ix4zoxyv@dhcp-3-221.uk.xensource.com> <41E487BC91654544B2B8F31096F2D9D4D1B060AA3E@ex1> <002101d25484$c0a068c0$41e13a40$@ezwind.net> <20161214140224.rbg5h3v2opa3q3sj@dhcp-3-221.uk.xensource.com> <41E487BC91654544B2B8F31096F2D9D4D1B060ABBF@ex1> In-Reply-To: <41E487BC91654544B2B8F31096F2D9D4D1B060ABBF@ex1> Subject: RE: 11-RELEASE and live migration Date: Thu, 5 Jan 2017 15:52:13 -0600 Message-ID: <001701d2679d$f99eb560$ecdc2020$@ezwind.net> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 15.0 Thread-Index: AQH1zUDfKZwhlQ3oullSr3GwEGJmYQG1SrqiAXap4pICEW0WjwI3N6W4AtIBtTcCsekXcgJTOwrVAZU4RUGgXJlNwA== Content-Language: en-us X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jan 2017 21:52:51 -0000 Roger wrote... > I'm able to see these glitches, as said the VM doesn't really hang, it > just gets stuck for a long time (and I think that depends on the > uptime differences between the source and destination hosts). In the > meantime, could you please test if changing the timecounter from > XENTIMER to any other (like HPET or > ACPI-fast) solves the issue? > > # sysctl -w kern.timecounter.hardware=ACPI-fast Christian replied... I changed the timecounter from XENTIMER to ACPI-fast and then I did about 10 migrations between our 3 hosts. I always started the next migration a few seconds after the previous migration finished. The VM didn't stuck now and I didn't see the VGABios screen on the console. I can also add to this... changing timecounter to ACPI-fast definitely fixes the problem. I can migrate a handful of freebsd11 vm's back and forth and all stay running correctly. So... bug in XENTIMER? From owner-freebsd-xen@freebsd.org Fri Jan 6 11:20:24 2017 Return-Path: Delivered-To: freebsd-xen@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5AFE3CA2355; Fri, 6 Jan 2017 11:20:24 +0000 (UTC) (envelope-from prvs=172991b13=roger.pau@citrix.com) Received: from SMTP.EU.CITRIX.COM (smtp.ctxuk.citrix.com [185.25.65.24]) (using TLSv1.2 with cipher RC4-SHA (128/128 bits)) (Client CN "mail.citrix.com", Issuer "DigiCert SHA2 Secure Server CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id AC6C51947; Fri, 6 Jan 2017 11:20:23 +0000 (UTC) (envelope-from prvs=172991b13=roger.pau@citrix.com) X-IronPort-AV: E=Sophos;i="5.33,324,1477958400"; d="scan'208";a="38017595" Date: Fri, 6 Jan 2017 11:18:46 +0000 From: 'Roger Pau =?iso-8859-1?Q?Monn=E9'?= To: Jay West CC: "'Hoyer-Reuther, Christian'" , , Subject: Re: 11-RELEASE and live migration Message-ID: <20170106111846.qnvrq6vwvdi3245y@dhcp-3-221.uk.xensource.com> References: <001f01d250a6$05ca4160$115ec420$@ezwind.net> <003501d250af$032dd580$09898080$@ezwind.net> <41E487BC91654544B2B8F31096F2D9D4D1B060A925@ex1> <20161212103148.5prkggi3ix4zoxyv@dhcp-3-221.uk.xensource.com> <41E487BC91654544B2B8F31096F2D9D4D1B060AA3E@ex1> <002101d25484$c0a068c0$41e13a40$@ezwind.net> <20161214140224.rbg5h3v2opa3q3sj@dhcp-3-221.uk.xensource.com> <41E487BC91654544B2B8F31096F2D9D4D1B060ABBF@ex1> <001701d2679d$f99eb560$ecdc2020$@ezwind.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <001701d2679d$f99eb560$ecdc2020$@ezwind.net> User-Agent: NeoMutt/20161126 (1.7.1) X-ClientProxiedBy: AMSPEX02CAS01.citrite.net (10.69.22.112) To AMSPEX02CL02.citrite.net (10.69.22.126) X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Jan 2017 11:20:24 -0000 On Thu, Jan 05, 2017 at 03:52:13PM -0600, Jay West wrote: > > Roger wrote... > > I'm able to see these glitches, as said the VM doesn't really hang, it > > just gets stuck for a long time (and I think that depends on the > > uptime differences between the source and destination hosts). In the > > meantime, could you please test if changing the timecounter from > > XENTIMER to any other (like HPET or > > ACPI-fast) solves the issue? > > > > # sysctl -w kern.timecounter.hardware=ACPI-fast > > Christian replied... > I changed the timecounter from XENTIMER to ACPI-fast and then I did about 10 > migrations between our 3 hosts. I always started the next migration a few > seconds after the previous migration finished. The VM didn't stuck now and I > didn't see the VGABios screen on the console. > > I can also add to this... changing timecounter to ACPI-fast definitely fixes > the problem. I can migrate a handful of freebsd11 vm's back and forth and > all stay running correctly. > > So... bug in XENTIMER? Not exactly, let's say it's a shortcoming of the current resume procedure. IF you want to know more about the gory details: https://lists.freebsd.org/pipermail/freebsd-arch/2016-December/018041.html I'm currently quite busy, so I don't really have an ETA of when this is going to be fixed. Roger. From owner-freebsd-xen@freebsd.org Fri Jan 6 12:44:38 2017 Return-Path: Delivered-To: freebsd-xen@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 71BDDB88C75 for ; Fri, 6 Jan 2017 12:44:38 +0000 (UTC) (envelope-from prvs=172991b13=roger.pau@citrix.com) Received: from SMTP.EU.CITRIX.COM (smtp.ctxuk.citrix.com [185.25.65.24]) (using TLSv1.2 with cipher RC4-SHA (128/128 bits)) (Client CN "mail.citrix.com", Issuer "DigiCert SHA2 Secure Server CA" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id ABF2614E0 for ; Fri, 6 Jan 2017 12:44:37 +0000 (UTC) (envelope-from prvs=172991b13=roger.pau@citrix.com) X-IronPort-AV: E=Sophos;i="5.33,324,1477958400"; d="scan'208";a="38020886" Date: Fri, 6 Jan 2017 12:44:25 +0000 From: freebsd xen To: Liuyingdong CC: "freebsd-xen@freebsd.org" , "Zhaojun (Euler)" , Suoben , "Ouyangzhaowei (Charles)" , chuzhaosong , Wanglinkai Subject: Re: [PATCH]netfront: need release all resources after adding and removing NICs time and again Message-ID: <20170106124425.ofqrvfzkzmyybkcn@dhcp-3-221.uk.xensource.com> References: <5825272C.3010704@huawei.com> <584F8F1C.6030006@huawei.com> <20161213142919.ulvvvwoz2da5fedw@dhcp-3-221.uk.xensource.com> <3655E9A8B903724782E1F75DCFD74E6B01229F6F11@szxema506-mbs.china.huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <3655E9A8B903724782E1F75DCFD74E6B01229F6F11@szxema506-mbs.china.huawei.com> User-Agent: NeoMutt/20161126 (1.7.1) X-ClientProxiedBy: AMSPEX02CAS01.citrite.net (10.69.22.112) To AMSPEX02CL02.citrite.net (10.69.22.126) X-BeenThere: freebsd-xen@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Discussion of the freebsd port to xen - implementation and usage List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Jan 2017 12:44:38 -0000 On Thu, Dec 15, 2016 at 02:00:53AM +0000, Liuyingdong wrote: > Hello Roger, > Thank you for the time and patience you devoted to reading my messages and e-mails. I really appreciate that. > I can't use git send-email so I attach the patches directly. In the 0001 patch I introduce suspend_cancel mechanism for frontend devices and in the 0002 patch I release all resources after hot unplug NICs. > > Note: These two patches is on the base of the origin/release/10.2.0 branch and the 0002 patch is made after the 0001 patch. Hello, Thanks for the patches, now they look fine. You will however need to submit those against current HEAD (origin/master), and then I will do the backport to stable/10 and stable/11. I cannot apply a patch directly to stable branches without it being in HEAD first unless there's a good reason for it. Replying to both patches here inline. ---8<--- > From fc85ac7eba55a5f14f5f7d81f0e1fc7fbf6a7447 Mon Sep 17 00:00:00 2001 > From: Yingdong Liu > Date: Tue, 13 Dec 2016 21:53:25 +0800 > Subject: [PATCH] introduce suspend cancel mechanism for frontend devices > Can you elaborate a little bit more on the commit message here? What issues are you seeing without this patch applied? What is the result after applying the patch? Commit messages are very important in order to know why a change is needed, specially when you look at them in for example 3 years time. > --- > sys/dev/xen/blkfront/blkfront.c | 13 +++++++++++ > sys/dev/xen/control/control.c | 9 ++++++- > sys/dev/xen/netfront/netfront.c | 52 ++++++++++++++++++++++++++++++----------- > sys/x86/xen/hvm.c | 16 ++++++++----- > sys/xen/xenbus/xenbusb.c | 45 ++++++++++++++++++++++------------- > sys/xen/xenbus/xenbusvar.h | 4 ++++ > 6 files changed, 102 insertions(+), 37 deletions(-) > > diff --git a/sys/dev/xen/blkfront/blkfront.c b/sys/dev/xen/blkfront/blkfront.c > index a71251d..8d7c32a 100644 > --- a/sys/dev/xen/blkfront/blkfront.c > +++ b/sys/dev/xen/blkfront/blkfront.c > @@ -68,6 +68,8 @@ __FBSDID("$FreeBSD$"); > > #include "xenbus_if.h" > > +static int blkfront_suspend_cancelled = 0; I see that you add one of those to each frontend you add cancel suspend support, together with a *_set_suspend_cancel function. Wouldn't it be easier to declare a global suspend_cancelled variable in xen/control.c and export that to all the frontends that need it? It should also be a bool type instead of int. You can clear that variable after DEVICE_RESUME has finished. > /*--------------------------- Forward Declarations ---------------------------*/ > static void xbd_closing(device_t); > static void xbd_startio(struct xbd_softc *sc); > @@ -1417,10 +1419,21 @@ xbd_suspend(device_t dev) > return (retval); > } > > +void xbd_set_suspend_cancel(void) > +{ > + blkfront_suspend_cancelled = 1; > +} > + > static int > xbd_resume(device_t dev) > { > struct xbd_softc *sc = device_get_softc(dev); > + Stray tab. > + if(blkfront_suspend_cancelled == 1) { ^ missing space between "if" and "(". > + sc->xbd_state = XBD_STATE_CONNECTED; > + blkfront_suspend_cancelled = 0; > + return (0); > + } > > DPRINTK("xbd_resume: %s\n", xenbus_get_node(dev)); > > diff --git a/sys/dev/xen/control/control.c b/sys/dev/xen/control/control.c > index bc0609d..b500100 100644 > --- a/sys/dev/xen/control/control.c > +++ b/sys/dev/xen/control/control.c > @@ -400,7 +400,9 @@ xctrl_suspend() > /* > * Reset grant table info. > */ > - gnttab_resume(); > + if(suspend_cancelled == 0) { ^ missing space. > + gnttab_resume(); > + } > > #ifdef SMP > if (smp_started && !CPU_EMPTY(&cpu_suspend_map)) { > @@ -416,6 +418,11 @@ xctrl_suspend() > * FreeBSD really needs to add DEVICE_SUSPEND_CANCEL or > * similar. > */ > + if(suspend_cancelled == 1) { > + xenbusb_set_suspend_cancel(); > + xbd_set_suspend_cancel(); > + xn_set_suspend_cancel(); > + } If you make suspend_cancelled global you wouldn't need to call all of those functions. > mtx_lock(&Giant); > DEVICE_RESUME(root_bus); > mtx_unlock(&Giant); > diff --git a/sys/dev/xen/netfront/netfront.c b/sys/dev/xen/netfront/netfront.c > index d89c0e0..0caaa2c 100644 > --- a/sys/dev/xen/netfront/netfront.c > +++ b/sys/dev/xen/netfront/netfront.c > @@ -98,6 +98,8 @@ __FBSDID("$FreeBSD$"); > #define NET_TX_RING_SIZE __RING_SIZE((netif_tx_sring_t *)0, PAGE_SIZE) > #define NET_RX_RING_SIZE __RING_SIZE((netif_rx_sring_t *)0, PAGE_SIZE) > > +static int netfront_suspend_cancelled = 0; > + > #if __FreeBSD_version >= 700000 > /* > * Should the driver do LRO on the RX end > @@ -192,6 +194,10 @@ static int xennet_get_responses(struct netfront_info *np, > struct netfront_rx_info *rinfo, RING_IDX rp, RING_IDX *cons, > struct mbuf **list, int *pages_flipped_p); > > +#ifdef INET > +static void netfront_send_fake_arp(device_t dev, struct netfront_info *info); > +#endif > + > #define virt_to_mfn(x) (vtomach(x) >> PAGE_SHIFT) > > #define INVALID_P2M_ENTRY (~0UL) > @@ -493,6 +499,11 @@ netfront_suspend(device_t dev) > return (0); > } > > +void xn_set_suspend_cancel(void) > +{ > + netfront_suspend_cancelled = 1; > +} > + > /** > * We are reconnecting to the backend, due to a suspend/resume, or a backend > * driver restart. We tear down our netif structure and recreate it, but > @@ -504,6 +515,19 @@ netfront_resume(device_t dev) > { > struct netfront_info *info = device_get_softc(dev); > > + if(netfront_suspend_cancelled == 1) { > + info->xn_resume = true; > + XN_RX_LOCK(info); > + XN_TX_LOCK(info); > + netfront_carrier_on(info); > + XN_TX_UNLOCK(info); > + XN_RX_UNLOCK(info); > +#ifdef INET > + netfront_send_fake_arp(dev, info); > +#endif I don't think you need to send an ARP on a cancelled suspend, the bridge should already have the mac address of the virtual interface in it's cache right? > + netfront_suspend_cancelled = 0; > + return (0); > + } > info->xn_resume = true; > netif_disconnect_backend(info); > return (0); > @@ -2108,26 +2132,26 @@ create_netdev(device_t dev) > > /* Set up ifnet structure */ > ifp = np->xn_ifp = if_alloc(IFT_ETHER); > - ifp->if_softc = np; > - if_initname(ifp, "xn", device_get_unit(dev)); > - ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; > - ifp->if_ioctl = xn_ioctl; > - ifp->if_output = ether_output; > - ifp->if_start = xn_start; > + ifp->if_softc = np; > + if_initname(ifp, "xn", device_get_unit(dev)); > + ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST; > + ifp->if_ioctl = xn_ioctl; > + ifp->if_output = ether_output; > + ifp->if_start = xn_start; > #ifdef notyet > - ifp->if_watchdog = xn_watchdog; > + ifp->if_watchdog = xn_watchdog; > #endif > - ifp->if_init = xn_ifinit; > - ifp->if_snd.ifq_maxlen = NET_TX_RING_SIZE - 1; > - > - ifp->if_hwassist = XN_CSUM_FEATURES; > - ifp->if_capabilities = IFCAP_HWCSUM; > + ifp->if_init = xn_ifinit; > + ifp->if_snd.ifq_maxlen = NET_TX_RING_SIZE - 1; > + > + ifp->if_hwassist = XN_CSUM_FEATURES; > + ifp->if_capabilities = IFCAP_HWCSUM; > ifp->if_hw_tsomax = 65536 - (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN); > ifp->if_hw_tsomaxsegcount = MAX_TX_REQ_FRAGS; > ifp->if_hw_tsomaxsegsize = PAGE_SIZE; > > - ether_ifattach(ifp, np->mac); > - callout_init(&np->xn_stat_ch, CALLOUT_MPSAFE); > + ether_ifattach(ifp, np->mac); > + callout_init(&np->xn_stat_ch, CALLOUT_MPSAFE); > netfront_carrier_off(np); This chunk seems completely unrelated to the patch itself, is this some kind of code formatting cleanup? If so, this should be sent in a separate patch. > return (0); > diff --git a/sys/x86/xen/hvm.c b/sys/x86/xen/hvm.c > index c386953..1c2ba9d 100644 > --- a/sys/x86/xen/hvm.c > +++ b/sys/x86/xen/hvm.c > @@ -517,10 +517,9 @@ xen_hvm_init(enum xen_hvm_init_type init_type) > int error; > int i; > > - if (init_type == XEN_HVM_INIT_CANCELLED_SUSPEND) > - return; > - > - error = xen_hvm_init_hypercall_stubs(); > + if (init_type != XEN_HVM_INIT_CANCELLED_SUSPEND) { > + error = xen_hvm_init_hypercall_stubs(); > + } > > switch (init_type) { > case XEN_HVM_INIT_COLD: > @@ -541,13 +540,17 @@ xen_hvm_init(enum xen_hvm_init_type init_type) > CPU_FOREACH(i) > DPCPU_ID_SET(i, vcpu_info, NULL); > break; > + case XEN_HVM_INIT_CANCELLED_SUSPEND: > + break; > default: > panic("Unsupported HVM initialization type"); > } > > xen_vector_callback_enabled = 0; > xen_domain_type = XEN_HVM_DOMAIN; > - xen_hvm_init_shared_info_page(); > + if (init_type != XEN_HVM_INIT_CANCELLED_SUSPEND) { > + xen_hvm_init_shared_info_page(); > + } > xen_hvm_set_callback(NULL); > xen_hvm_disable_emulated_devices(); I'm not sure I follow why this change to xen_hvm_init is needed, on a cancelled suspend you shouldn't need to re-set the callback vector or unplug any emulated devices. > } > @@ -565,7 +568,8 @@ xen_hvm_resume(bool suspend_cancelled) > XEN_HVM_INIT_CANCELLED_SUSPEND : XEN_HVM_INIT_RESUME); > > /* Register vcpu_info area for CPU#0. */ > - xen_hvm_cpu_init(); > + if(!suspend_cancelled) > + xen_hvm_cpu_init(); > } Seeing this here, why don't we just avoid calling xen_hvm_resume from xctrl_suspend on a cancelled suspend? > static void > diff --git a/sys/xen/xenbus/xenbusb.c b/sys/xen/xenbus/xenbusb.c > index 1f84795..cce83cd 100644 > --- a/sys/xen/xenbus/xenbusb.c > +++ b/sys/xen/xenbus/xenbusb.c > @@ -75,6 +75,8 @@ __FBSDID("$FreeBSD$"); > #include > #include > > +static int xenbusb_suspend_cancelled = 0; > + > /*------------------------- Private Functions --------------------------------*/ > /** > * \brief Deallocate XenBus device instance variables. > @@ -776,6 +778,11 @@ xenbusb_attach(device_t dev, char *bus_node, u_int id_components) > return (0); > } > > +void xenbusb_set_suspend_cancel(void) > +{ > + xenbusb_suspend_cancelled = 1; > +} > + > int > xenbusb_resume(device_t dev) > { > @@ -793,29 +800,32 @@ xenbusb_resume(device_t dev) > if (device_get_state(kids[i]) == DS_NOTPRESENT) > continue; > > - ivars = device_get_ivars(kids[i]); > + if(xenbusb_suspend_cancelled == 0) { > + ivars = device_get_ivars(kids[i]); > > - xs_unregister_watch(&ivars->xd_otherend_watch); > - xenbus_set_state(kids[i], XenbusStateInitialising); > + xs_unregister_watch(&ivars->xd_otherend_watch); > + xenbus_set_state(kids[i], XenbusStateInitialising); > > - /* > - * Find the new backend details and > - * re-register our watch. > - */ > - error = XENBUSB_GET_OTHEREND_NODE(dev, ivars); > - if (error) > - return (error); > + /* > + * Find the new backend details and > + * re-register our watch. > + */ > + error = XENBUSB_GET_OTHEREND_NODE(dev, ivars); > + if (error) > + return (error); > > - statepath = malloc(ivars->xd_otherend_path_len > - + strlen("/state") + 1, M_XENBUS, M_WAITOK); > - sprintf(statepath, "%s/state", ivars->xd_otherend_path); > + statepath = malloc(ivars->xd_otherend_path_len > + + strlen("/state") + 1, M_XENBUS, M_WAITOK); > + sprintf(statepath, "%s/state", ivars->xd_otherend_path); > > - free(ivars->xd_otherend_watch.node, M_XENBUS); > - ivars->xd_otherend_watch.node = statepath; > + free(ivars->xd_otherend_watch.node, M_XENBUS); > + ivars->xd_otherend_watch.node = statepath; > + } > > DEVICE_RESUME(kids[i]); > > - xs_register_watch(&ivars->xd_otherend_watch); > + if(xenbusb_suspend_cancelled == 0) > + xs_register_watch(&ivars->xd_otherend_watch); Why don't you just add: if (xenbusb_suspend_cancelled == 1) { DEVICE_RESUME(kids[i]); continue; } To the top of the loop? This would present adding a bunch of nested conditionals. > #if 0 > /* > * Can't do this yet since we are running in > @@ -834,6 +844,9 @@ xenbusb_resume(device_t dev) > free(kids, M_TEMP); > } > > + if(xenbusb_suspend_cancelled == 1) > + xenbusb_suspend_cancelled = 0; > + > return (0); > } > > diff --git a/sys/xen/xenbus/xenbusvar.h b/sys/xen/xenbus/xenbusvar.h > index ab5d01f..aff3b60 100644 > --- a/sys/xen/xenbus/xenbusvar.h > +++ b/sys/xen/xenbus/xenbusvar.h > @@ -272,4 +272,8 @@ void xenbus_localend_changed(device_t dev, const char *path); > > #include "xenbus_if.h" > > +void xenbusb_set_suspend_cancel(void); > +void xbd_set_suspend_cancel(void); > +void xn_set_suspend_cancel(void); > + > #endif /* _XEN_XENBUS_XENBUSVAR_H */ > -- > 1.8.3.4 > > From 423a959a268b205c048489e6addeba5db449e322 Mon Sep 17 00:00:00 2001 > From: Yingdong Liu > Date: Thu, 15 Dec 2016 09:09:16 +0800 > Subject: [PATCH] [PATCH]netfront: need release all resources after adding and > removing NICs time and again > > --- > sys/dev/xen/netfront/netfront.c | 46 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 46 insertions(+) > > diff --git a/sys/dev/xen/netfront/netfront.c b/sys/dev/xen/netfront/netfront.c > index 0caaa2c..5b67e12 100644 > --- a/sys/dev/xen/netfront/netfront.c > +++ b/sys/dev/xen/netfront/netfront.c > @@ -2196,6 +2196,46 @@ netfront_detach(device_t dev) > } > > static void > +netif_release_rx_bufs_copy(struct netfront_info *np) > +{ > + struct mbuf *m; > + int i, ref; > + int busy = 0, inuse = 0; > + > + XN_RX_LOCK(np); > + > + for (i = 0; i < NET_RX_RING_SIZE; i++) { > + ref = np->grant_rx_ref[i]; > + > + if (ref == GRANT_REF_INVALID) > + continue; > + > + inuse++; > + > + m = np->rx_mbufs[i]; > + > + if (!gnttab_end_foreign_access_ref(ref)) > + { > + busy++; > + continue; > + } > + > + gnttab_release_grant_reference(&np->gref_rx_head, ref); > + np->grant_rx_ref[i] = GRANT_REF_INVALID; > + add_id_to_freelist(np->rx_mbufs, i); > + > + m_freem(m); > + } > + > + if (busy) > + device_printf(np->xbdev, "Unable to release %d of %d " > + "inuse grant references out of %ld total.\n", > + busy, inuse, NET_RX_RING_SIZE); > + Stray tab. > + XN_RX_UNLOCK(np); > +} > + > +static void > netif_free(struct netfront_info *info) > { > XN_LOCK(info); > @@ -2209,6 +2249,12 @@ netif_free(struct netfront_info *info) > info->xn_ifp = NULL; > } > ifmedia_removeall(&info->sc_media); > + netif_release_tx_bufs(info); > + if (info->copying_receiver) > + netif_release_rx_bufs_copy(info); > + > + gnttab_free_grant_references(info->gref_tx_head); > + gnttab_free_grant_references(info->gref_rx_head); > } This patch looks fine, but it needs to be rebased on top of HEAD in order to be applied. Roger.