RE: [PATCH 0/3] xenoprof: XSA-313 follow-up

Paul Durrant posted 3 patches 3 years, 11 months ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH 0/3] xenoprof: XSA-313 follow-up
Posted by Paul Durrant 3 years, 11 months ago
> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: 20 April 2020 15:15
> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
> <wl@xen.org>
> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
> 
> Hi Paul,
> 
> On 15/04/2020 09:50, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 15 April 2020 09:45
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian
> Jackson
> >> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
> >> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
> >> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
> >>
> >> Patch 1 was considered to become part of the XSA, but it was then
> >> decided against. The other two are a little bit of cleanup, albeit
> >> there's certainly far more room for tidying. Yet then again Paul,
> >> in his mail from Mar 13, was asking whether we shouldn't drop
> >> xenoprof altogether, at which point cleaning up the code would be
> >> wasted effort.
> >>
> >
> > That's still my opinion. This is a large chunk of (only passively maintained) code which I think is
> of very limited value (since it relates to an old tool, and it only works for PV domains).
> 
> While there are no active user we are aware of, this is an example on
> how to implement a profiler backend with Xen. So I would agree with
> Andrew here.
> 
> IIRC, the reason behind your request is it makes difficult for your
> xenheap work. Am I correct? If so, do you have a thread explaining the
> issues?

After shared info and grant table, it is the only other occurrence of a xenheap page shared with a non-system domain. Also, it cannot be trivially replaced with an 'extra' domheap page because its assignment changes. Thus a whole bunch of cleanup work that I was hoping to do (largely in domain_relinquish_resources and free_domheap_pages) is either ruled out, or would have to special-case this type of page.
Also, I am unconvinced that PV guests are sufficiently common these days (apart from dom0) that profiling them is of any real use.

  Paul


Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
Posted by Julien Grall 3 years, 11 months ago
Hi Paul,

On 20/04/2020 16:01, Paul Durrant wrote:
>> -----Original Message-----
>> From: Julien Grall <julien@xen.org>
>> Sent: 20 April 2020 15:15
>> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
>> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
>> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
>> <wl@xen.org>
>> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
>>
>> Hi Paul,
>>
>> On 15/04/2020 09:50, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: 15 April 2020 09:45
>>>> To: xen-devel@lists.xenproject.org
>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian
>> Jackson
>>>> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
>>>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
>>>> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
>>>>
>>>> Patch 1 was considered to become part of the XSA, but it was then
>>>> decided against. The other two are a little bit of cleanup, albeit
>>>> there's certainly far more room for tidying. Yet then again Paul,
>>>> in his mail from Mar 13, was asking whether we shouldn't drop
>>>> xenoprof altogether, at which point cleaning up the code would be
>>>> wasted effort.
>>>>
>>>
>>> That's still my opinion. This is a large chunk of (only passively maintained) code which I think is
>> of very limited value (since it relates to an old tool, and it only works for PV domains).
>>
>> While there are no active user we are aware of, this is an example on
>> how to implement a profiler backend with Xen. So I would agree with
>> Andrew here.
>>
>> IIRC, the reason behind your request is it makes difficult for your
>> xenheap work. Am I correct? If so, do you have a thread explaining the
>> issues?
> 
> After shared info and grant table, it is the only other occurrence of a xenheap page shared with a non-system domain. Also, it cannot be trivially replaced with an 'extra' domheap page because its assignment changes. Thus a whole bunch of cleanup work that I was hoping to do (largely in domain_relinquish_resources and free_domheap_pages) is either ruled out, or would have to special-case this type of page.

My knowledge of xenoprof is very limited, so I might be wrong.

 From my understanding, a buffer can only be shared between two domains:
    - When in passive mode, the buffer will be shared with the primary 
profiler (always the hwdom per the check in xenoprof_op_init()).
    - When in active mode, the buffer will be shared with the domain 
requesting to be profiled.

Would it be possible to consider to have a separate buffer for the 
passive mode and active mode?

> Also, I am unconvinced that PV guests are sufficiently common these days (apart from dom0) that profiling them is of any real use.

Even an HVM guest can't profile itself, I think it would be useful to 
have dom0 to profile an HVM guest. Are you suggesting this doesn't work?

Cheers,

-- 
Julien Grall

RE: [PATCH 0/3] xenoprof: XSA-313 follow-up
Posted by Paul Durrant 3 years, 11 months ago
> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: 24 April 2020 11:35
> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
> <wl@xen.org>
> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
> 
> Hi Paul,
> 
> On 20/04/2020 16:01, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Julien Grall <julien@xen.org>
> >> Sent: 20 April 2020 15:15
> >> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
> >> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> >> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
> >> <wl@xen.org>
> >> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
> >>
> >> Hi Paul,
> >>
> >> On 15/04/2020 09:50, Paul Durrant wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: 15 April 2020 09:45
> >>>> To: xen-devel@lists.xenproject.org
> >>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian
> >> Jackson
> >>>> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
> >>>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
> >>>> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
> >>>>
> >>>> Patch 1 was considered to become part of the XSA, but it was then
> >>>> decided against. The other two are a little bit of cleanup, albeit
> >>>> there's certainly far more room for tidying. Yet then again Paul,
> >>>> in his mail from Mar 13, was asking whether we shouldn't drop
> >>>> xenoprof altogether, at which point cleaning up the code would be
> >>>> wasted effort.
> >>>>
> >>>
> >>> That's still my opinion. This is a large chunk of (only passively maintained) code which I think
> is
> >> of very limited value (since it relates to an old tool, and it only works for PV domains).
> >>
> >> While there are no active user we are aware of, this is an example on
> >> how to implement a profiler backend with Xen. So I would agree with
> >> Andrew here.
> >>
> >> IIRC, the reason behind your request is it makes difficult for your
> >> xenheap work. Am I correct? If so, do you have a thread explaining the
> >> issues?
> >
> > After shared info and grant table, it is the only other occurrence of a xenheap page shared with a
> non-system domain. Also, it cannot be trivially replaced with an 'extra' domheap page because its
> assignment changes. Thus a whole bunch of cleanup work that I was hoping to do (largely in
> domain_relinquish_resources and free_domheap_pages) is either ruled out, or would have to special-case
> this type of page.
> 
> My knowledge of xenoprof is very limited, so I might be wrong.
> 
>  From my understanding, a buffer can only be shared between two domains:
>     - When in passive mode, the buffer will be shared with the primary
> profiler (always the hwdom per the check in xenoprof_op_init()).
>     - When in active mode, the buffer will be shared with the domain
> requesting to be profiled.
> 
> Would it be possible to consider to have a separate buffer for the
> passive mode and active mode?

I think that may well work, yes.

> 
> > Also, I am unconvinced that PV guests are sufficiently common these days (apart from dom0) that
> profiling them is of any real use.
> 
> Even an HVM guest can't profile itself, I think it would be useful to
> have dom0 to profile an HVM guest. Are you suggesting this doesn't work?
> 

No. That may work for a PV dom0; I'll have to try it. So, yes, passive profiling may indeed still have value.

  Paul