Re: [PATCH] MPSAFE/LOOKUP_SHARED cd9660

On Thursday 20 November 2008 05:47:28 pm John Baldwin wrote:
> 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).

So this should now be fixed with this commit.  If you could verify that iconv 
works ok with the latest kern_module.c I would appreciate it.

Author: jhb
Date: Fri Dec  5 16:47:30 2008
New Revision: 185642
URL: http://svn.freebsd.org/changeset/base/185642

Log:
  When the SYSINIT() to load a module invokes the MOD_LOAD event successfully,
  move that module to the head of the associated linker file's list of 
modules.
  The end result is that once all the modules are loaded, they are sorted in
  the reverse of their load order.  This causes the kernel linker to invoke
  the MOD_QUIESCE and MOD_UNLOAD events in the reverse of the order that
  MOD_LOAD was invoked.  This means that the ordering of MOD_LOAD events that
  is set by the SI_* paramters to DECLARE_MODULE() are now honored in the same
  order they would be for SYSUNINIT() for the MOD_QUIESCE and MOD_UNLOAD
  events.
  
  MFC after:    1 month

-- 
John Baldwin