From owner-svn-src-head@FreeBSD.ORG Mon Sep 13 17:30:33 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 CEF651065777; Mon, 13 Sep 2010 17:30:31 +0000 (UTC) (envelope-from alc@rice.edu) Received: from mh2.mail.rice.edu (mh2.mail.rice.edu [128.42.201.21]) by mx1.freebsd.org (Postfix) with ESMTP id 8177C8FC1D; Mon, 13 Sep 2010 17:30:31 +0000 (UTC) Received: from mh2.mail.rice.edu (localhost.localdomain [127.0.0.1]) by mh2.mail.rice.edu (Postfix) with ESMTP id 904A428F6FE; Mon, 13 Sep 2010 12:30:30 -0500 (CDT) X-Virus-Scanned: by amavis-2.6.4 at mh2.mail.rice.edu, auth channel Received: from mh2.mail.rice.edu ([127.0.0.1]) by mh2.mail.rice.edu (mh2.mail.rice.edu [127.0.0.1]) (amavis, port 10026) with ESMTP id k0nIjkv6JAHP; Mon, 13 Sep 2010 12:30:30 -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 mh2.mail.rice.edu (Postfix) with ESMTPSA id B4BE228F6D6; Mon, 13 Sep 2010 12:30:29 -0500 (CDT) Message-ID: <4C8E5FB5.9070009@rice.edu> Date: Mon, 13 Sep 2010 12:30:29 -0500 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100725) MIME-Version: 1.0 To: Kostik Belousov , Ryan Stone References: <201009070023.o870Njtg072607@svn.freebsd.org> <20100907080446.GA2853@deviant.kiev.zoral.com.ua> In-Reply-To: <20100907080446.GA2853@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, svn-src-all@freebsd.org, src-committers@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: Mon, 13 Sep 2010 17:30:34 -0000 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. Regards, Alan