Re: [PATCH] Identify which executable object the userspace address belongs to. Store thread group leader id, and use it to lookup the address in the process's map. We could have looked up the address on thread's map, but the thread might not exist by the time we are called. The process might not exist either, but if you are reading trace_pipe, that is unlikely.

["Török Edwin" <edwintorok@gmail.com>]

On 2008-11-03 09:47, Ingo Molnar wrote:
> * Török Edwin <edwintorok@gmail.com> wrote:
>
>   
>> +		struct task_struct *task;
>> +		/* we do the lookup on the thread group leader,
>> +		 * since individual threads might have already quit! */
>>     
>
> please use the customary comment style:
>
>   /*
>    * Comment .....
>    * ...... goes here:
>    */
>
>   

. Can checkpatch.pl warn me of situations like this?

>> -		if (ret /*&& (sym_flags & TRACE_ITER_SYM_ADDR)*/)
>> +		if (mm) {
>> +			const struct vm_area_struct *vma = find_vma(mm, ip);
>> +			if (vma) {
>> +				file = vma->vm_file;
>> +				vmstart = vma->vm_start;
>> +			}
>> +		}
>> +		if (file) {
>> +			ret = trace_seq_path(s, &file->f_path);
>> +			if (ret)
>> +				ret = trace_seq_printf(s, "[+0x%lx]",
>> +						ip - vmstart);
>> +		}
>> +		if (ret && ((sym_flags & TRACE_ITER_SYM_ADDR) || !file))
>>  			ret = trace_seq_printf(s, " <" IP_FMT ">", ip);
>>     
>
> the function is getting a bit large - would it make sense to split out 
> this section into a helper inline function?
>   

Yes, I'll do that.

> another small nit: i cleaned up the subject line and the commit log 
> message for you - see the two patches below - could you please keep it 
> for future (v3) submissions of these patches? It's just small 
> formatting changes.
>   

Ok, luckily git-rebase -i allows me to rewrite the log message too ;)

> Your patches are a nice feature we want to have nevertheless - to be 
> able to see where a user-space app is running has been one of the 
> historically weak points of kernel instrumentation.

Thanks.
It currently works for x86 only, but architecture porters can add
support for theirs quite easily, it just needs to modeled after how
oprofile does it for example.
BTW would it make sense to change oprofile and the sysprof tracer to use
save_stack_trace_user? It would eliminate some code duplication.

Would it make sense to add a script that post-processes the output to
scripts/tracing?
It would parse a trace log (from trace or latency_trace) and use
addr2line to resolve the address to source:line, and if successful
replace the relative address with that; and also
group identical stack traces together.

Best regards,
--Edwin