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.bangoria@xxxxxxxxxxxxxxxxxx> 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
