[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <643a8fb2-fb96-8dbe-9f36-2540bd8a1de5@linux.vnet.ibm.com>
Date: Mon, 9 Apr 2018 13:59:16 +0530
From: Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
To: Masami Hiramatsu <mhiramat@...nel.org>
Cc: oleg@...hat.com, peterz@...radead.org, srikar@...ux.vnet.ibm.com,
rostedt@...dmis.org, acme@...nel.org, ananth@...ux.vnet.ibm.com,
akpm@...ux-foundation.org, alexander.shishkin@...ux.intel.com,
alexis.berlemont@...il.com, corbet@....net,
dan.j.williams@...el.com, jolsa@...hat.com, kan.liang@...el.com,
kjlx@...pleofstupid.com, kstewart@...uxfoundation.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, milian.wolff@...b.com, mingo@...hat.com,
namhyung@...nel.org, naveen.n.rao@...ux.vnet.ibm.com,
pc@...ibm.com, tglx@...utronix.de, yao.jin@...ux.intel.com,
fengguang.wu@...el.com, jglisse@...hat.com,
Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
Subject: Re: [PATCH v2 9/9] perf probe: Support SDT markers having reference
counter (semaphore)
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@...ux.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
Powered by blists - more mailing lists