From owner-freebsd-current@FreeBSD.ORG Sun Dec 26 18:22:40 2004 Return-Path: Delivered-To: freebsd-current@hub.freebsd.org Received: by hub.freebsd.org (Postfix, from userid 680) id 7018216A4CF; Sun, 26 Dec 2004 18:22:40 +0000 (GMT) Date: Sun, 26 Dec 2004 18:22:40 +0000 From: Darren Reed To: keramida@freebsd.org Message-ID: <20041226182240.GA20920@hub.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.1i X-Mailman-Approved-At: Mon, 27 Dec 2004 13:07:45 +0000 cc: Scott Long cc: freebsd-current@hub.freebsd.org cc: Robert Watson Subject: witness found wanting (was Re: LORs in ipfilter) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 26 Dec 2004 18:22:40 -0000 In some mail from Giorgos Keramidas, sie said: > The locking changes of ipfilter have introduced a few LORs, which became > visible right after the build fixes committed by Scott. Here are the > ones I've seen so far. > > : lock order reversal > : 1st 0xc072d0a0 ifnet (ifnet) @ /usr/src/sys/contrib/ipfilter/netinet/fil.c:2146 > : 2nd 0xc06f9380 ipf IP NAT rwlock (ipf IP NAT rwlock) @ /usr/src/sys/contrib/ipfilter/netinet/ip_nat.c:2836 > : KDB: stack backtrace: > : kdb_backtrace(0,ffffffff,c0708df8,c07083f8,c06d9aac) at kdb_backtrace+0x29 > : witness_checkorder(c06f9380,9,c0676e6c,b14) at witness_checkorder+0x54c > : _sx_xlock(c06f9380,c0676e6c,b14,3,c1e9a000) at _sx_xlock+0x50 > : ip_natsync(c1e9a000,0,d95f9c84,c0448dd9,0) at ip_natsync+0x20 > : frsync(0,c04f7994,c1d55fac,0,c068949f) at frsync+0x2e > : iplioctl(c1e98b00,80047249,c1fa09e0,3,c1fba450) at iplioctl+0x551 > : devfs_ioctl_f(c1ff1d48,80047249,c1fa09e0,c1d67d80,c1fba450) at devfs_ioctl_f+0x87 > : ioctl(c1fba450,d95f9d14,3,1,246) at ioctl+0x370 > : syscall(2f,2f,2f,280556c0,bfbfeed4) at syscall+0x213 > : Xint0x80_syscall() at Xint0x80_syscall+0x1f > : --- syscall (54, FreeBSD ELF32, ioctl), eip = 0x280c67e7, esp = 0xbfbfea7c, ebp = 0xbfbfea98 --- What exactly is it complaining about ? The first two LORs in your email are similar - threads that start in a system call, filter through to the ipfilter code that acquires an sx lock after the system has acquireqd a mutex. In neither case does the mutex provide any protection against what is being achieved by getting the sx lock. If there is a bug here, it is in the assumptions that are built into the "witness code" checking of how locks can be used together. It may be that these need to be "blessed" (and you use the BLESSED option) because this behaviour is absolutely required and is not a "bug" or flaw. What is strange, I find, is that with witness turned on, you have not reported a panic for the recusrive acquiring of the ifnet lock. I suppose we're still approaching the point where it is safe to do that sort of recusive checking on mutexes ? > Converting the sx locks used by ipfilter to mutexes fixed these LORs but > introduced a new one, which I'm not sure how to fix: Which is surprising because it doesn't really fix the problem, in my eyes, it just highlights the above weakness/bug in the witness code. It also seriously degrades the performance of ipfilter on an smp system. That this sort of change makes the witness code silent is further evidence that it is not really doing its job. If you like, this is a classic case of bottom/top half driver locking where shared locks are used, instead of mutexes, to provide the requried synchronisation. In summary, what you've reported is not a problem with ipfilter code or use of locks, as far as I can tell. I could go on... Darren