From owner-freebsd-mips@FreeBSD.ORG Thu Aug 12 07:06:55 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 E0B2C106567C; Thu, 12 Aug 2010 07:06:55 +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 A744B8FC15; Thu, 12 Aug 2010 07:06:55 +0000 (UTC) Received: from mail.cs.rice.edu (localhost.localdomain [127.0.0.1]) by mail.cs.rice.edu (Postfix) with ESMTP id CA1972C2B0E; Thu, 12 Aug 2010 02:06:54 -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 FSRg31EETRL5; Thu, 12 Aug 2010 02:06:47 -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 B7EAA2C2A63; Thu, 12 Aug 2010 02:06:46 -0500 (CDT) Message-ID: <4C639D85.9050000@cs.rice.edu> Date: Thu, 12 Aug 2010 02:06:45 -0500 From: Alan Cox User-Agent: Thunderbird 2.0.0.24 (X11/20100725) MIME-Version: 1.0 To: "Jayachandran C." References: <201008041412.o74ECAix092415@svn.freebsd.org> <4C5A569B.9090401@cs.rice.edu> <4C5BA088.7060105@cs.rice.edu> <4C5C3A08.500@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: Thu, 12 Aug 2010 07:06:56 -0000 Jayachandran C. wrote: > On Fri, Aug 6, 2010 at 10:06 PM, Alan Cox wrote: > >> 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. >> > > I've made the changes suggested, the changes are attached. > > The first set of changes just re-arranges the pmap calls that use > smp_rendezvous() on SMP, so that their per-cpu variants are also > available to be called. The first patch also has an optimization from > Juli's branch, to call pmap_update_page in pmap_kenter only if the pte > is valid. > > The patch looks good. style(9) requires a blank line after the opening "{" here: +static __inline void +pmap_invalidate_all_local(pmap_t pmap) +{ + if (pmap == kernel_pmap) { (There is also an extra blank line after the above "if" statement that could be deleted.) > The second patch makes the changes suggested above. My testing shows > no issues so far, but please let me know if you have any comments. > I believe that you can now make pmap_update_page() static and delete the declaration of pmap_set_modified() from pmap.h. Alan