Re: CVS commit: src/sys/uvm

[Masao Uebayashi <uebayasi%tombi.co.jp@localhost>]

On Thu, Nov 25, 2010 at 05:44:21AM +0000, YAMAMOTO Takashi wrote:
> hi,
> 
> > On Thu, Nov 25, 2010 at 04:18:25AM +0000, YAMAMOTO Takashi wrote:
> >> hi,
> >> 
> >> > Hi, thanks for review.
> >> > 
> >> > On Thu, Nov 25, 2010 at 01:58:04AM +0000, YAMAMOTO Takashi wrote:
> >> >> hi,
> >> >> 
> >> >> - what's VM_PHYSSEG_OP_PG?
> >> > 
> >> > It's to lookup vm_physseg by "struct vm_page *", relying on that
> >> > "struct vm_page *[]" is allocated linearly.  It'll be used to remove
> >> > vm_page::phys_addr as we talked some time ago.
> >> 
> >> i'm not sure if commiting this unused uncommented code now helps it.
> >> some try-and-benchmark cycles might be necessary given that
> >> vm_page <-> paddr conversion could be performace critical.
> > 
> > If you really care performance, we can directly pass "struct vm_page
> > *" to pmap_enter().
> > 
> > We're doing "struct vm_page *" -> "paddr_t" just before pmap_enter(),
> > then doing "paddr_t" -> "vm_physseg" reverse lookup again in
> > pmap_enter() to check if a given PA is managed.  What is really
> > needed here is, to lookup "struct vm_page *" -> "vm_physseg" once
> > and you'll know both paddr_t and managed or not.
> 
> i agree that the current code is not ideal in that respect.
> otoh, i'm not sure if passing vm_physseg around is a good idea.

It's great you share the interest.

I chose vm_physseg, because it was there.  I'm open to alternatives,
but I don't think you have many options...