From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-6.0 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI,T_RP_MATCHES_RCVD autolearn=ham autolearn_force=no version=3.4.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id 6F2DF7DE78 for ; Mon, 9 Apr 2018 14:08:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751662AbeDIOIh (ORCPT ); Mon, 9 Apr 2018 10:08:37 -0400 Received: from mail.kernel.org ([198.145.29.99]:34752 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751543AbeDIOIh (ORCPT ); Mon, 9 Apr 2018 10:08:37 -0400 Received: from devbox (NE2965lan1.rev.em-net.ne.jp [210.141.244.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DAE1320779; Mon, 9 Apr 2018 14:08:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DAE1320779 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=mhiramat@kernel.org Date: Mon, 9 Apr 2018 23:08:30 +0900 From: Masami Hiramatsu To: Ravi Bangoria Cc: oleg@redhat.com, peterz@infradead.org, srikar@linux.vnet.ibm.com, rostedt@goodmis.org, acme@kernel.org, ananth@linux.vnet.ibm.com, akpm@linux-foundation.org, alexander.shishkin@linux.intel.com, alexis.berlemont@gmail.com, corbet@lwn.net, dan.j.williams@intel.com, jolsa@redhat.com, kan.liang@intel.com, kjlx@templeofstupid.com, kstewart@linuxfoundation.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, milian.wolff@kdab.com, mingo@redhat.com, namhyung@kernel.org, naveen.n.rao@linux.vnet.ibm.com, pc@us.ibm.com, tglx@linutronix.de, yao.jin@linux.intel.com, fengguang.wu@intel.com, jglisse@redhat.com Subject: Re: [PATCH v2 9/9] perf probe: Support SDT markers having reference counter (semaphore) Message-Id: <20180409230830.48118d3c32f7ec448936ed8a@kernel.org> In-Reply-To: <643a8fb2-fb96-8dbe-9f36-2540bd8a1de5@linux.vnet.ibm.com> References: <20180404083110.18647-1-ravi.bangoria@linux.vnet.ibm.com> <20180404083110.18647-10-ravi.bangoria@linux.vnet.ibm.com> <20180409162856.df4c32b840eb5f2ef8c028f1@kernel.org> <643a8fb2-fb96-8dbe-9f36-2540bd8a1de5@linux.vnet.ibm.com> X-Mailer: Sylpheed 3.5.1 (GTK+ 2.24.31; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Mon, 9 Apr 2018 13:59:16 +0530 Ravi Bangoria wrote: > 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 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? I think it should be pr_warning() and return NULL, since user may not be able to trace the event even if it is enabled. > > >> @@ -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! > > Thanks for the review :). > Ravi > -- Masami Hiramatsu -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html