From owner-freebsd-arch@FreeBSD.ORG Thu Nov 25 01:23:05 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 7FD07106566C for ; Thu, 25 Nov 2010 01:23:05 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id 43A998FC16 for ; Thu, 25 Nov 2010 01:23:04 +0000 (UTC) Received: by iwn39 with SMTP id 39so436064iwn.13 for ; Wed, 24 Nov 2010 17:23:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=OdXdCikEaabJPzC+cJO3+NSluQ24Xq/5iep+xKRd09E=; b=T4hJzwWgYhopvNXjMkXhwIkHsKEyFxS9fU1rLMXbsT5Kcaal0z5DZxRrwPJzdhfD0Z cHL4Q1MCzRUirP+jvitGJ7ECQfclPGM7OjN6r6hA83LpWJQCGMzhCXh5em0TnryIstKd atzLU2veHwkvbxFRiXTCrq/i7HomxoDkikbSc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=CBal6IE+T0VH4sIbzMTQoHlPRODu5ASkbqSeR654cPvJCcXFLOM+RZZRP6X4vH7Mc+ A/1xR+UNY5hlsTt5hrjWOrzeVg864Rkxeoia/oZxXYonxzm6YSDyBa0Y0rSK+Vr9c7sP cSpDgGCNdVN2aZ9JU/nPk1Uxqqv/jJqkrBBhs= MIME-Version: 1.0 Received: by 10.231.35.203 with SMTP id q11mr54853ibd.197.1290648184281; Wed, 24 Nov 2010 17:23:04 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.231.21.35 with HTTP; Wed, 24 Nov 2010 17:23:04 -0800 (PST) In-Reply-To: <20101124184934.35b6766a@kan.dnsalias.net> References: <20101124184934.35b6766a@kan.dnsalias.net> Date: Wed, 24 Nov 2010 17:23:04 -0800 X-Google-Sender-Auth: pr-Aq6q0njrAkPeRC2ezXNRxlyw Message-ID: From: mdf@FreeBSD.org To: Alexander Kabaev Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-arch@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: Thu, 25 Nov 2010 01:23:05 -0000 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. =A0These 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. =A0The >> 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. =A0I 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. =A0The only exception is canbus_detach() >> in pc98/pc98/canbus.c. =A0So 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-sysc= tl-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