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

[Milton Miller]

On Feb 8, 2007, at 10:59 AM, Arnd Bergmann wrote:

> On Thursday 08 February 2007 16:48, Milton Miller wrote:
>
>>> +	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;

[I was not talking the above, only the below]

>>> +
>>> +	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

Why should we be limited?

Besides, this is a user api, it belongs in a header.


>> 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?


Why would we do anything so drastic?   The user didn't pass this as
an argument to the syscall.   The old libraries will not pass the
address that we rely on to find the headers.

I'd say we stop parsing the headers of that object id and return
back to caller.   With the current patch, we just loose the samples
(need to make sure we don't spam the logs).  With my other proposal
we could log create a event "header parse error" and continue, loosing
only file info.


miilton