From owner-freebsd-mips@FreeBSD.ORG Thu Aug 12 07:28:41 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 B6C181065675 for ; Thu, 12 Aug 2010 07:28:41 +0000 (UTC) (envelope-from c.jayachandran@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 46FAF8FC13 for ; Thu, 12 Aug 2010 07:28:40 +0000 (UTC) Received: by wyj26 with SMTP id 26so1349320wyj.13 for ; Thu, 12 Aug 2010 00:28:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=j+YMVA+KQZxkSYdvnquQEEUrx51JRxYcwuuW58s2FHw=; b=Lf/zwveovyLmgPJxlVjvq2p3I4XmJp206OalAe8kv6xGF46hpo0V6G/JnpMzwT/Po/ vTmzgsg++HEKRkSg1hbc1K+Qk2Y5r8GHDGPT0z+CEDFmpnxtIu4nAli5jf+p0gSCGxFh vR26wFgqTyyTemlm0INVTbGLKFufdYFuSrpPc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=Oe1DIcNOilUZH3ZxpXUSWYxORwOA3OqGAJE5Ya95/AEzIlXIeTJJLrxalChPJwYB7d GOFz+2SwvD4ydFx+sAbL0PWleE9phN7ht+30TOH6ESN6VRn5TKSqlNuO4JieYmC/+APm 8kjmpe+yBxBgt1Fhx1X/EGkXQ6/x7hiVulgUs= MIME-Version: 1.0 Received: by 10.216.178.130 with SMTP id f2mr13181432wem.101.1281598119860; Thu, 12 Aug 2010 00:28:39 -0700 (PDT) Received: by 10.216.160.10 with HTTP; Thu, 12 Aug 2010 00:28:39 -0700 (PDT) In-Reply-To: <4C639D85.9050000@cs.rice.edu> References: <201008041412.o74ECAix092415@svn.freebsd.org> <4C5A569B.9090401@cs.rice.edu> <4C5BA088.7060105@cs.rice.edu> <4C5C3A08.500@cs.rice.edu> <4C639D85.9050000@cs.rice.edu> Date: Thu, 12 Aug 2010 12:58:39 +0530 Message-ID: From: "Jayachandran C." To: Alan Cox Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: 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:28:41 -0000 On Thu, Aug 12, 2010 at 12:36 PM, Alan Cox wrote: > 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 encoura= ge >>> you to look at two things: >>> >>> 1. trap.c contains two copies of the same code for emulation. =A0I woul= d >>> encourage you to eliminate this duplication by creating a >>> pmap_emulate_modified(). >>> >>> 2. Software dirty bit emulation is using pmap_update_page() to invalida= te >>> the TLB entry on which the modified bit is being set. =A0On a >>> multiprocessor, >>> this is going to make dirty bit emulation very costly because every >>> processor will be interrupted. =A0In principle, it should be possible a= nd >>> faster to only flush the TLB entry from the current processor. =A0The o= ther >>> processors can handle this lazily. =A0They either do not have that mapp= ing >>> in >>> their TLB, in which case interrupting them was wasted effort, or they d= o >>> have it in their TLB and when they fault on it they'll discover the dir= ty >>> bit is already set. =A0In fact, the emulation code already handles this >>> case, >>> on account of the fact that two processors could simultaneously write t= o >>> 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. =A0The 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. =A0style(9) requires a blank line after the opening= "{" > here: > > +static __inline void > +pmap_invalidate_all_local(pmap_t pmap) > +{ > + =A0 =A0 =A0 if (pmap =3D=3D kernel_pmap) { > > (There is also an extra blank line after the above "if" statement that co= uld > be deleted.) Will fix this. >> 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. I missed this part, thanks. Will check-in with the changes. JC.