LOR with sysctl lock

[John Baldwin]

On Wednesday, November 24, 2010 9:36:27 pm Alexander Kabaev wrote:
> 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.

That's called an 'sx' lock and is what the linker uses.

-- 
John Baldwin