Message ID | 20110715091650.23026.78221.sendpatchset@krkumar2.in.ibm.com |
---|---|
State | Awaiting Upstream, archived |
Delegated to: | David Miller |
Headers | show |
From: Krishna Kumar <krkumar2@in.ibm.com> Date: Fri, 15 Jul 2011 14:46:50 +0530 > modprobe -r virtio_net panics in free_netdev() as the > dev is already freed in the newly introduced virtnet_free > (commit 3fa2a1df9094). Since virtnet_remove doesn't require > dev after unregister, I am removing the free_netdev call > in virtnet_remove instead of in virtnet_free (which seems > to be the right place to free the dev). Confirmed that > the panic is fixed with this patch. > > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> I guess the virtio NET maintainers don't deserve to get CC:'d on a fix like this? :-( Michael Tsirkin is who maintains this driver actively and is the one who will merge this kind of fix to me, therefore if you don't CC: him it might not get integrated at all. -- 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
David Miller <davem@davemloft.net> wrote on 07/15/2011 08:49:09 PM: > > modprobe -r virtio_net panics in free_netdev() as the > > dev is already freed in the newly introduced virtnet_free > > (commit 3fa2a1df9094). Since virtnet_remove doesn't require > > dev after unregister, I am removing the free_netdev call > > in virtnet_remove instead of in virtnet_free (which seems > > to be the right place to free the dev). Confirmed that > > the panic is fixed with this patch. > > > > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> > > I guess the virtio NET maintainers don't deserve to get CC:'d on a fix > like this? :-( > > Michael Tsirkin is who maintains this driver actively and is the one > who will merge this kind of fix to me, therefore if you don't CC: > him it might not get integrated at all. Sorry, but it was not intentional. I saw that Stephen had made that change so cc'd him instead. Michael, I am attaching the patch below, please let me know if you need it inlined: (See attached file: patch) Thanks, - KK
On Fri, Jul 15, 2011 at 02:46:50PM +0530, Krishna Kumar wrote: > modprobe -r virtio_net panics in free_netdev() as the > dev is already freed in the newly introduced virtnet_free > (commit 3fa2a1df9094). Good catch, thanks! > Since virtnet_remove doesn't require > dev after unregister, I'm not sure that true: vi used just above the line you remove is I think the struct virtnet_info that is allocated as part of that structure. > I am removing the free_netdev call > in virtnet_remove instead of in virtnet_free (which seems > to be the right place to free the dev). Confirmed that > the panic is fixed with this patch. This might be just because the memory isn't reused. Try enabling slab poisoning, I think you'll observe some problems. Do we absolutely have to have a destructor? Can't we move the per cpu counter free from virtnet_free to virtnet_remove, and get rid of virtnet_free completely? > Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> > --- > drivers/net/virtio_net.c | 2 -- > 1 file changed, 2 deletions(-) > > diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c > --- org/drivers/net/virtio_net.c 2011-07-04 10:38:33.000000000 +0530 > +++ new/drivers/net/virtio_net.c 2011-07-15 14:27:48.000000000 +0530 > @@ -1121,8 +1121,6 @@ static void __devexit virtnet_remove(str > > while (vi->pages) > __free_pages(get_a_page(vi, GFP_KERNEL), 0); > - > - free_netdev(vi->dev); > } > > static struct virtio_device_id id_table[] = { > -- > 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 -- 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 -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c --- org/drivers/net/virtio_net.c 2011-07-04 10:38:33.000000000 +0530 +++ new/drivers/net/virtio_net.c 2011-07-15 14:27:48.000000000 +0530 @@ -1121,8 +1121,6 @@ static void __devexit virtnet_remove(str while (vi->pages) __free_pages(get_a_page(vi, GFP_KERNEL), 0); - - free_netdev(vi->dev); } static struct virtio_device_id id_table[] = {
modprobe -r virtio_net panics in free_netdev() as the dev is already freed in the newly introduced virtnet_free (commit 3fa2a1df9094). Since virtnet_remove doesn't require dev after unregister, I am removing the free_netdev call in virtnet_remove instead of in virtnet_free (which seems to be the right place to free the dev). Confirmed that the panic is fixed with this patch. Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> --- drivers/net/virtio_net.c | 2 -- 1 file changed, 2 deletions(-) -- 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