Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 26 Jul 2013 01:54:33 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Marko Zec <zec@fer.hr>
Cc:        Craig Rodrigues <rodrigc@FreeBSD.org>, svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Marius Strobl <marius@alchemy.franken.de>
Subject:   Re: svn commit: r253346 - in head/sys: kern net netgraph netgraph/bluetooth/socket
Message-ID:  <20130726015202.O2570@besplex.bde.org>
In-Reply-To: <201307251048.33475.zec@fer.hr>
References:  <201307150132.r6F1WttU081255@svn.freebsd.org> <20130725080758.GE948@alchemy.franken.de> <201307251048.33475.zec@fer.hr>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 25 Jul 2013, Marko Zec wrote:

> On Thursday 25 July 2013 10:07:58 Marius Strobl wrote:
>> On Mon, Jul 15, 2013 at 01:32:55AM +0000, Craig Rodrigues wrote:
>>> Author: rodrigc
>>> Date: Mon Jul 15 01:32:55 2013
>>> New Revision: 253346
>>> URL: http://svnweb.freebsd.org/changeset/base/253346
>>>
>>> Log:
>>>   PR: 168520 170096
>>>   Submitted by: adrian, zec
>>>
>>>   Fix multiple kernel panics when VIMAGE is enabled in the kernel.
>>>   These fixes are based on patches submitted by Adrian Chadd and Marko
>>> Zec.
>>>
>>>   (1)  Set curthread->td_vnet to vnet0 in device_probe_and_attach()
>>> just before calling device_attach().  This fixes multiple VIMAGE
>>> related kernel panics when trying to attach Bluetooth or USB Ethernet
>>> devices because curthread->td_vnet is NULL.
>>>
>>>   (2)  Set curthread->td_vnet in if_detach().  This fixes kernel panics
>>> when detaching networking interfaces, especially USB Ethernet devices.
>>>
>>>   (3)  Use VNET_DOMAIN_SET() in ng_btsocket.c
>>>
>>>   (4)  In ng_unref_node() set curthread->td_vnet.  This fixes kernel
>>> panics when detaching Netgraph nodes.
>>>
>>> Modified:
>>>   head/sys/kern/subr_bus.c
>>>   head/sys/net/if.c
>>>   head/sys/netgraph/bluetooth/socket/ng_btsocket.c
>>>   head/sys/netgraph/ng_base.c
>>>
>>> Modified: head/sys/kern/subr_bus.c
>>> =======================================================================
>>> ======= --- head/sys/kern/subr_bus.c	Mon Jul 15 00:49:10 2013	(r253345)
>>> +++ head/sys/kern/subr_bus.c	Mon Jul 15 01:32:55 2013	(r253346) @@
>>> -53,6 +53,8 @@ __FBSDID("$FreeBSD$");
>>>  #include <sys/bus.h>
>>>  #include <sys/interrupt.h>
>>>
>>> +#include <net/vnet.h>
>>> +
>>>  #include <machine/stdarg.h>
>>>
>>>  #include <vm/uma.h>
>>> @@ -2735,7 +2737,11 @@ device_probe_and_attach(device_t dev)
>>>  		return (0);
>>>  	else if (error != 0)
>>>  		return (error);
>>> -	return (device_attach(dev));
>>> +
>>> +	CURVNET_SET_QUIET(vnet0);
>>> +	error = device_attach(dev);
>>> +	CURVNET_RESTORE();
>>> +	return error;
>>>  }
>>
>> Uhm - do we really need to have that layering violation in subr_bus.c?
>> Wouldn't it be sufficient to set curthread->td_vnet to vnet0 in
>> if_alloc(9) or if_attach(9) at least instead?
>
> That wouldn't solve the issues with attaching netgraph node(s) to the
> arriving device, nor potentially other issues unrelated to if_alloc or
> if_attach.

This patch also has some style bugs (change from silly parentheses around
the return value to none.  2 returns in unchanged code visible in the
patch still use normal style).

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130726015202.O2570>