From owner-cvs-all@FreeBSD.ORG Thu Sep 22 10:11:38 2005 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9ED9416A41F; Thu, 22 Sep 2005 10:11:38 +0000 (GMT) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [204.156.12.53]) by mx1.FreeBSD.org (Postfix) with ESMTP id 2030043D48; Thu, 22 Sep 2005 10:11:38 +0000 (GMT) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by cyrus.watson.org (Postfix) with ESMTP id 10C5C46B4C; Thu, 22 Sep 2005 06:11:37 -0400 (EDT) Date: Thu, 22 Sep 2005 11:11:36 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: "M. Warner Losh" In-Reply-To: <20050921.130241.102576086.imp@bsdimp.com> Message-ID: <20050922111002.N34322@fledge.watson.org> References: <20050920223315.V34322@fledge.watson.org> <20050921154153.GB22964@ip.net.ua> <200509211455.59154.jhb@FreeBSD.org> <20050921.130241.102576086.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed 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 ... X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2005 10:11:39 -0000 On Wed, 21 Sep 2005, M. Warner Losh wrote: > In message: <200509211455.59154.jhb@FreeBSD.org> > John Baldwin 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