[Cbe-oss-dev] [RFC, PATCH] CELL Oprofile SPU profiling updated patch

[Arnd Bergmann]

On Thursday 08 February 2007 16:48, Milton Miller wrote:
> > +	if (offset) {
> > +	  /* When offset is non-zero,  this means the SPU ELF was embedded;
> > +	   * otherwise, it was loaded from a separate binary file.  For the
> > +	   * embedded case, we record the offset of the SPU ELF into the PPU
> > +	   * executable; for the non-embedded case, we record a dcookie that
> > +	   * points to the location of the SPU binary that was loaded.
> > +	   */
> > +		add_event_entry(SPU_OFFSET_CODE);
> > +		add_event_entry(offset);
> > +	} else {
> > +		add_event_entry(SPU_COOKIE_CODE);
> > +		add_event_entry(spu_cookie);
> > +	
> 
> 
> Why should the be inherently exclusive?
> 
> why not compare the spu dcookie with app_cookie to include it,
> and offset whenever non-zero?
> 
> That way I can mmap an .ar of my spu programs for instance.

Right, I already commented on this in the last version of the
patch. The case we want to be able to handle is not a .ar file
but rather a shared library on the powerpc side containing
multiple spu binaries. The answer is the same though.

> 
> As Christoph alluded to, vma is very commonly used to refer
> to a vm_area struct.   This file appears to be related
> to translating a spu address to the elf header offset
> from whence it came.   How about map_saddr (for spu address)
> or map_spu.c ?

The common abbreviation for an spu address would be 'ls'
for 'local store'. so how about map_ls?

> > +	static const unsigned char expected[EI_PAD] = {
> > +		[EI_MAG0] = ELFMAG0,
> > +		[EI_MAG1] = ELFMAG1,
> > +		[EI_MAG2] = ELFMAG2,
> > +		[EI_MAG3] = ELFMAG3,
> > +		[EI_CLASS] = ELFCLASS32,
> > +		[EI_DATA] = ELFDATA2MSB,
> > +		[EI_VERSION] = EV_CURRENT,
> > +		[EI_OSABI] = ELFOSABI_NONE
> > +	};
> > +
> > +	struct vma_to_fileoffset_map *map = NULL;
> > +	unsigned int overlay_tbl_offset = -1;
> > +	unsigned long phdr_start, shdr_start;
> > +	Elf32_Ehdr ehdr;
> > +	Elf32_Phdr phdr;
> > +	Elf32_Shdr shdr, shdr_str;
> > +	Elf32_Sym sym;
> > +	int i, j;
> > +	char name[32];
> > +
> > +	unsigned int ovly_table_sym = 0;
> > +	unsigned int ovly_buf_table_sym = 0;
> > +	unsigned int ovly_table_end_sym = 0;
> > +	unsigned int ovly_buf_table_end_sym = 0;
> > +	unsigned long ovly_table;
> > +	unsigned int n_ovlys;
> > +
> > +	struct {
> > +		unsigned int vma;
> > +		unsigned int size;
> > +		unsigned int offset;
> > +		unsigned int buf;
> > +	} ovly;
> 
> 
> This should struct be in asm/elf.h no?  or perhaps elf-spe.h

only if it is in the respective place in the binutils elf headers,
I'd say. AFAIK, that is the canonical version of elf.h

> All the copy_from_user returns must be checked.

Right, though an interesting question is what to do if one of them
fails. Can we just return to say that there are no overlays int
that context, or should there be some more drastic measure, like
killing the thread?

	Arnd <><