Re: [PATCH] MPSAFE/LOOKUP_SHARED cd9660

On 11/20/08, John Baldwin <jhb_at_freebsd.org> 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. :(

Exactly, next time I tried it again and it did not livelock.
>
> 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).

Yes, I tested it on kernel without ddb and friends.