From owner-freebsd-current@FreeBSD.ORG Fri Dec 5 21:54:25 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 D5E661065676 for ; Fri, 5 Dec 2008 21:54:24 +0000 (UTC) (envelope-from onemda@gmail.com) Received: from an-out-0708.google.com (an-out-0708.google.com [209.85.132.243]) by mx1.freebsd.org (Postfix) with ESMTP id 8929B8FC13 for ; Fri, 5 Dec 2008 21:54:24 +0000 (UTC) (envelope-from onemda@gmail.com) Received: by an-out-0708.google.com with SMTP id b6so140856ana.13 for ; Fri, 05 Dec 2008 13:54:23 -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=5BZDcPSz9IJk8sMThnSYaJJTSKaFBqvYa58swrsHajo=; b=V8YctdA0WZluS0AXKlSIqlu/tv6kURCJtcaLNJlmPitGODNf6/MMZ/k0d7RwP9UEOM HvNUrbE2pK6dIy7BvIJ6BxJjOfNTi+1tQ17DOBKwN+kOT9pn4RMSwUBGJab240GNKhqz Et0H5ovaoXgSTYZFepX0hysKyBXKhPSemkViI= 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=T5K6zg8r9ipv3kanb93M8J47lO0ud/1ahng533kWTRcMj7tidSU+Gb4epx/eE4ll3I cRQZrpvjOPH5daP8UwHfMBOZaBybBFWhuAgA1fymMrYZPXpZDpPo4KNjcZGMLR0KnXJf EQqozPqUCbWsNmBWBDV6V+i+pNjwQOwkd2TIY= Received: by 10.231.20.2 with SMTP id d2mr4585ibb.27.1228514063379; Fri, 05 Dec 2008 13:54:23 -0800 (PST) Received: by 10.231.12.5 with HTTP; Fri, 5 Dec 2008 13:54:23 -0800 (PST) Message-ID: <3a142e750812051354n747bcbcayb31d8d5f4cc15098@mail.gmail.com> Date: Fri, 5 Dec 2008 22:54:23 +0100 From: "Paul B. Mahol" To: "John Baldwin" In-Reply-To: <200812051608.50120.jhb@freebsd.org> 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> <200812051206.01927.jhb@freebsd.org> <3a142e750812051256v1c0c4f3eke2f953b907a4653@mail.gmail.com> <200812051608.50120.jhb@freebsd.org> 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: Fri, 05 Dec 2008 21:54:25 -0000 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 .... -- Paul