(2013/11/07 17:48), Namhyung Kim wrote: > On Wed, 6 Nov 2013 18:37:54 +0100, Oleg Nesterov wrote: >> On 11/06, Namhyung Kim wrote: >>> >>> On Tue, 5 Nov 2013 20:24:01 +0100, Oleg Nesterov wrote: >>>> On 11/05, Oleg Nesterov wrote: >>>>> >>>>> As for "-= tu->offset"... Can't we avoid it? User-space needs to calculate >>>>> the "@" argument anyway, why it can't also substruct this offset? >>>>> >>>>> Or perhaps we can change parse_probe_arg("@") to update "param" ? Yes, >>>>> in this case it needs another argument, not sure... >>>> >>>> Or, >>>> >>>>> + if (is_ret_probe(tu)) { >>>>> + saved_ip = instruction_pointer(regs); >>>>> + instruction_pointer_set(func); >>>>> + } >>>>> store_trace_args(...); >>>>> + if (is_ret_probe(tu)) >>>>> + instruction_pointer_set(saved_ip); >>>> >>>> we can put "-= tu->offset" here. >>> >>> I don't think I get the point. >> >> I meant, >> >> saved_ip = instruction_pointer(regs); >> >> // pass the "ip" which was used to calculate >> // the @addr argument to fetch_*() methods >> >> temp_ip = is_ret_probe(tu) ? func : saved_ip; >> temp_ip -= tu->offset; >> instruction_pointer_set(temp_ip); >> >> store_trace_args(...); >> >> instruction_pointer_set(saved_ip); >> >> This way we can avoid the new "void *" argument for fetch_func_t, >> we do not need it to calculate the address. > > Okay, but as I said before, subtracting tu->offset part can be removed.
Ah, that's good to me too :) >> >> However, now I think it would be more clean to leave FETCH_MTD_memory >> alone and add FETCH_MTD_memory_dotranslate instead. >> >> So trace_uprobes.c should define >> >> void FETCH_FUNC_NAME(memory, type)(addr, ...) >> { >> copy_from_user((void __user *)addr); >> } >> >> void FETCH_FUNC_NAME(memory_dotranslate, type)(addr, ...) >> { >> void __user *uaddr = get_user_vaddr(regs, addr); >> copy_from_user(uaddr); >> } > > Looks good. > >> >> Then, >> >>>> Or. Perhaps we can leave "case '@'" in parse_probe_arg() and >>>> FETCH_MTD_memory alone. You seem to agree that "absolute address" >>>> can be useful anyway. >>> >>> Yes, but it's only meaningful to process-wide tracing sessions IMHO. >> >> Yes, yes, sure. >> >> I meant, we need both. Say, "perf probe "func global=@addr" means >> FETCH_MTD_memory, and "perf probe "func global=*addr" means >> FETCH_MTD_memory_dotranslate. >> >> Just in case, of course I do not care about the syntax, for example we >> can use "@~addr" for translate (or not translate) or whatever. > > Yeah, and I want to hear from Masami. Hm, this part I need to clarify. So you mean the @addr is for referring the absolute address in a user process, but @~addr is for referring the relative address of a executable or library, correct? In that case, I suggest you to use "@+addr" for the relative address, since that is an offset, isn't that? :) BTW, it seems that @addr syntax is hard to use for uprobes, because current uprobes is based on a binary, not a process, we cannot specify which process is probed when we define it. >> My only point: I think we need both to >> >> 1. avoid the new argument in fetch_func_t >> >> 2. allow the dump the data from the absolute address Looks good to me :) Thank you! -- Masami HIRAMATSU IT Management Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/