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 1/5 TAKE 2] xenoprof: split xen x86 xenoprof code

To: "Isaku Yamahata" <yamahata@xxxxxxxxxxxxx>, "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH 1/5 TAKE 2] xenoprof: split xen x86 xenoprof code
From: "Santos, Jose Renato G" <joserenato.santos@xxxxxx>
Date: Wed, 15 Nov 2006 21:58:40 -0600
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 15 Nov 2006 19:59:35 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20061115021425.GA15884%yamahata@xxxxxxxxxxxxx>
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/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AccIahSqJlZWm0+aRZSKXbNtl7zLBQAxbxhQ
Thread-topic: [Xen-devel] [PATCH 1/5 TAKE 2] xenoprof: split xen x86 xenoprof code
Isaku,

Mostly, the patches look good. 
I have a few questions though , mostly for clarification

Keir,

Could you please wait until I have a chance to test the patches on my
machine before applying.
I will be out all day tomorrow (Thursday) to attend an external event so
I will only be able to do this on Friday.
Also, I think it would be good  if you could review parts of patch 5.
There are some changes on how xenoprof shares page with the guest. The
patch decouples the code for allocating xenoprof buffer from the code
for sharing it with the domain. This is very welcome as it has been on
my wish list for a while. This modification should remove the current
limitation that guests cannot use different profiling modes (active and
passive) in different profiling sessions. But I am afraid I do not
understand Xen memory management in detail to review this part of the
patch. Please, take a look.

-----------------------
QUESTIONS for Isaku:

patch 4/5:
==========

> +struct xenoprof_shared_buffer {
> +     char                                    *buffer;
> +     struct xenoprof_arch_shared_buffer      arch;
> +};

The arch field has no extra info for x86. Why do you need this?


> @@ -103,7 +98,7 @@ static int __init init_driverfs(void)
>  }
>  
>  
> -static void __exit exit_driverfs(void)
> +static void /* __exit */ exit_driverfs(void)
>  {
>       sysdev_unregister(&device_oprofile);
>       sysdev_class_unregister(&oprofile_sysclass);

Why removing "__exit" is needed? 


> +void xenoprof_arch_start(void) 
> +{
> +     /* nothing */
> +}
> +
> +void xenoprof_arch_stop(void)
> +{
> +     /* nothing */
> +}
> +

Why do we need arch specific functions for start and stop?

> @@ -22,9 +22,25 @@
>  
>  #ifndef __XEN_XENOPROF_H__
>  #define __XEN_XENOPROF_H__
> +
>  #ifdef CONFIG_OPROFILE
> -
>  #include <asm/xenoprof.h>
>  
> +struct oprofile_operations;
> +int xenoprofile_init(struct oprofile_operations * ops);
> +void xenoprofile_exit(void);
> +
> +// for perfmon/xen

Please drop the comment or change it (xenoprofile is not using perfmon)


patch 5/5:
=========

> -static char *alloc_xenoprof_buf(struct domain *d, int npages)
> +static char *alloc_xenoprof_buf(struct domain *d, int npages,
uint64_t gmaddr)

The extra parameter gmaddr is not needed since if it is not used by the
function.


Regards

Renato

 

> -----Original Message-----
> From: Isaku Yamahata [mailto:yamahata@xxxxxxxxxxxxx] 
> Sent: Tuesday, November 14, 2006 6:14 PM
> To: Santos, Jose Renato G
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH 1/5 TAKE 2] xenoprof: split 
> xen x86 xenoprof code
> 
> On Tue, Nov 14, 2006 at 11:23:42AM -0600, Santos, Jose Renato G wrote:
> 
> > This patch is corrupt. It does not apply cleanly. It has several 
> > missing "newlines" such as for example the line above.
> > Could you please regenerate ?
> > I am not sure about the other patches, but it would help if you can 
> > make sure they are all OK and resend.
> 
> I regenerated them for 12446:fb107b9eee86 of xen-unstable.hg.
> They should apply cleanly and compile.
> To make sure not to corrupt the patches, I attached them as a 
> tar ball.
> Please find it.
> 
> --
> yamahata
> 

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

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