From owner-svn-src-head@FreeBSD.ORG Tue Sep 14 06:48:14 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 725F7106566C; Tue, 14 Sep 2010 06:48:14 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mh1.mail.rice.edu (mh1.mail.rice.edu [128.42.201.20]) by mx1.freebsd.org (Postfix) with ESMTP id 397448FC1B; Tue, 14 Sep 2010 06:48:13 +0000 (UTC) Received: from mh1.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh1.mail.rice.edu (Postfix) with ESMTP id 77B1828F6BA; Tue, 14 Sep 2010 01:48:13 -0500 (CDT) X-Virus-Scanned: by amavis-2.6.4 at mh1.mail.rice.edu, auth channel Received: from mh1.mail.rice.edu ([127.0.0.1]) by mh1.mail.rice.edu (mh1.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id sruk1lgL2Kq0; Tue, 14 Sep 2010 01:48:13 -0500 (CDT) Received: from adsl-216-63-78-18.dsl.hstntx.swbell.net (adsl-216-63-78-18.dsl.hstntx.swbell.net [216.63.78.18]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh1.mail.rice.edu (Postfix) with ESMTPSA id C242228F6B7; Tue, 14 Sep 2010 01:48:12 -0500 (CDT) Message-ID: <4C8F1AAC.4000301@rice.edu> Date: Tue, 14 Sep 2010 01:48:12 -0500 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100725) MIME-Version: 1.0 To: Kostik Belousov References: <201009070023.o870Njtg072607@svn.freebsd.org> <20100907080446.GA2853@deviant.kiev.zoral.com.ua> <4C8E5FB5.9070009@rice.edu> <20100913191247.GN2465@deviant.kiev.zoral.com.ua> In-Reply-To: <20100913191247.GN2465@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Sep 2010 06:48:14 -0000 Kostik Belousov wrote: > 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: >>> >>> >>>> 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 >>>> 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 >>>> 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. >>>> >>>> Without this change I could cause a deadlock on a multicore 8.1-RELEASE >>>> 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 >>>> pmcstat >>>> in system-wide sampling mode. >>>> >>>> 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 >>>> ============================================================================== >>>> --- 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 = (uintptr_t) NULL; >>>> if (vm_map_lookup_entry(map, addr, &entry)) { >>>> for (; >>>> entry != &map->header && entry->start < addr + size; >>>> @@ -587,16 +588,23 @@ munmap(td, uap) >>>> entry->end, VM_PROT_EXECUTE) == TRUE) { >>>> pkm.pm_address = (uintptr_t) addr; >>>> pkm.pm_size = (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 != (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); >>>> } >>>> >>>> >>>> >>> 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. >>> >>> >>> >> I'm afraid that this understates the effect. Over the weekend, when I >> updated one of my amd64 machines to include this change, I found that >> the delay in object and page deallocation is leading to severe >> fragmentation within the physical memory allocator. As a result, the >> time spent in the kernel during a "buildworld" increased by about 8%. >> Moreover, superpage promotion by applications effectively stopped. >> >> For now, I think it would be best to back out r212281 and r212282. >> Ultimately, the fix may be to change the vm map synchronization >> primitives, and simply reinstate r212281 and r212282, but I'd like some >> 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. > > Yes, I saw it. I don't understand it either. Alan .