Isaku Yamahata wrote:
> On Thu, Oct 23, 2008 at 03:10:55PM +0800, Xu, Anthony wrote:
>> The new one,
>
> It looks almost okay. The last one.
>
>
>> diff -r 02c8733e2d91 xen/arch/ia64/vmx/viosapic.c
>> --- a/xen/arch/ia64/vmx/viosapic.c Wed Oct 22 17:20:15 2008 +0900
>> +++ b/xen/arch/ia64/vmx/viosapic.c Thu Oct 23 14:48:09 2008 +0800 @@
>> -121,6 +121,13 @@ redir_num, vector);
>> return;
>> }
>> + if ( iommu_enabled )
>> + {
>> + spin_unlock(&viosapic->lock);
>> + hvm_dpci_eoi(current->domain, redir_num,
>> &viosapic->redirtbl[redir_num]); + spin_lock(&viosapic->lock);
>> + }
>> +
>> service_iosapic(viosapic);
>> spin_unlock(&viosapic->lock);
>> }
>
> Is this unlock/lock sequence okay?
> I'm asking simply because I'm not sure.
>
> viosapic->irr and isr are protected by the lock.
> And viosapic_update_EOI() updates them atomically.
> The above unlock/lock seems to break its atomicity.
I think it is Okay,
One atomical operation in viosapic_update_EOI is divided into two atomical
operations.
If you get spin_lock again, when returning from hvm_dpci_eoi.
There are many code segments in linux kernel.
And viosapic->irr and isr is still protected by lock.
Anthony
>
> I'm not sure it's okay or not. To make sure, it is required
> to take closer look at viosapic.c.
>
> thanks,
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|