netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH repost] Fix panic in virtnet_remove
@ 2011-07-20 13:56 Krishna Kumar
  2011-07-20 14:31 ` Michael S. Tsirkin
  0 siblings, 1 reply; 3+ messages in thread
From: Krishna Kumar @ 2011-07-20 13:56 UTC (permalink / raw)
  To: Rusty Russell, Michael S. Tsirkin
  Cc: netdev, linux-kernel, Krishna Kumar, virtualization

Fix a panic in virtnet_remove. unregister_netdev has already
freed up the netdev (and virtnet_info) due to dev->destructor
being set, while virtnet_info is still required. Remove
virtnet_free altogether, and move the freeing of the per-cpu
statistics from virtnet_free to virtnet_remove.

Tested patch below.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
(Michael suggested me to re-post with copy to all maintainers)

 drivers/net/virtio_net.c |   10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c
--- org/drivers/net/virtio_net.c	2011-07-18 09:14:02.000000000 +0530
+++ new/drivers/net/virtio_net.c	2011-07-18 09:16:35.000000000 +0530
@@ -705,14 +705,6 @@ static void virtnet_netpoll(struct net_d
 }
 #endif
 
-static void virtnet_free(struct net_device *dev)
-{
-	struct virtnet_info *vi = netdev_priv(dev);
-
-	free_percpu(vi->stats);
-	free_netdev(dev);
-}
-
 static int virtnet_open(struct net_device *dev)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -959,7 +951,6 @@ static int virtnet_probe(struct virtio_d
 	/* Set up network device as normal. */
 	dev->netdev_ops = &virtnet_netdev;
 	dev->features = NETIF_F_HIGHDMA;
-	dev->destructor = virtnet_free;
 
 	SET_ETHTOOL_OPS(dev, &virtnet_ethtool_ops);
 	SET_NETDEV_DEV(dev, &vdev->dev);
@@ -1122,6 +1113,7 @@ static void __devexit virtnet_remove(str
 	while (vi->pages)
 		__free_pages(get_a_page(vi, GFP_KERNEL), 0);
 
+	free_percpu(vi->stats);
 	free_netdev(vi->dev);
 }
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH repost] Fix panic in virtnet_remove
  2011-07-20 13:56 [PATCH repost] Fix panic in virtnet_remove Krishna Kumar
@ 2011-07-20 14:31 ` Michael S. Tsirkin
  2011-07-22  4:29   ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Michael S. Tsirkin @ 2011-07-20 14:31 UTC (permalink / raw)
  To: Krishna Kumar
  Cc: Rusty Russell, netdev, linux-kernel, virtualization, shemminger

On Wed, Jul 20, 2011 at 07:26:02PM +0530, Krishna Kumar wrote:
> Fix a panic in virtnet_remove. unregister_netdev has already
> freed up the netdev (and virtnet_info) due to dev->destructor
> being set, while virtnet_info is still required. Remove
> virtnet_free altogether, and move the freeing of the per-cpu
> statistics from virtnet_free to virtnet_remove.
> 
> Tested patch below.
> 
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>

Also note that the crash was apparently introduced by
3fa2a1df909482cc234524906e4bd30dee3514df in net-next,
so this is a net-next only patch.

Stephen, was there any special reason to free the memory
in the destructor like you did?

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> (Michael suggested me to re-post with copy to all maintainers)
> 
>  drivers/net/virtio_net.c |   10 +---------
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c
> --- org/drivers/net/virtio_net.c	2011-07-18 09:14:02.000000000 +0530
> +++ new/drivers/net/virtio_net.c	2011-07-18 09:16:35.000000000 +0530
> @@ -705,14 +705,6 @@ static void virtnet_netpoll(struct net_d
>  }
>  #endif
>  
> -static void virtnet_free(struct net_device *dev)
> -{
> -	struct virtnet_info *vi = netdev_priv(dev);
> -
> -	free_percpu(vi->stats);
> -	free_netdev(dev);
> -}
> -
>  static int virtnet_open(struct net_device *dev)
>  {
>  	struct virtnet_info *vi = netdev_priv(dev);
> @@ -959,7 +951,6 @@ static int virtnet_probe(struct virtio_d
>  	/* Set up network device as normal. */
>  	dev->netdev_ops = &virtnet_netdev;
>  	dev->features = NETIF_F_HIGHDMA;
> -	dev->destructor = virtnet_free;
>  
>  	SET_ETHTOOL_OPS(dev, &virtnet_ethtool_ops);
>  	SET_NETDEV_DEV(dev, &vdev->dev);
> @@ -1122,6 +1113,7 @@ static void __devexit virtnet_remove(str
>  	while (vi->pages)
>  		__free_pages(get_a_page(vi, GFP_KERNEL), 0);
>  
> +	free_percpu(vi->stats);
>  	free_netdev(vi->dev);
>  }
>  

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH repost] Fix panic in virtnet_remove
  2011-07-20 14:31 ` Michael S. Tsirkin
@ 2011-07-22  4:29   ` David Miller
  0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2011-07-22  4:29 UTC (permalink / raw)
  To: mst; +Cc: krkumar2, rusty, netdev, linux-kernel, virtualization, shemminger

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Wed, 20 Jul 2011 17:31:15 +0300

> On Wed, Jul 20, 2011 at 07:26:02PM +0530, Krishna Kumar wrote:
>> Fix a panic in virtnet_remove. unregister_netdev has already
>> freed up the netdev (and virtnet_info) due to dev->destructor
>> being set, while virtnet_info is still required. Remove
>> virtnet_free altogether, and move the freeing of the per-cpu
>> statistics from virtnet_free to virtnet_remove.
>> 
>> Tested patch below.
>> 
>> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> 
> Also note that the crash was apparently introduced by
> 3fa2a1df909482cc234524906e4bd30dee3514df in net-next,
> so this is a net-next only patch.
> 
> Stephen, was there any special reason to free the memory
> in the destructor like you did?
> 
> Acked-by: Michael S. Tsirkin <mst@redhat.com>

Applied.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-07-22  4:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-20 13:56 [PATCH repost] Fix panic in virtnet_remove Krishna Kumar
2011-07-20 14:31 ` Michael S. Tsirkin
2011-07-22  4:29   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).