Date: Fri, 06 Aug 2010 11:36:24 -0500 From: Alan Cox <alc@cs.rice.edu> To: "Jayachandran C." <c.jayachandran@gmail.com> Cc: "Jayachandran C." <jchandra@freebsd.org>, Alan Cox <alc@cs.rice.edu>, mips@freebsd.org Subject: Re: svn commit: r210846 - in head/sys/mips: include mips Message-ID: <4C5C3A08.500@cs.rice.edu> In-Reply-To: <AANLkTinxAkRTK8pLRkQ7JwesNkuwmuiRevOZMDpj_aj7@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On 08/06/2010 05:29, Jayachandran C. wrote: > On Fri, Aug 6, 2010 at 11:11 AM, Alan Cox<alc@cs.rice.edu> wrote: > >> On 08/05/2010 09:25, Jayachandran C. wrote: >> >>> On Thu, Aug 5, 2010 at 4:26 PM, Jayachandran C. >>> <c.jayachandran@gmail.com> wrote: >>> >>> >>>> On Thu, Aug 5, 2010 at 11:43 AM, Alan Cox<alc@cs.rice.edu> 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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C5C3A08.500>