Re: [Xen-devel] [PATCH v4 4/4] Xentrace: add support for HVM's PI blocking list operation

[Chao Gao <chao.gao@xxxxxxxxx>]

On Fri, Jul 21, 2017 at 05:26:47PM +0100, George Dunlap wrote:
>On Fri, Jul 7, 2017 at 7:49 AM, Chao Gao <chao.gao@xxxxxxxxx> wrote:
>> In order to analyze PI blocking list operation frequence and obtain
>> the list length, add some relevant events to xentrace and some
>> associated code in xenalyze. Event ASYNC_PI_LIST_DEL may happen in interrupt
>> context, which incurs current assumptions checked in toplevel_assert_check()
>> are not suitable any more. Thus, this patch extends the 
>> toplevel_assert_check()
>> to remove such assumptions for events of type ASYNC_PI_LIST_DEL.
>>
>> Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
>
>Hey Chao Gao,
>
>Thanks for doing the work to add this tracing support to xentrace --
>and in particular taking the effort to adapt the assert mechanism to
>be able to handle asynchronous events.
>
>I think in this case though, having a separate HVM sub-class for
>asynchronous events isn't really the right approach.  The main purpose
>of sub-classes is to help filter the events you want; and I can't
>think of any time you'd want to trace PI_LIST_DEL and not PI_LIST_ADD
>(or vice versa).  Secondly, the "asynchronous event" problem will be
>an issue for other contexts as well, and the solution will be the
>same.
>
>I think a better solution would be to do something similar to
>TRC_64_FLAG and TRC_HVM_IOMEM_[read,write], and claim another bit to
>create a TRC_ASYNC_FLAG (0x400 probably).  Then we can filter the
>"not_idle_domain" and "vcpu_data_mode" asserts on that.
>
>What do you think?

It makes sense to me. Your other comments on this series are also fine
to me. I will cook another version based on your suggestions.

Thanks
Chao

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