From owner-freebsd-current@FreeBSD.ORG Tue Dec 9 23:56:05 2008 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A11C61065672 for ; Tue, 9 Dec 2008 23:56:05 +0000 (UTC) (envelope-from onemda@gmail.com) Received: from mail-gx0-f18.google.com (mail-gx0-f18.google.com [209.85.217.18]) by mx1.freebsd.org (Postfix) with ESMTP id 359258FC1E for ; Tue, 9 Dec 2008 23:56:05 +0000 (UTC) (envelope-from onemda@gmail.com) Received: by gxk11 with SMTP id 11so431182gxk.12 for ; Tue, 09 Dec 2008 15:56:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:cc:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=iPY+lnazi89JzCuetPFbsLm2aLFzALR1Rox6BtCJtPo=; b=aM0I47i3KjMfWe1ccKvcblRdVdYwCgYFD7K0ob3lkPKz5vZUr5vmdbeITPtdv3/Ru/ Qib5GA0ieSk3TYmyx3euCQGdFWk/AKysDDIfbyAdlL9q3c2ef/wj+8ZhxExZLiHZim0t zIFCvtD21stDXlgO96usAzj81cahhtx8AjRJc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=Vosxedzv29dFEsBqGnW5Nm4rCpCdQkjV7ZqeTXWjVQt1pJhAAq2ORs6X+JLZvCqIC7 gIFMXKI2N9hZ3eXiKznmlvPipzLKjMkaLUcJVzsejyVJcFAcBNJ94G088cKZLmwBxrlK iuZW0udZojvptE2Q5LwIH9bQXsKPYrIpGNViM= Received: by 10.231.15.73 with SMTP id j9mr14055iba.35.1228866964105; Tue, 09 Dec 2008 15:56:04 -0800 (PST) Received: by 10.231.15.194 with HTTP; Tue, 9 Dec 2008 15:56:04 -0800 (PST) Message-ID: <3a142e750812091556x682008dagb8e1e867d0bdca79@mail.gmail.com> Date: Wed, 10 Dec 2008 00:56:04 +0100 From: "Paul B. Mahol" To: "John Baldwin" In-Reply-To: <3a142e750812091516n6bb0880ewb298314e9ba296c@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <200811191510.53793.jhb@FreeBSD.org> <200812051608.50120.jhb@freebsd.org> <3a142e750812051354n747bcbcayb31d8d5f4cc15098@mail.gmail.com> <200812091602.10850.jhb@freebsd.org> <3a142e750812091515u3f2e5807j3652f2cc3551a3b2@mail.gmail.com> <3a142e750812091516n6bb0880ewb298314e9ba296c@mail.gmail.com> Cc: freebsd-current@freebsd.org Subject: Re: [PATCH] MPSAFE/LOOKUP_SHARED cd9660 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Dec 2008 23:56:05 -0000 On 12/10/08, Paul B. Mahol wrote: > On 12/10/08, Paul B. Mahol wrote: >> On 12/9/08, John Baldwin wrote: >>> On Friday 05 December 2008 04:54:23 pm Paul B. Mahol wrote: >>>> On 12/5/08, John Baldwin wrote: >>>> > On Friday 05 December 2008 03:56:31 pm Paul B. Mahol wrote: >>>> >> On 12/5/08, John Baldwin 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 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 >>>> > >>>> > This LOR should be addressed in the latest cd9660 locking patches. >>>> > >>>> > -- >>>> > John Baldwin >>>> > >>>> >>>> Oh, why I did not checked new version? >>>> >>>> Yes that LOR have gone, but when doing "ll -R" first time on /mnt >>>> I got following messages from kernel: >>>> >>>> RRIP without PX field? x ~ 50 times. >>>> >>>> I see you changed LK_EXCLUSIVE to flags, and with MPSAFE .... >>> >>> The RRIP stuff is all done in cd9660_vget_internal() under an exclusive >>> lock. >>> It could be a property of the ISO image. "PX" holds permissions (owner, >>> etc.). Do you get the same messages w/o the patch with the same ISO >>> image >>> / >>> CD? >>> >>> -- >>> John Baldwin >>> >> >> No I do not, but I may try other CDs I have many of them, including >> FreeBSD FreeBSD snapshot CD have same "issue". It doesnt appear always (I mean I do not count vfs cache) maybe disabling SMP and PREEMPTION will silent it. > s/Yes I do. Its to late here ... > >> one. >> If it is irrelevant than it should not be displayed. -- Paul