Hi Masami,

On 04/09/2018 12:58 PM, Masami Hiramatsu wrote:
> Hi Ravi,
>
> On Wed,  4 Apr 2018 14:01:10 +0530
> Ravi Bangoria <ravi.bango...@linux.vnet.ibm.com> wrote:
>
>> @@ -2054,15 +2060,21 @@ char *synthesize_probe_trace_command(struct 
>> probe_trace_event *tev)
>>      }
>>  
>>      /* Use the tp->address for uprobes */
>> -    if (tev->uprobes)
>> +    if (tev->uprobes) {
>>              err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
>> -    else if (!strncmp(tp->symbol, "0x", 2))
>> +            if (uprobe_ref_ctr_is_supported() &&
>> +                tp->ref_ctr_offset &&
>> +                err >= 0)
>> +                    err = strbuf_addf(&buf, "(0x%lx)", tp->ref_ctr_offset);
> If the kernel doesn't support uprobe_ref_ctr but the event requires
> to increment uprobe_ref_ctr, I think we should (at least) warn user here.

pr_debug("A semaphore is associated with %s:%s and seems your kernel doesn't 
support it.\n"
         tev->group, tev->event);

Looks good?

>> @@ -776,14 +784,21 @@ static char *synthesize_sdt_probe_command(struct 
>> sdt_note *note,
>>  {
>>      struct strbuf buf;
>>      char *ret = NULL, **args;
>> -    int i, args_count;
>> +    int i, args_count, err;
>> +    unsigned long long ref_ctr_offset;
>>  
>>      if (strbuf_init(&buf, 32) < 0)
>>              return NULL;
>>  
>> -    if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
>> -                            sdtgrp, note->name, pathname,
>> -                            sdt_note__get_addr(note)) < 0)
>> +    err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
>> +                    sdtgrp, note->name, pathname,
>> +                    sdt_note__get_addr(note));
>> +
>> +    ref_ctr_offset = sdt_note__get_ref_ctr_offset(note);
>> +    if (uprobe_ref_ctr_is_supported() && ref_ctr_offset && err >= 0)
>> +            err = strbuf_addf(&buf, "(0x%llx)", ref_ctr_offset);
> We don't have to care about uprobe_ref_ctr support here, because
> this information will be just cached, not directly written to
> uprobe_events.

Sure, will remove the check.

Thanks for the review :).
Ravi

Reply via email to