| Message ID | 1360944010-15336-3-git-send-email-wei.liu2@citrix.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Feb 15, 2013 at 04:00:03PM +0000, Wei Liu wrote: > Enable users to unload netback module. Users should make sure there is not vif > runnig. 'sure there are no vif's running.' Any way of making this VIF part be automatic? Meaning that netback can figure out if there are VIFs running and if so don't unload all of the parts and just mention that you are leaking memory. This looks quite dangerous - meaning if there are guests running and we for fun do 'rmmod xen_netback' it looks like we could crash dom0? > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > --- > drivers/net/xen-netback/common.h | 1 + > drivers/net/xen-netback/netback.c | 18 ++++++++++++++++++ > drivers/net/xen-netback/xenbus.c | 5 +++++ > 3 files changed, 24 insertions(+) > > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h > index 9d7f172..35d8772 100644 > --- a/drivers/net/xen-netback/common.h > +++ b/drivers/net/xen-netback/common.h > @@ -120,6 +120,7 @@ void xenvif_get(struct xenvif *vif); > void xenvif_put(struct xenvif *vif); > > int xenvif_xenbus_init(void); > +void xenvif_xenbus_exit(void); > > int xenvif_schedulable(struct xenvif *vif); > > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c > index db8d45a..de59098 100644 > --- a/drivers/net/xen-netback/netback.c > +++ b/drivers/net/xen-netback/netback.c > @@ -1761,5 +1761,23 @@ failed_init: > > module_init(netback_init); > > +static void __exit netback_exit(void) > +{ > + int group, i; > + xenvif_xenbus_exit(); You should check the return code of this function. > + for (group = 0; group < xen_netbk_group_nr; group++) { > + struct xen_netbk *netbk = &xen_netbk[group]; > + for (i = 0; i < MAX_PENDING_REQS; i++) { > + if (netbk->mmap_pages[i]) > + __free_page(netbk->mmap_pages[i]); > + } > + del_timer_sync(&netbk->net_timer); > + kthread_stop(netbk->task); > + } > + vfree(xen_netbk); > +} > + > +module_exit(netback_exit); > + > MODULE_LICENSE("Dual BSD/GPL"); > MODULE_ALIAS("xen-backend:vif"); > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c > index 410018c..65d14f2 100644 > --- a/drivers/net/xen-netback/xenbus.c > +++ b/drivers/net/xen-netback/xenbus.c > @@ -485,3 +485,8 @@ int xenvif_xenbus_init(void) > { > return xenbus_register_backend(&netback_driver); > } > + > +void xenvif_xenbus_exit(void) > +{ > + return xenbus_unregister_driver(&netback_driver); > +} > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/03/13 20:55, Konrad Rzeszutek Wilk wrote: > On Fri, Feb 15, 2013 at 04:00:03PM +0000, Wei Liu wrote: >> Enable users to unload netback module. Users should make sure there is not vif >> runnig. > 'sure there are no vif's running.' If we are picking at grammar, no apostrophe in 'vifs' ~Andrew > > Any way of making this VIF part be automatic? Meaning that netback > can figure out if there are VIFs running and if so don't unload > all of the parts and just mention that you are leaking memory. > > This looks quite dangerous - meaning if there are guests running and > we for fun do 'rmmod xen_netback' it looks like we could crash dom0? > >> Signed-off-by: Wei Liu <wei.liu2@citrix.com> >> --- >> drivers/net/xen-netback/common.h | 1 + >> drivers/net/xen-netback/netback.c | 18 ++++++++++++++++++ >> drivers/net/xen-netback/xenbus.c | 5 +++++ >> 3 files changed, 24 insertions(+) >> >> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h >> index 9d7f172..35d8772 100644 >> --- a/drivers/net/xen-netback/common.h >> +++ b/drivers/net/xen-netback/common.h >> @@ -120,6 +120,7 @@ void xenvif_get(struct xenvif *vif); >> void xenvif_put(struct xenvif *vif); >> >> int xenvif_xenbus_init(void); >> +void xenvif_xenbus_exit(void); >> >> int xenvif_schedulable(struct xenvif *vif); >> >> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c >> index db8d45a..de59098 100644 >> --- a/drivers/net/xen-netback/netback.c >> +++ b/drivers/net/xen-netback/netback.c >> @@ -1761,5 +1761,23 @@ failed_init: >> >> module_init(netback_init); >> >> +static void __exit netback_exit(void) >> +{ >> + int group, i; >> + xenvif_xenbus_exit(); > You should check the return code of this function. > >> + for (group = 0; group < xen_netbk_group_nr; group++) { >> + struct xen_netbk *netbk = &xen_netbk[group]; >> + for (i = 0; i < MAX_PENDING_REQS; i++) { >> + if (netbk->mmap_pages[i]) >> + __free_page(netbk->mmap_pages[i]); >> + } >> + del_timer_sync(&netbk->net_timer); >> + kthread_stop(netbk->task); >> + } >> + vfree(xen_netbk); >> +} >> + >> +module_exit(netback_exit); >> + >> MODULE_LICENSE("Dual BSD/GPL"); >> MODULE_ALIAS("xen-backend:vif"); >> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c >> index 410018c..65d14f2 100644 >> --- a/drivers/net/xen-netback/xenbus.c >> +++ b/drivers/net/xen-netback/xenbus.c >> @@ -485,3 +485,8 @@ int xenvif_xenbus_init(void) >> { >> return xenbus_register_backend(&netback_driver); >> } >> + >> +void xenvif_xenbus_exit(void) >> +{ >> + return xenbus_unregister_driver(&netback_driver); >> +} >> -- >> 1.7.10.4 >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel >> > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 15 Feb 2013 16:00:03 +0000 Wei Liu <wei.liu2@citrix.com> wrote: > Enable users to unload netback module. Users should make sure there is not vif > runnig. Isn't it likely that some admin might be trying to cleanup or do shutdown and there is an ordering problem. What happens if vif's are still running? Why depend on users? You should allow it anytime and do refcounting and auto-destroy the vif's for them. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-03-04 at 21:58 +0000, Stephen Hemminger wrote: > On Fri, 15 Feb 2013 16:00:03 +0000 > Wei Liu <wei.liu2@citrix.com> wrote: > > > Enable users to unload netback module. Users should make sure there is not vif > > runnig. > > > Isn't it likely that some admin might be trying to cleanup or do shutdown and there > is an ordering problem. What happens if vif's are still running? > > Why depend on users? You should allow it anytime and do refcounting and auto-destroy the vif's for them. Sure, we should not depend on users. I will move my vif get/put module patch before this one. Wei. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2013-03-04 at 20:55 +0000, Konrad Rzeszutek Wilk wrote: > On Fri, Feb 15, 2013 at 04:00:03PM +0000, Wei Liu wrote: > > Enable users to unload netback module. Users should make sure there is not vif > > runnig. > > 'sure there are no vif's running.' > > Any way of making this VIF part be automatic? Meaning that netback > can figure out if there are VIFs running and if so don't unload > all of the parts and just mention that you are leaking memory. > > This looks quite dangerous - meaning if there are guests running and > we for fun do 'rmmod xen_netback' it looks like we could crash dom0? > Dom0 will not crash. But as you suggested in a latter email, I should move the get/put module patch before this one. The rationale behind this patch is that if there's anything wrong discovered inside netback, we can just migrate all VMs to a new host, unload old netback, load new netback then migrate all VMs back. This should be useful for both production and testing. Wei. > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > > --- > > drivers/net/xen-netback/common.h | 1 + > > drivers/net/xen-netback/netback.c | 18 ++++++++++++++++++ > > drivers/net/xen-netback/xenbus.c | 5 +++++ > > 3 files changed, 24 insertions(+) > > > > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h > > index 9d7f172..35d8772 100644 > > --- a/drivers/net/xen-netback/common.h > > +++ b/drivers/net/xen-netback/common.h > > @@ -120,6 +120,7 @@ void xenvif_get(struct xenvif *vif); > > void xenvif_put(struct xenvif *vif); > > > > int xenvif_xenbus_init(void); > > +void xenvif_xenbus_exit(void); > > > > int xenvif_schedulable(struct xenvif *vif); > > > > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c > > index db8d45a..de59098 100644 > > --- a/drivers/net/xen-netback/netback.c > > +++ b/drivers/net/xen-netback/netback.c > > @@ -1761,5 +1761,23 @@ failed_init: > > > > module_init(netback_init); > > > > +static void __exit netback_exit(void) > > +{ > > + int group, i; > > + xenvif_xenbus_exit(); > > You should check the return code of this function. > > > + for (group = 0; group < xen_netbk_group_nr; group++) { > > + struct xen_netbk *netbk = &xen_netbk[group]; > > + for (i = 0; i < MAX_PENDING_REQS; i++) { > > + if (netbk->mmap_pages[i]) > > + __free_page(netbk->mmap_pages[i]); > > + } > > + del_timer_sync(&netbk->net_timer); > > + kthread_stop(netbk->task); > > + } > > + vfree(xen_netbk); > > +} > > + > > +module_exit(netback_exit); > > + > > MODULE_LICENSE("Dual BSD/GPL"); > > MODULE_ALIAS("xen-backend:vif"); > > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c > > index 410018c..65d14f2 100644 > > --- a/drivers/net/xen-netback/xenbus.c > > +++ b/drivers/net/xen-netback/xenbus.c > > @@ -485,3 +485,8 @@ int xenvif_xenbus_init(void) > > { > > return xenbus_register_backend(&netback_driver); > > } > > + > > +void xenvif_xenbus_exit(void) > > +{ > > + return xenbus_unregister_driver(&netback_driver); > > +} > > -- > > 1.7.10.4 > > > > > > _______________________________________________ > > Xen-devel mailing list > > Xen-devel@lists.xen.org > > http://lists.xen.org/xen-devel > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h index 9d7f172..35d8772 100644 --- a/drivers/net/xen-netback/common.h +++ b/drivers/net/xen-netback/common.h @@ -120,6 +120,7 @@ void xenvif_get(struct xenvif *vif); void xenvif_put(struct xenvif *vif); int xenvif_xenbus_init(void); +void xenvif_xenbus_exit(void); int xenvif_schedulable(struct xenvif *vif); diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index db8d45a..de59098 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1761,5 +1761,23 @@ failed_init: module_init(netback_init); +static void __exit netback_exit(void) +{ + int group, i; + xenvif_xenbus_exit(); + for (group = 0; group < xen_netbk_group_nr; group++) { + struct xen_netbk *netbk = &xen_netbk[group]; + for (i = 0; i < MAX_PENDING_REQS; i++) { + if (netbk->mmap_pages[i]) + __free_page(netbk->mmap_pages[i]); + } + del_timer_sync(&netbk->net_timer); + kthread_stop(netbk->task); + } + vfree(xen_netbk); +} + +module_exit(netback_exit); + MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("xen-backend:vif"); diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index 410018c..65d14f2 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -485,3 +485,8 @@ int xenvif_xenbus_init(void) { return xenbus_register_backend(&netback_driver); } + +void xenvif_xenbus_exit(void) +{ + return xenbus_unregister_driver(&netback_driver); +}
Enable users to unload netback module. Users should make sure there is not vif runnig. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- drivers/net/xen-netback/common.h | 1 + drivers/net/xen-netback/netback.c | 18 ++++++++++++++++++ drivers/net/xen-netback/xenbus.c | 5 +++++ 3 files changed, 24 insertions(+)