LOR with sysctl lock

[Alexander Kabaev]

On Wed, 24 Nov 2010 17:23:04 -0800
mdf at FreeBSD.org wrote:

> On Wed, Nov 24, 2010 at 3:49 PM, Alexander Kabaev <kabaev at gmail.com>
> wrote:
> > On Wed, 24 Nov 2010 11:53:58 -0800
> > mdf at 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.

Having sysctl handlers running outside of sysctl lock is still a very
worthwhile goal IMHO.

-- 
Alexander Kabaev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20101125/58580c74/signature.pgp