From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753483AbbE1Id4 (ORCPT ); Thu, 28 May 2015 04:33:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48262 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751372AbbE1Idx (ORCPT ); Thu, 28 May 2015 04:33:53 -0400 Date: Thu, 28 May 2015 10:33:48 +0200 From: Jiri Olsa To: Wang Nan Cc: acme@kernel.org, jolsa@kernel.org, mingo@redhat.com, namhyung@kernel.org, lizefan@huawei.com, pi3orama@163.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6] perf: __kmod_path__parse: deal with kernel module names in '[]' correctly. Message-ID: <20150528083347.GA3712@krava.redhat.com> References: <1429587190-208370-1-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1429587190-208370-1-git-send-email-wangnan0@huawei.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 21, 2015 at 03:33:10AM +0000, Wang Nan wrote: > Before patch ba92732e9808df679ddf75c5ea1c0caae6d7dce2 ('perf kmaps: > Check kmaps to make code more robust'), perf report and perf annotate > will segfault if trace data contains kernel module information like > this: sry I've lost track on this one.. found it under: [PATCH v4 2/2] perf: report/annotate: fix segfault problem. my comments were addressed and other than one nit below it looks ok to me Namhyung, I looked up in history that you had question on this one, could you please recheck Wang's reply? thanks, jirka SNIP > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index fc0ddd5..e9d4ae4 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -165,13 +165,25 @@ bool is_supported_compression(const char *ext) > return false; > } > > -bool is_kernel_module(const char *pathname) > +bool is_kernel_module(const char *pathname, int cpumode) > { > struct kmod_path m; > > - if (kmod_path__parse(&m, pathname)) > - return NULL; > + /* caller should pass a masked cpumode. Mask again for safety. */ > + switch (cpumode & PERF_RECORD_MISC_CPUMODE_MASK) { I think it's better to print out if there's unexpected error as warning rather than hide it.. like we do with WARN_ONCE or pr_warning stuff > + case PERF_RECORD_MISC_USER: > + case PERF_RECORD_MISC_HYPERVISOR: > + case PERF_RECORD_MISC_GUEST_USER: > + return false; > + /* Regard PERF_RECORD_MISC_CPUMODE_UNKNOWN as kernel */ > + default: > + if (kmod_path__parse(&m, pathname)) { > + pr_err("Failed to check whether %s is a kernel module or not. Assume it is.", > + pathname); > > + return true; > + } > + } > return m.kmod; SNIP