Re: [Xen-devel] [xen-unstable test] 106504: regressions - FAIL

[Chao Gao <chao.gao@xxxxxxxxx>]

On Wed, Apr 05, 2017 at 01:48:22AM -0600, Jan Beulich wrote:
>>>> On 05.04.17 at 01:57, <chao.gao@xxxxxxxxx> wrote:
>> On Wed, Mar 22, 2017 at 06:47:33AM -0600, Jan Beulich wrote:
>> 
>> Hi, Jan.
>> 
>> I plan to do the following changes:
>> 1. get the vector set in vIRR to avoid getting a wrong interrupt vector
>>  I think there are two appoaches. One is to extend hvm_isa_irq_assert()
>>  to return the vector set in vIRR. Several functions in call trees are
>>  also involved. The other is to make vIOAPIC support disabling
>>  write operations to RTE. In this case, a rwlock_t is introduced to 
>>  protect RTE. pt_update_irq() will disable write operations
>>  at first, then get the vector and assert the vector, at last enable
>>  write operations. Which one do you think is better?
>
>That's hard to tell without seeing the changes each actually involves.
>On the surface I'd probably prefer the 2nd, provided the locking can
>be got into a shape where there's no meaningful risk of missing an
>unlock on some path.

Thanks your opinion. I will try to add the lock.

>
>> 2. let pt_update_irq() pass the periodic timer
>>  whose interrupt is to be injected to vmx_intr_assit() which
>>  in turn can pass it to pt_intr_post(). After this, pt_intr_post()
>>  needn't search the periodic timer that matches the interrupt has
>>  been injected. Through this, we can avoid reading the RTE there.
>
>If the RTE can't be changed behind your back, why would you
>need this?

Yes. With the first change described above, the second change is needless
theoretically. But If the lock is acquired in vmx_intr_assist(), the lock is
also acquired in major cases (using LAPIC timer) in which getting lock is
useless.  If the lock is acquired in pt_update_irq(), it would better be
released there. Thus the lock can't protect pt_intr_post(). Also the second
change would reduce the time spends in locked region. I am worried about adding
a lock here (very critical path) may hurt the performance.

Also I admit making this change should be very careful. Changing less decreases
the possibility of introducing errors.

Thanks
Chao

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel