hvm_set_callback_irq_level() deadlock?

hvm_set_callback_irq_level() deadlock?

[Akio Takebe]

Hi,

hvm_set_callback_irq_level() and so on call vioapic_irq_positive_edge()
before spin_unlock(&d->arch.hvm_domain.irq_lock).
I think it cause a deadlock.
If it is right, how should we fix them?
What do you think?

Best Regards,

Akio Takebe

Re: hvm_set_callback_irq_level() deadlock?

[Keir Fraser]

On 07/01/2009 09:41, "Akio Takebe" <takebe_akio@jp.fujitsu.com> wrote:

> hvm_set_callback_irq_level() and so on call vioapic_irq_positive_edge()
> before spin_unlock(&d->arch.hvm_domain.irq_lock).
> I think it cause a deadlock.
> If it is right, how should we fix them?
> What do you think?

Doesn't vioapic_irq_positive_edge() clearly expect to be called with that
lock held?

 -- Keir

Re: hvm_set_callback_irq_level() deadlock?

[Akio Takebe]

Hi,

Keir Fraser wrote:
> On 07/01/2009 09:41, "Akio Takebe" <takebe_akio@jp.fujitsu.com> wrote:
> 
>> hvm_set_callback_irq_level() and so on call vioapic_irq_positive_edge()
>> before spin_unlock(&d->arch.hvm_domain.irq_lock).
>> I think it cause a deadlock.
>> If it is right, how should we fix them?
>> What do you think?
> 
> Doesn't vioapic_irq_positive_edge() clearly expect to be called with that
> lock held?
I concern about that vioapic_deliver() calls vcpu_kick(). If vcpu0 has the lock and
vcpu1 cannot get lock and spin then vcpu0 sleep in another function,
it may cause deadlock because vioapic_irq_positive_edge() may call vcpu_kick().
For example, the following function is OK?

1087 static void time_calibration_rendezvous(void *_r)
1088 {
1089     struct cpu_calibration *c = &this_cpu(cpu_calibration);
1090     struct calibration_rendezvous *r = _r;
1091     unsigned int total_cpus = cpus_weight(r->cpu_calibration_map);
1092
1093     if ( smp_processor_id() == 0 )
1094     {
1095         while ( atomic_read(&r->nr_cpus) != (total_cpus - 1) )
1096             cpu_relax();
1097         r->master_stime = read_platform_stime();
1098         rdtscll(r->master_tsc_stamp);
1099         mb(); /* write r->master_* /then/ signal */
1100         atomic_inc(&r->nr_cpus);
1101         c->local_tsc_stamp = r->master_tsc_stamp;
1102     }
1103     else
1104     {
1105         atomic_inc(&r->nr_cpus);
1106         while ( atomic_read(&r->nr_cpus) != total_cpus )
1107             cpu_relax();
1108         mb(); /* receive signal /then/ read r->master_* */
1109         if ( boot_cpu_has(X86_FEATURE_CONSTANT_TSC) )
1110             wrmsrl(MSR_IA32_TSC, r->master_tsc_stamp);
1111         rdtscll(c->local_tsc_stamp);
1112     }
1113
1114     c->stime_local_stamp = get_s_time();
1115     c->stime_master_stamp = r->master_stime;
1116
1117     raise_softirq(TIME_CALIBRATE_SOFTIRQ);
1118 }


Best Regards,

Akio Takebe

Re: hvm_set_callback_irq_level() deadlock?

[Keir Fraser]

On 07/01/2009 10:22, "Akio Takebe" <takebe_akio@jp.fujitsu.com> wrote:

>> Doesn't vioapic_irq_positive_edge() clearly expect to be called with that
>> lock held?
> I concern about that vioapic_deliver() calls vcpu_kick(). If vcpu0 has the
> lock and
> vcpu1 cannot get lock and spin then vcpu0 sleep in another function,
> it may cause deadlock because vioapic_irq_positive_edge() may call
> vcpu_kick().
> For example, the following function is OK?

I still don't see the problem. You'll have to spell out a concrete example,
explaining where the two CPUs would be spinning and what state they're in
(i.e, why they are depending on each other, and why no progress can be
made).

 -- Keir

Re: hvm_set_callback_irq_level() deadlock?

[Akio Takebe]

Keir Fraser wrote:
> On 07/01/2009 10:22, "Akio Takebe" <takebe_akio@jp.fujitsu.com> wrote:
> 
>>> Doesn't vioapic_irq_positive_edge() clearly expect to be called with that
>>> lock held?
>> I concern about that vioapic_deliver() calls vcpu_kick(). If vcpu0 has the
>> lock and
>> vcpu1 cannot get lock and spin then vcpu0 sleep in another function,
>> it may cause deadlock because vioapic_irq_positive_edge() may call
>> vcpu_kick().
>> For example, the following function is OK?
> 
> I still don't see the problem. You'll have to spell out a concrete example,
> explaining where the two CPUs would be spinning and what state they're in
> (i.e, why they are depending on each other, and why no progress can be
> made).
> 
I thought more. Sorry, I was wrong.

Best Regards,

Akio Takebe