WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [Patch] continue_hypercall_on_cpu rework using tasklets

To: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>, Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [Patch] continue_hypercall_on_cpu rework using tasklets
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Fri, 16 Apr 2010 18:57:31 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "Yu, Ke" <ke.yu@xxxxxxxxx>
Delivery-date: Sun, 18 Apr 2010 08:39:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <789F9655DD1B8F43B48D77C5D30659731D797868@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcrccV74GaC9GZ9lTbWk9MTZP2+EwQAAhsPFAABQK/sAAk0GUAADclFDACFVn6AACNGn5wAAMOlAABZMAfw=
Thread-topic: [Xen-devel] [Patch] continue_hypercall_on_cpu rework using tasklets
User-agent: Microsoft-Entourage/12.24.0.100205
On 16/04/2010 09:16, "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx> wrote:

> Oops, yes, this should be there already, without c_h_o_c.
> So, if another vcpu is trying to vcpu_pause() this vcpu, it will deadlock
> becaues this vcpu can't be paused, right? (I assume the same reason to
> hvmop_flush_tlb_all).

Yes.

> If this is true, then how about checking !vcpu_runnable(current) in
> stop_machine_run()'s stopmachine_set_state()? If this check failed, it means
> someone try to change our state and potential deadlock may happen, we can
> cancel this stop_machine_run()? This is at least a bit cleaner than the
> timeout mechanism.

Interesting... That could work. But...

>>> Maybe the cleaner way is your previous suggestion, that is, put the
>>> stop_machine_run() in the idle_vcpu(), another way is, turn back to the
>>> original method, i.e. do it in the schedule_tail.
>> 
>> Argh. That would be annoying!
> 
> Seems do it in the schedule_tail will not benifit this deadlock issues.

Yes, but now I think about it, scheduling c_h_o_c() and s_m_r() in idle-vcpu
context instead of softirq context might not be *that* hard to do, and it
avoids all these subtle deadlock possibilities. I think I'm warming to it
compared with the alternatives.

> Yes, c_h_o_c does not introduce any more deadlock, my concern is, it's name
> maybe confusing if one try to use it for other hypercall . After all, if a
> hypercall and function used by the hypercall depends on current, it should not
> use this c_h_o_c.

Well, I think there are two issues: (1) we run the continuation as a
softirq; (2) we don't run the continuation in the context of the original
vcpu. I think (1) is a bigger problem than (2) as it introduces the
possibility of all these nasty subtle deadlocks. (2) is obviously not
desirable, *but* I don't think any callers much care about the context of
the original caller, *and* if they do the resulting bug is generally going
to be pretty obvious. That is, the hypercall just won't work, ever -- it's
much less likely than (1) to result in very rare very subtle bugs.

 -- Keir



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

<Prev in Thread] Current Thread [Next in Thread>