Re: [PATCH] MPSAFE/LOOKUP_SHARED cd9660

On Thursday 20 November 2008 04:30:57 pm Paul B. Mahol wrote:
> On 11/19/08, John Baldwin <jhb_at_freebsd.org> wrote:
> > This is a relatively simple patch to mark cd9660 MPSAFE and enable shared
> > lookups.  The changes to cd9660_lookup() mirror similar changes to
> > ufs_lookup() to use static variables for local data rather than abusing
> > i-node members of the parent directory.  I've done some light testing of
> > this, but not super-strenuous.  This patch also includes simple locking 
for
> > the iconv support in the kernel.  That locking uses an sx lock to 
serialize
> > open and close of translator tables and the associated refcount.  Actual
> > conversions do not need any locks, however as the mount holds a reference 
on
> > the table.
> >
> > http://www.FreeBSD.org/~jhb/patches/cd9660_mpsafe.patch
> >
> 
> With this patch I'm unable to kldunload libiconv.ko once it is loaded.
> And trying to kldunload libiconv.ko will make any next 
kldload/kldstat/kldunload
> to fail waiting forever(livelock).
> 
> Regression were not encountered while only cd9660.ko were kldloaded.

So this is actually due to a bug in the module code.  If you have two modules 
like this:

DECLARE_MODULE(foo, SI_SUB_DRIVERS, SI_ORDER_FIRST);
DECLARE_MODULE(bar, SI_SUB_DRIVERS, SI_ORDER_SECOND);

The SI_* constants ensure that foo's module handler is called before bar's 
module handler for MOD_LOAD.  However, we don't enforce a reverse order (bar 
then foo) for MOD_UNLOAD.  In fact, the order of MOD_UNLOAD events is random 
and has no relation to the SI_* constants. :(

What is happening here is that one of the 'bar' modules in libiconv.ko is 
getting unloaded after 'foo' gets unloaded and using a destroyed lock (you 
get a panic if you run with INVARIANTS).

-- 
John Baldwin