Re: CVS commit: src/sys/uvm

[yamt%mwd.biglobe.ne.jp@localhost (YAMAMOTO Takashi)]

[ adding cc: tech-kern@ ]

hi,

> 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 guess matt meant "if the pointer to the vm_page is NULL,".

> 
> 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...

i guess your suggestion is too vague.
where do you want to use vm_physseg * + off_t instead of vm_page * ?
getpages, pmap_enter, and?  how their function prototypes would be?
any valid paddr_t value will belong to exactly one vm_phsseg?

YAMAMOTO Takashi