Messages in this thread | | | Subject | Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore) | From | Ravi Bangoria <> | Date | Wed, 14 Mar 2018 20:42:24 +0530 |
| |
Hi Masami,
On 03/14/2018 07:18 PM, Masami Hiramatsu wrote: > Hi Ravi, > > On Tue, 13 Mar 2018 18:26:00 +0530 > Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote: > >> Userspace Statically Defined Tracepoints[1] are dtrace style markers >> inside userspace applications. These markers are added by developer at >> important places in the code. Each marker source expands to a single >> nop instruction in the compiled code but there may be additional >> overhead for computing the marker arguments which expands to couple of >> instructions. In case the overhead is more, execution of it can be >> ommited by runtime if() condition when no one is tracing on the marker: >> >> if (reference_counter > 0) { >> Execute marker instructions; >> } >> >> Default value of reference counter is 0. Tracer has to increment the >> reference counter before tracing on a marker and decrement it when >> done with the tracing. >> >> Implement the reference counter logic in trace_uprobe, leaving core >> uprobe infrastructure as is, except one new callback from uprobe_mmap() >> to trace_uprobe. >> >> trace_uprobe definition with reference counter will now be: >> >> <path>:<offset>[(ref_ctr_offset)] > Would you mean > <path>:<offset>(<ref_ctr_offset>) > ? > > or use "[]" for delimiter?
[] indicates optional field.
> Since, > >> @@ -454,6 +458,26 @@ static int create_trace_uprobe(int argc, char **argv) >> goto fail_address_parse; >> } >> >> + /* Parse reference counter offset if specified. */ >> + rctr = strchr(arg, '('); > This seems you choose "()" for delimiter.
Correct.
>> + if (rctr) { >> + rctr_end = strchr(arg, ')'); > rctr_end = strchr(rctr, ')'); > > ? since we are sure rctr != NULL.
Yes. we can use rctr instead of arg.
>> + if (rctr > rctr_end || *(rctr_end + 1) != 0) { >> + ret = -EINVAL; >> + pr_info("Invalid reference counter offset.\n"); >> + goto fail_address_parse; >> + } > > Also > >> + >> + *rctr++ = 0; >> + *rctr_end = 0; > Please consider to use '\0' for nul;
Sure. Will change it.
Thanks for the review :) Ravi
| |