From owner-freebsd-arch@FreeBSD.ORG Mon Nov 29 15:32:18 2010 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D800F106566C; Mon, 29 Nov 2010 15:32:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 97EFD8FC1A; Mon, 29 Nov 2010 15:32:18 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 3DA5546B06; Mon, 29 Nov 2010 10:32:18 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 3B1538A027; Mon, 29 Nov 2010 10:32:17 -0500 (EST) From: John Baldwin To: freebsd-arch@freebsd.org Date: Mon, 29 Nov 2010 08:00:51 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20101102; KDE/4.4.5; amd64; ; ) References: <20101124213627.2d95840f@kan.dnsalias.net> In-Reply-To: <20101124213627.2d95840f@kan.dnsalias.net> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201011290800.51498.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Mon, 29 Nov 2010 10:32:17 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: mdf@freebsd.org Subject: Re: LOR with sysctl lock X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Nov 2010 15:32:19 -0000 On Wednesday, November 24, 2010 9:36:27 pm Alexander Kabaev wrote: > On Wed, 24 Nov 2010 17:23:04 -0800 > mdf@FreeBSD.org wrote: > > > On Wed, Nov 24, 2010 at 3:49 PM, Alexander Kabaev > > wrote: > > > On Wed, 24 Nov 2010 11:53:58 -0800 > > > mdf@FreeBSD.org wrote: > > > > > >> The sysctl lock can cause "random" LOR warnings. These usually > > >> show on reboot/shutdown when sysctl_ctx_free() is called by a > > >> kernel module, since the mod handler is called with the module > > >> lock. The reason for the LOR is that, at least theoretically, the > > >> sysctl lock is the first lock in any hierarchy, because a > > >> SYSCTL_PROC handler can take any locks it wants, and will be > > >> called with the sysctl lock held shared. > > >> > > >> The below patch will fix the problem generically and with no > > >> changes to other code. I slightly prefer this to an explicit > > >> sysctl_ctx_free_sched(9), because many times code doesn't know if > > >> some caller holds *any* lock at all; this is especially true for > > >> mod handlers who shouldn't be expected to know how FreeBSD locks > > >> calls to the handler. > > >> > > >> I also note that the return value from sysctl_ctx_free(9) is almost > > >> never checked on CURRENT, and the only places it is, the value is > > >> merely used to print a warning. The only exception is > > >> canbus_detach() in pc98/pc98/canbus.c. So I wonder if > > >> sysctl_ctx_free(9) should return void and print a warning itself. > > >> > > >> Patch: > > >> http://people.freebsd.org/~mdf/0001-Proposed-patch-to-fix-LORs-with- sysctl-lock.patch > > >> > > >> If there are no objections, I'd like to commit this next week. > > >> > > >> Thanks, > > >> matthew > > > > > > Correct me if I am wrong, but doesn't this open a race where, say, > > > device detach routine destroys the device softc and schedules sysctl > > > context to be destroyed asynchronously via task queue? Since sysctl > > > entries are still visible in between the point where softc is > > > destroyed and the point where task queue picks the sysctl destroy > > > task up, can any access to said sysctls potentially operate on now > > > freed softc data? > > > > D'oh, yeah. > > > > I'm thinking of something a little more grand, now, that increments a > > hold count on the sysctl oid and releases the lock before calling the > > handler. This would prevent the LOR, and allow sysctl_ctx_free(9) to > > be run in-line, but it would then have to wait for any in progress > > sysctl calls to an oid to drain. > > > > I think this will work out; I'm planning to work on the code over the > > Thanksgiving holiday and have something ready in a few days. > > > > Thanks, > > matthew > > This sounds like a useful strategy, but I think you are attacking wrong > problem here. The linker running initialization code code under lock is > problematic and something like mutex to protect linker module lists and > some kind of gate with global flag allowing only one thread into module > load/unload code path should address the same annoying LOR as well and > possibly many more. That's called an 'sx' lock and is what the linker uses. -- John Baldwin