Date: Thu, 15 Dec 2016 02:00:53 +0000 From: Liuyingdong <liuyingdong@huawei.com> To: freebsd xen <roger.pau@citrix.com> Cc: "freebsd-xen@freebsd.org" <freebsd-xen@freebsd.org>, "Zhaojun (Euler)" <zhao.zhaojun@huawei.com>, Suoben <suoben@huawei.com>, "Ouyangzhaowei (Charles)" <ouyangzhaowei@huawei.com>, chuzhaosong <chuzhaosong@huawei.com>, Wanglinkai <wanglinkai@huawei.com> Subject: Re: [PATCH]netfront: need release all resources after adding and removing NICs time and again Message-ID: <3655E9A8B903724782E1F75DCFD74E6B01229F6F11@szxema506-mbs.china.huawei.com> In-Reply-To: <20161213142919.ulvvvwoz2da5fedw@dhcp-3-221.uk.xensource.com> References: <5825272C.3010704@huawei.com> <584F8F1C.6030006@huawei.com> <20161213142919.ulvvvwoz2da5fedw@dhcp-3-221.uk.xensource.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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.
-----邮件原件-----
发件人: freebsd xen [mailto:roger.pau@citrix.com]
发送时间: 2016年12月13日 22:29
收件人: Liuyingdong
抄送: freebsd-xen@freebsd.org; Zhaojun (Euler); Suoben; Ouyangzhaowei (Charles)
主题: Re: [PATCH]netfront: need release all resources after adding and removing NICs time and again
On Tue, Dec 13, 2016 at 02:03:08PM +0800, liuyingdong wrote:
> Hello Roger,
> I want to know how about this patch,Please let me know if you have any questions.
> Thanks.
Hello,
Thanks for the patches! This one is looking fine, it's just that I'm a little bit busy at the moment, and there are some issues in HEAD related to Xen that I would like to fix before pushing anything new.
In any case, I see that you are sending the patches using Thunderbird, which is not ideal (MUAs tend to mangle patches). The preferred way for sending patches is using "git send-email"[0] directly. There are also several tutorials online that will help you setup git send-email correctly. If there's some reason why you can't use git send-email I would recommend that you also attach the patches directly to emails, that way they won't probably get mangled.
Roger.
[0] https://git-scm.com/docs/git-send-email
[-- Attachment #2 --]
From fc85ac7eba55a5f14f5f7d81f0e1fc7fbf6a7447 Mon Sep 17 00:00:00 2001
From: Yingdong Liu <liuyingdong@huawei.com>
Date: Tue, 13 Dec 2016 21:53:25 +0800
Subject: [PATCH] introduce suspend cancel mechanism for frontend devices
---
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;
+
/*--------------------------- 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);
+
+ if(blkfront_suspend_cancelled == 1) {
+ 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) {
+ 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();
+ }
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
+ 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);
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();
}
@@ -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();
}
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 <xen/xenbus/xenbusb.h>
#include <xen/xenbus/xenbusvar.h>
+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);
#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
[-- Attachment #3 --]
From 423a959a268b205c048489e6addeba5db449e322 Mon Sep 17 00:00:00 2001
From: Yingdong Liu <liuyingdong@huawei.com>
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);
+
+ 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);
}
static void
--
1.8.3.4
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3655E9A8B903724782E1F75DCFD74E6B01229F6F11>