From owner-svn-src-all@FreeBSD.ORG Mon Sep 13 19:12:54 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8B1CB106567A; Mon, 13 Sep 2010 19:12:54 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id A512A8FC1C; Mon, 13 Sep 2010 19:12:53 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id o8DJClPn028197 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 13 Sep 2010 22:12:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id o8DJClTQ084253; Mon, 13 Sep 2010 22:12:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id o8DJClHE084252; Mon, 13 Sep 2010 22:12:47 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 13 Sep 2010 22:12:47 +0300 From: Kostik Belousov To: Alan Cox Message-ID: <20100913191247.GN2465@deviant.kiev.zoral.com.ua> References: <201009070023.o870Njtg072607@svn.freebsd.org> <20100907080446.GA2853@deviant.kiev.zoral.com.ua> <4C8E5FB5.9070009@rice.edu> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vRCs+Tj6pwiFrQdW" Content-Disposition: inline In-Reply-To: <4C8E5FB5.9070009@rice.edu> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.2 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-2.1 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_50, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: svn-src-head@freebsd.org, Ryan Stone , src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r212281 - head/sys/vm X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 13 Sep 2010 19:12:54 -0000 --vRCs+Tj6pwiFrQdW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 13, 2010 at 12:30:29PM -0500, Alan Cox wrote: > Kostik Belousov wrote: > >On Tue, Sep 07, 2010 at 12:23:45AM +0000, Ryan Stone wrote: > > =20 > >>Author: rstone > >>Date: Tue Sep 7 00:23:45 2010 > >>New Revision: 212281 > >>URL: http://svn.freebsd.org/changeset/base/212281 > >> > >>Log: > >> In munmap() downgrade the vm_map_lock to a read lock before taking a= =20 > >> read > >> lock on the pmc-sx lock. This prevents a deadlock with > >> pmc_log_process_mappings, which has an exclusive lock on pmc-sx and= =20 > >> tries > >> to get a read lock on a vm_map. Downgrading the vm_map_lock in munmap > >> allows pmc_log_process_mappings to continue, preventing the deadlock. > >> =20 > >> Without this change I could cause a deadlock on a multicore 8.1-RELEA= SE > >> system by having one thread constantly mmap'ing and then munmap'ing a > >> PROT_EXEC mapping in a loop while I repeatedly invoked and stopped=20 > >> pmcstat > >> in system-wide sampling mode. > >> =20 > >> Reviewed by: fabient > >> Approved by: emaste (mentor) > >> MFC after: 2 weeks > >> > >>Modified: > >> head/sys/vm/vm_mmap.c > >> > >>Modified: head/sys/vm/vm_mmap.c > >>=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > >>--- head/sys/vm/vm_mmap.c Mon Sep 6 23:52:04 2010 (r212280) > >>+++ head/sys/vm/vm_mmap.c Tue Sep 7 00:23:45 2010 (r212281) > >>@@ -579,6 +579,7 @@ munmap(td, uap) > >> * Inform hwpmc if the address range being unmapped contains > >> * an executable region. > >> */ > >>+ pkm.pm_address =3D (uintptr_t) NULL; > >> if (vm_map_lookup_entry(map, addr, &entry)) { > >> for (; > >> entry !=3D &map->header && entry->start < addr + size; > >>@@ -587,16 +588,23 @@ munmap(td, uap) > >> entry->end, VM_PROT_EXECUTE) =3D=3D TRUE) { > >> pkm.pm_address =3D (uintptr_t) addr; > >> pkm.pm_size =3D (size_t) size; > >>- PMC_CALL_HOOK(td, PMC_FN_MUNMAP, > >>- (void *) &pkm); > >> break; > >> } > >> } > >> } > >> #endif > >>- /* returns nothing but KERN_SUCCESS anyway */ > >> vm_map_delete(map, addr, addr + size); > >>+ > >>+#ifdef HWPMC_HOOKS > >>+ /* downgrade the lock to prevent a LOR with the pmc-sx lock */ > >>+ vm_map_lock_downgrade(map); > >>+ if (pkm.pm_address !=3D (uintptr) NULL) > >>+ PMC_CALL_HOOK(td, PMC_FN_MUNMAP, (void *) &pkm); > >>+ vm_map_unlock_read(map); > >>+#else > >> vm_map_unlock(map); > >>+#endif > >>+ /* vm_map_delete returns nothing but KERN_SUCCESS anyway */ > >> return (0); > >> } > >>=20 > >> =20 > >Note that vm_map_unlock() is more then just dropping the lock on the map. > >Due to ordering of the vnode lock before vm map lock, vm_map_unlock() > >processes the deferred free entries after map lock is dropped. After your > >change, the deferred list might keep entries for some time until next > >unlock is performed. > > > > =20 >=20 > I'm afraid that this understates the effect. Over the weekend, when I=20 > updated one of my amd64 machines to include this change, I found that=20 > the delay in object and page deallocation is leading to severe=20 > fragmentation within the physical memory allocator. As a result, the=20 > time spent in the kernel during a "buildworld" increased by about 8%. =20 > Moreover, superpage promotion by applications effectively stopped. >=20 > For now, I think it would be best to back out r212281 and r212282. =20 > Ultimately, the fix may be to change the vm map synchronization=20 > primitives, and simply reinstate r212281 and r212282, but I'd like some= =20 > time to consider the options. Did you noted the thread on current@ about r212281 ? The submitter claims that the rev. causes panics in unrelated code pathes when vnode_create_vobject() locks vm object lock. I cannot understand how this can happen, with or without the rev. More, when I suggested the following change, that is intended to minimize the window, the answer was that it makes the situation worse. diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c index 63dfb67..d13e488 100644 --- a/sys/vm/vm_mmap.c +++ b/sys/vm/vm_mmap.c @@ -597,13 +597,15 @@ munmap(td, uap) =20 #ifdef HWPMC_HOOKS /* downgrade the lock to prevent a LOR with the pmc-sx lock */ - vm_map_lock_downgrade(map); - if (pkm.pm_address !=3D (uintptr_t) NULL) - PMC_CALL_HOOK(td, PMC_FN_MUNMAP, (void *) &pkm); - vm_map_unlock_read(map); -#else - vm_map_unlock(map); + if (pkm.pm_address !=3D (uintptr_t)NULL) { + vm_map_lock_downgrade(map); + if (pkm.pm_address !=3D (uintptr_t)NULL) + PMC_CALL_HOOK(td, PMC_FN_MUNMAP, (void *)&pkm); + vm_map_unlock_read(map); + vm_map_lock(map); + } #endif + vm_map_unlock(map); /* vm_map_delete returns nothing but KERN_SUCCESS anyway */ return (0); } --vRCs+Tj6pwiFrQdW Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (FreeBSD) iEYEARECAAYFAkyOd68ACgkQC3+MBN1Mb4h4YACg5VBzgUZtyKq6f5N0W1fl4pik YpoAoOCtlCJjs5Lq+dkjEHegFUvZwEXl =wtoz -----END PGP SIGNATURE----- --vRCs+Tj6pwiFrQdW--