Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 22 Sep 2005 11:11:36 +0100 (BST)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        "M. Warner Losh" <imp@bsdimp.com>
Cc:        cvs-src@FreeBSD.org, src-committers@FreeBSD.org, ru@FreeBSD.org, cvs-all@FreeBSD.org, jhb@FreeBSD.org
Subject:   Re: cvs commit: src/sys/dev/an if_an.c src/sys/dev/arl if_arl_isa.c src/sys/dev/awi if_awi_pccard.c src/sys/dev/cm if_cm_isa.c src/sys/dev/cnw if_cnw.c src/sys/dev/cp if_cp.c src/sys/dev/cs if_cs.c src/sys/dev/ed if_ed.c src/sys/dev/em if_em.c ...
Message-ID:  <20050922111002.N34322@fledge.watson.org>
In-Reply-To: <20050921.130241.102576086.imp@bsdimp.com>
References:  <20050920223315.V34322@fledge.watson.org> <20050921154153.GB22964@ip.net.ua> <200509211455.59154.jhb@FreeBSD.org> <20050921.130241.102576086.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help

On Wed, 21 Sep 2005, M. Warner Losh wrote:

> In message: <200509211455.59154.jhb@FreeBSD.org>
>            John Baldwin <jhb@FreeBSD.org> writes:
> : > 5.  Fix all drivers to set some flag in foo_detach() and foo_shutdown()
> : >     and refuse to work in foo_ioctl() if it's set.  This should fix
> : >     panics when BPF listener is attached while interface goes away or
> : >     module is unloaded.
>
> : I'd rather 5) be simpler in
> : that it only check in the flags case to not force the driver lock to be
> : acquired for all the ioctls that the driver doesn't actually handle.
> : Actually, I think I'd really prefer that we think about how to fix the BPF
> : issue in BPF itself if possible.  It may be that we don't need to set the
> : flags (i.e. skip the actual ioctl) if the interface is in the process of
> : detaching and we can make that change centrally without having to scatter
> : gone flags in all the drivers.
>
> This would solve the race at hand.  However, it wouldn't solve the 
> problems with driver shutdown racing with other things in the system 
> (like ifconfig during detach).
>
> I've knocked around the idea of creating a if_dead() function that one 
> would call before foo_stop().  if_dead would just return (possibly an 
> error) for all the entry points before the device is actually detached. 
> This would mean that we don't need to add flags to all the drivers, but 
> we do need to change all the detach routine.  This would avoid the LOCK 
> operations that you are worried about...

Would if_dead simply replace the function vector, or would it also drain 
threads currently in those functions?  We have a pervasive problem with 
dead functions that fail to drain (phk has fixed this up for cdev, I 
believe, and colin for callouts, but there are many others), and without 
that there may be problems.  However, if adding draining and refcounting 
on entering ifnet methods, we need to be very cautious about the 
performance impact.

Robert N M Watson



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