Date: Fri, 5 Dec 2008 21:56:31 +0100 From: "Paul B. Mahol" <onemda@gmail.com> To: "John Baldwin" <jhb@freebsd.org> Cc: freebsd-current@freebsd.org Subject: Re: [PATCH] MPSAFE/LOOKUP_SHARED cd9660 Message-ID: <3a142e750812051256v1c0c4f3eke2f953b907a4653@mail.gmail.com> In-Reply-To: <200812051206.01927.jhb@freebsd.org> References: <200811191510.53793.jhb@FreeBSD.org> <3a142e750811201330p3084255em390d94b352dee532@mail.gmail.com> <200811201747.28540.jhb@freebsd.org> <200812051206.01927.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 12/5/08, John Baldwin <jhb@freebsd.org> wrote: > 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@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 > Yes it works, I tried hard multiple times kldload/kldunload {libiconv,cd9660,cd9660_iconv in various order} to livelock/panic it, but without success. FYI following LORs happened: lock order reversal: 1st 0xc4322ce8 isofs (isofs) @ /usr/src/sys/kern/vfs_lookup.c:442 2nd 0xd7d8d740 bufwait (bufwait) @ /usr/src/sys/kern/vfs_bio.c:2443 3rd 0xc4322bdc isofs (isofs) @ /usr/src/sys/modules/cd9660/../../fs/cd9660/cd9660_vfsops.c:694 KDB: stack backtrace: db_trace_self_wrapper(c061bff9,c3b566c0,c04e75e5,4,c0617724,...) at db_trace_self_wrapper+0x26 kdb_backtrace(4,c0617724,c3c91468,c3c93828,c3b5671c,...) at kdb_backtrace+0x29 _witness_debugger(c061ecc6,c4322bdc,c44e75c3,c3c93828,c44e7507,...) at _witness_debugger+0x25 witness_checkorder(c4322bdc,9,c44e7507,2b6,0,...) at witness_checkorder+0x839 __lockmgr_args(c4322bdc,80000,0,0,0,...) at __lockmgr_args+0x797 cd9660_vget_internal(c3feea00,d000,200000,c3b568a8,0,...) at cd9660_vget_internal+0x147 cd9660_lookup(c3b569f8,c4322c90,c3b56bb0,c4322c90,c3b56a18,...) at cd9660_lookup+0x4c1 VOP_CACHEDLOOKUP_APV(c44e8a20,c3b569f8,c3b56bb0,c3b56b9c,c3fd1b00,...) at VOP_CACHEDLOOKUP_APV+0xa5 vfs_cache_lookup(c3b56a78,c3b56a78,5000104,200000,c4322c90,...) at vfs_cache_lookup+0xcc VOP_LOOKUP_APV(c44e8a20,c3b56a78,c06249d7,1ba,c3b56b9c,...) at VOP_LOOKUP_APV+0xa5 lookup(c3b56b84,c06249d7,dc,bc,c410482c,...) at lookup+0x51e namei(c3b56b84,c108ba38,c108ba34,c3c5e000,c3b56b2c,...) at namei+0x48b kern_statat(c442d480,200,ffffff9c,282162f8,0,...) at kern_statat+0x6b kern_lstat(c442d480,282162f8,0,c3b56c18,0,...) at kern_lstat+0x36 lstat(c442d480,c3b56cf8,8,c061f959,c0647af0,...) at lstat+0x2f syscall(c3b56d38) at syscall+0x283 Xint0x80_syscall() at Xint0x80_syscall+0x20 --- syscall (190, FreeBSD ELF32, lstat), eip = 0x281ac413, esp = 0xbfbfe5ac, ebp = 0xbfbfe638 --- lock order reversal: 1st 0xc440fbdc ufs (ufs) @ /usr/src/sys/kern/vfs_mount.c:1207 2nd 0xc440fad0 devfs (devfs) @ /usr/src/sys/kern/vfs_subr.c:2179 KDB: stack backtrace: db_trace_self_wrapper(c061bff9,c3b50a38,c04e75e5,4,c0617724,...) at db_trace_self_wrapper+0x26 kdb_backtrace(4,c0617724,c3c934e8,c3c93418,c3b50a94,...) at kdb_backtrace+0x29 _witness_debugger(c061ecad,c440fad0,c0610530,c3c93418,c06252f3,...) at _witness_debugger+0x25 witness_checkorder(c440fad0,9,c06252f3,883,0,...) at witness_checkorder+0x839 __lockmgr_args(c440fad0,80100,c440faec,0,0,...) at __lockmgr_args+0x797 vop_stdlock(c3b50b9c,4,c0617724,80100,c440fa78,...) at vop_stdlock+0x62 VOP_LOCK1_APV(c0644760,c3b50b9c,c067fa18,c066aac0,c440fa78,...) at VOP_LOCK1_APV+0xa5 _vn_lock(c440fa78,80100,c06252f3,883,c44e7507,...) at _vn_lock+0x5e vrele(c440fa78,0,c44e7507,208,0,...) at vrele+0x142 cd9660_unmount(c3feea00,8000000,c416d240,4fc,0,...) at cd9660_unmount+0xed dounmount(c3feea00,8000000,c416d240,482,3,...) at dounmount+0x482 unmount(c416d240,c3b50cf8,8,c416d240,c0646b30,...) at unmount+0x2bf syscall(c3b50d38) at syscall+0x283 Xint0x80_syscall() at Xint0x80_syscall+0x20 --- syscall (22, FreeBSD ELF32, unmount), eip = 0x280d1e7f, esp = 0xbfbfe5bc, ebp = 0xbfbfe688 --- -- Paul
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3a142e750812051256v1c0c4f3eke2f953b907a4653>