Re: CVS commit: src/sys/uvm

[Matt Thomas <matt%3am-software.com@localhost>]

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.