Date: Thu, 12 Aug 2010 12:58:39 +0530 From: "Jayachandran C." <c.jayachandran@gmail.com> To: Alan Cox <alc@cs.rice.edu> Cc: mips@freebsd.org Subject: Re: svn commit: r210846 - in head/sys/mips: include mips Message-ID: <AANLkTinvjZbfwcqn6eArLZ3kNUcZN2sUDGmqcG5ELunB@mail.gmail.com> In-Reply-To: <4C639D85.9050000@cs.rice.edu> References: <201008041412.o74ECAix092415@svn.freebsd.org> <4C5A569B.9090401@cs.rice.edu> <AANLkTinP7eMNm4yp6T2TTteSvthdgLJOj-ihHrQJ4T49@mail.gmail.com> <AANLkTi=vkG-cntJYYEdhO4AzOO91LB6n%2B45dUSxCMTp3@mail.gmail.com> <4C5BA088.7060105@cs.rice.edu> <AANLkTinxAkRTK8pLRkQ7JwesNkuwmuiRevOZMDpj_aj7@mail.gmail.com> <4C5C3A08.500@cs.rice.edu> <AANLkTi=FowMNMy87aA2K=120a4L_Fd5GPDH%2BdEPKOsek@mail.gmail.com> <4C639D85.9050000@cs.rice.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Aug 12, 2010 at 12:36 PM, Alan Cox <alc@cs.rice.edu> wrote: > Jayachandran C. wrote: >> >> On Fri, Aug 6, 2010 at 10:06 PM, Alan Cox <alc@cs.rice.edu> 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTinvjZbfwcqn6eArLZ3kNUcZN2sUDGmqcG5ELunB>