Re: CVS commit: src/sys/uvm

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

On Wed, Nov 24, 2010 at 11:26:39PM -0800, Matt Thomas wrote:
> 
> On Nov 24, 2010, at 10:47 PM, Masao Uebayashi wrote:
> 
> > 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...
> 
> Passing vm_page * doesn't work if the page isn't managed since there
> won't be a vm_page for the paddr_t.
> 
> Now passing both paddr_t and vm_page * would work and if the pointer
> to the vm_page, it would be an unmanaged mapping.  This also gives the
> access to mdpg without another lookup.

What if XIP'ed md(4), where physical pages are in .data (or .rodata)?

And don't forget that you're the one who first pointed out that
allocating vm_pages for XIP is a pure waste of memory. ;)

I'm allocating vm_pages, only because of phys_addr and loan_count.
I believe vm_pages is unnecessary for read-only XIP segments.
Because they're read-only, and stateless.

I've already concluded that the current "managed or not" model
doesn't work for XIP.  I'm pretty sure that my vm_physseg + off_t
model can explain everything.  I'm rather waiting for a counter
example how vm_physseg doesn't work...