From owner-freebsd-current@FreeBSD.ORG Thu Sep 1 02:09:17 2005 Return-Path: X-Original-To: freebsd-current@FreeBSD.org Delivered-To: freebsd-current@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A400E16A41F; Thu, 1 Sep 2005 02:09:17 +0000 (GMT) (envelope-from truckman@FreeBSD.org) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4C42543D46; Thu, 1 Sep 2005 02:09:17 +0000 (GMT) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.13.3/8.13.3) with ESMTP id j8128uvW019560; Wed, 31 Aug 2005 19:09:00 -0700 (PDT) (envelope-from truckman@FreeBSD.org) Message-Id: <200509010209.j8128uvW019560@gw.catspoiler.org> Date: Wed, 31 Aug 2005 19:08:56 -0700 (PDT) From: Don Lewis To: imp@bsdimp.com In-Reply-To: <20050827.220303.130848154.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii Cc: bzeeb-lists@lists.zabbadoz.net, freebsd-current@FreeBSD.org, rwatson@FreeBSD.org, dandee@volny.cz Subject: Re: LOR route vr0 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 01 Sep 2005 02:09:18 -0000 On 27 Aug, M. Warner Losh wrote: > In message: <20050828025721.X43518@fledge.watson.org> > Robert Watson writes: > : > : On Sat, 27 Aug 2005, M. Warner Losh wrote: > : > : > : You need to add an entry to subr_witness.c creating a graph edge between > : > : the softc lock and the routing lock. An example of an entry in > : > : subr_witness.c: > : > : > : > : /* > : > : * TCP/IP > : > : */ > : > : { "tcp", &lock_class_mtx_sleep }, > : > : { "tcpinp", &lock_class_mtx_sleep }, > : > : { "so_snd", &lock_class_mtx_sleep }, > : > : { NULL, NULL }, > : > : > : > : Note that sets of ordered entries are terminated with a double-null. This > : > : declares that locks of type "tcp" preceed "tcpinp" which preceed > : > : "so_snd". > : > > : > So you have to have locks of type tcp BEFORE you take out tcpinp type > : > locks? > : > : Correct. 'tcp' reflects the global TCP state tables (pcbinfo) locks, and > : 'tcpinp' is for individual PCBs. If you acquire first a tcpinp and then > : tcp, the above settings should cause WITNESS to generate a lock order > : warning. Likewise, both tcp and tcpinp preceed so_snd, so if you acquire > : a protocol lock after a socket lock, it will get unhappy. WITNESS handles > : transitive relationships, so it gets connected up to the rest of the lock > : graph, explicit and implicit, so indirect violations of orders are fully > : handled. > > OK. I've been seeing similar LORs in ed, sn, iwi (ed is my locked > version of ed, not in tree GIANT locked ed). Just as a datapoint, I've got fxp interfaces on all my machines running -CURRENT and I'm not seeing the problem here. > I've made the following changes, and the LORs go away (except for > one, which was unrelated). I further don't get the first place where > they locks happen that caused the original LORs, so I'm mightly > confused. What is the other LOR that you are seeing? Does it go away if you unwire the MTX_NETWORK_LOCK order? If so, that LOR is where witness is breaking the loop in the lock ordering graph. As jhb mentioned, the output of "show witness" would be interesting in the case where the lock orders are not wired. > ==== //depot/user/imp/newcard/kern/subr_witness.c#62 - /dell/imp/p4/newcard/src/sys/kern/subr_witness.c ==== > @@ -273,6 +273,13 @@ > { "allprison", &lock_class_mtx_sleep }, > { NULL, NULL }, > /* > + * Network driver locking order > + */ > + { "rawinp", &lock_class_mtx_sleep }, > + { MTX_NETWORK_LOCK, &lock_class_mtx_sleep }, > + { "if_addr_mtx", &lock_class_mtx_sleep }, > + { NULL, NULL }, > + /* > * Sockets > */ > { "filedesc structure", &lock_class_mtx_sleep }, > @@ -309,6 +316,7 @@ > { "udp", &lock_class_mtx_sleep }, > { "udpinp", &lock_class_mtx_sleep }, > { "so_snd", &lock_class_mtx_sleep }, > + { MTX_NETWORK_LOCK, &lock_class_mtx_sleep }, > { NULL, NULL }, > /* > * TCP/IP > @@ -316,6 +324,7 @@ > { "tcp", &lock_class_mtx_sleep }, > { "tcpinp", &lock_class_mtx_sleep }, > { "so_snd", &lock_class_mtx_sleep }, > + { MTX_NETWORK_LOCK, &lock_class_mtx_sleep }, > { NULL, NULL }, > /* > * SLIP > > I'm not sure if I need to add the if_addr_mtx after each thing or > not. Nope. You also don't need to add MTX_NETWORK_LOCK after so_snd more than once. If you are finding that you need to wire the order of if_addr_mtx, that is a potential clue. The only lock I see taken after if_addr_mtx is "UMA zone". If you are seeing other locks under if_addr_mtx, maybe one of those is looping back to rtentry. I also see taskqueue, "if send queue", and various memory subsystem locks under "network driver". Both taskqueue and "if send queue" appear to be leaf locks.