From: "Michael S. Tsirkin" <mst@redhat.com>
To: Krishna Kumar <krkumar2@in.ibm.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, shemminger@vyatta.com
Subject: Re: [PATCH] Fix panic in virtnet_remove
Date: Sun, 17 Jul 2011 13:12:15 +0300 [thread overview]
Message-ID: <20110717101215.GA19287@redhat.com> (raw)
In-Reply-To: <20110715091650.23026.78221.sendpatchset@krkumar2.in.ibm.com>
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
next prev parent reply other threads:[~2011-07-17 10:11 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-07-15 9:16 [PATCH] Fix panic in virtnet_remove Krishna Kumar
2011-07-15 15:19 ` David Miller
2011-07-16 5:32 ` Krishna Kumar2
2011-07-17 10:12 ` Michael S. Tsirkin [this message]
-- strict thread matches above, loose matches on Subject: below --
2011-07-18 3:57 Krishna Kumar
2011-07-18 13:03 ` Michael S. Tsirkin
2011-07-20 7:43 Krishna Kumar
2011-07-20 15:17 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110717101215.GA19287@redhat.com \
--to=mst@redhat.com \
--cc=davem@davemloft.net \
--cc=krkumar2@in.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=shemminger@vyatta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.