RE: [Xen-ia64-devel][PATCH][VTD] small patches for VTD

["Xu, Anthony" <anthony.xu@xxxxxxxxx>]

Isaku Yamahata wrote:
> On Wed, Oct 22, 2008 at 02:30:50PM +0800, Xu, Anthony wrote:
>> Isaku Yamahata wrote:
>>> On Wed, Oct 22, 2008 at 02:11:52PM +0800, Xu, Anthony wrote:
>>>> Isaku Yamahata wrote:
>>>>> On Wed, Oct 22, 2008 at 01:56:05PM +0800, Xu, Anthony wrote:
>>>>>> Yes, it is not SMP-safe there is lock for p2m.
>>>>>> Modifying p2m is not a frequent operation, why not add a lock for
>>>>>> it?
>>>>>
>>>>> It is frequent to read p2m table. So lockless approach was adopted
>>>>> for scalability. It doesn't make sense to lock around only writer
>>>>> side.
>>>>>
>>>>
>>>> If only add write lock for p2m, is there any bad impact/senario?
>>>> Can you explain more details?
>>>
>>> Generally lock should protect both readers and writers.
>>> So locking around only writers doesn't make sense.
>>
>>
>> So you can use read/write lock, multiple reader one writer.
>> Because write is very rare, it will not impact performance, but it
>> makes code in mm.c clear and easy to modify. I think that's why
>> read/write lock exist.
>
> Yes, that's quite right.
> It's another discussion to go for reader/writer lock or
> to keep the current lockless approach.

If we want to keep the current lockless approach, we need to
use ptep_cmpxchg_rel as less as possible

Currently there are four functions using ptep_cmpxchg_rel.
assign_domain_page_cmpxchg_rel
zap_domain_page_one
replace_grant_host_mapping
__assign_domain_page

And the four functions have similar function.

I suggest implementing below code segment as a core function
__replace_domain_page(){

old_pte = ptep_xchg_rel();
If(old_pte is INVALID)
{
}
Elseif( old_pte is _PAGE_IO)
{
}
Else if (old_pte is assigned_MMIO_page)
{
}
Else if ( old_pte is normal page)
{
        put_page( old_page)
}
.....

We need to fully consider the old_pte type.

Other functions are simple wrapper of this core fucntion __replace_domain_page.
This way mm.c may be more readable.



Thanks,
Anthony





















>
> thanks,

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel