Re: [Patch 2/3] Cell BE SPU: daemon changes

[Maynard J. <may...@us...>]

John Levon wrote:

> 	/** anonymous mapping */
> 	struct anon_mapping * anon;
>+	unsigned int embedded_offset;
>
>Add doc comment. Is "unsigned int" really the right type??
>  
>
OK, I'll add a doc comment.   Yeah, probably should be unsigned long, 
here and every where.

>diff -paurNX diff_fileExclusionFilter oprof-cvs-04.20.2007/daemon/opd_trans.c oprof-cell-patch2/daemon/opd_trans.c
>--- oprof-cvs-04.20.2007/daemon/opd_trans.c	2006-11-17 19:18:58.000000000 -0600
>+++ oprof-cell-patch2/daemon/opd_trans.c	2007-04-20 12:33:16.000000000 -0500
>@@ -267,6 +270,73 @@ static void code_xen_enter(struct transi
>
> typedef void (*handler_t)(struct transient *);
>
>+static handler_t handlers[];
>
>The SPU code should probably be in an opd_spu.c
>  
>
OK.

>+static void code_spu_profiling(struct transient * trans)
>+{
>+        /* Next value in buffer is the number of SPUs. */
>+
>+	unsigned long long num_spus = pop_buffer_value(trans);
>+	spu_context_cache = malloc(sizeof(struct spu_context_info) * num_spus);
>
>Check malloc return?
>  
>
Right.

>@@ -310,6 +474,11 @@ void opd_process_samples(char const * bu
> 	 */
> 	unsigned long long code;
>
>+	if (special_processor) {
>+		special_processor(&trans);
>+		return;
>+	}
>+
>
>Rather than special casing, have a default processor and your SPU one,
>and switch between, so this code always do
>
>	process_samples(&trans);
>  
>
OK.

>regards
>john
>  
>