From owner-freebsd-mips@FreeBSD.ORG Fri Aug 6 16:36:36 2010 Return-Path: Delivered-To: mips@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 82947106566C; Fri, 6 Aug 2010 16:36:36 +0000 (UTC) (envelope-from alc@cs.rice.edu) Received: from mail.cs.rice.edu (mail.cs.rice.edu [128.42.1.31]) by mx1.freebsd.org (Postfix) with ESMTP id 48E428FC1F; Fri, 6 Aug 2010 16:36:35 +0000 (UTC) Received: from mail.cs.rice.edu (localhost.localdomain [127.0.0.1]) by mail.cs.rice.edu (Postfix) with ESMTP id 5B9CC2C2C56; Fri, 6 Aug 2010 11:36:35 -0500 (CDT) X-Virus-Scanned: by amavis-2.4.0 at mail.cs.rice.edu Received: from mail.cs.rice.edu ([127.0.0.1]) by mail.cs.rice.edu (mail.cs.rice.edu [127.0.0.1]) (amavisd-new, port 10024) with LMTP id iI2Nv6s-w5yj; Fri, 6 Aug 2010 11:36:27 -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 DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.cs.rice.edu (Postfix) with ESMTP id 1FF372C2C54; Fri, 6 Aug 2010 11:36:27 -0500 (CDT) Message-ID: <4C5C3A08.500@cs.rice.edu> Date: Fri, 06 Aug 2010 11:36:24 -0500 From: Alan Cox User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.9.1.11) Gecko/20100805 Thunderbird/3.0.6 MIME-Version: 1.0 To: "Jayachandran C." References: <201008041412.o74ECAix092415@svn.freebsd.org> <4C5A569B.9090401@cs.rice.edu> <4C5BA088.7060105@cs.rice.edu> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "Jayachandran C." , Alan Cox , mips@freebsd.org Subject: Re: svn commit: r210846 - in head/sys/mips: include mips X-BeenThere: freebsd-mips@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to MIPS List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Aug 2010 16:36:36 -0000 On 08/06/2010 05:29, Jayachandran C. wrote: > On Fri, Aug 6, 2010 at 11:11 AM, Alan Cox wrote: > >> On 08/05/2010 09:25, Jayachandran C. wrote: >> >>> On Thu, Aug 5, 2010 at 4:26 PM, Jayachandran C. >>> wrote: >>> >>> >>>> On Thu, Aug 5, 2010 at 11:43 AM, Alan Cox wrote: >>>> >>>> >>>>> Just an observation ... >>>>> >>>>> Jayachandran C. wrote: >>>>> >>>>> >>>>>> Author: jchandra >>>>>> Date: Wed Aug 4 14:12:09 2010 >>>>>> New Revision: 210846 >>>>>> URL: http://svn.freebsd.org/changeset/base/210846 >>>>>> >>>>>> Log: >>>>>> Add 3 level page tables for MIPS in n64. >>>>>> - 32 bit compilation will still use old 2 level page tables >>>>>> - re-arrange pmap code so that adding another level is easier >>>>>> - pmap code for 3 level page tables for n64 >>>>>> - update TLB handler to traverse 3 levels in n64 >>>>>> Reviewed by: jmallett >>>>>> >>>>>> >>>>> MIPS doesn't really need to use atomic_cmpset_int() in situations like >>>>> this >>>>> because the software dirty bit emulation in trap.c acquires the pmap >>>>> lock. >>>>> Atomics like this appear to be a carryover from i386 where the >>>>> hardware-managed TLB might concurrently set the modified bit. >>>>> >>>>> >>>> Then I guess we should be able to use *pte directly, without pbits, >>>> obits and the retry loop. >>>> Will try this change... >>>> >>>> >>> Can you have a look at the attached patch and see if it is okay? This >>> has the above changes, and I have attempted to fix the other issue you >>> had reported on wired mapping count too. >>> >>> >> The patch looks good. >> >> >>> There are a few calls for loadandclear() on pte too, with pmap lock >>> held, can this be avoided too? >>> >>> >> I haven't looked at them, but almost certainly yes. >> > The calls are in pmap_remove_pte(), pmap_remove_all() and > get_pv_entry(). the attached patch changes it normal pointer > operations. Thought I would post it for review before checking in... > The patch looks good. While we're talking about software dirty bit emulation, I would encourage you to look at two things: 1. trap.c contains two copies of the same code for emulation. I would encourage you to eliminate this duplication by creating a pmap_emulate_modified(). 2. Software dirty bit emulation is using pmap_update_page() to invalidate the TLB entry on which the modified bit is being set. On a multiprocessor, this is going to make dirty bit emulation very costly because every processor will be interrupted. In principle, it should be possible and faster to only flush the TLB entry from the current processor. The other processors can handle this lazily. They either do not have that mapping in their TLB, in which case interrupting them was wasted effort, or they do have it in their TLB and when they fault on it they'll discover the dirty bit is already set. In fact, the emulation code already handles this case, on account of the fact that two processors could simultaneously write to the same clean page and only one will get the pmap lock first.