All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Wang Nan <wangnan0@huawei.com>
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.
Date: Thu, 28 May 2015 10:33:48 +0200	[thread overview]
Message-ID: <20150528083347.GA3712@krava.redhat.com> (raw)
In-Reply-To: <1429587190-208370-1-git-send-email-wangnan0@huawei.com>

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

  parent reply	other threads:[~2015-05-28  8:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-21  3:33 [PATCH v6] perf: __kmod_path__parse: deal with kernel module names in '[]' correctly Wang Nan
2015-04-21  5:16 ` Namhyung Kim
2015-04-21  9:28   ` Wang Nan
2015-05-29  0:00     ` Namhyung Kim
2015-06-03  7:49       ` [PATCH v7] " Wang Nan
2015-06-03  8:47         ` Jiri Olsa
2015-06-03  8:52           ` [PATCH v8] " Wang Nan
2015-06-03  9:12             ` Jiri Olsa
2015-06-03 13:09               ` Arnaldo Carvalho de Melo
2015-06-04 14:12             ` [tip:perf/core] perf tools: Deal " tip-bot for Wang Nan
2015-05-28  8:33 ` Jiri Olsa [this message]
     [not found] <E1Yxs9m-0004lw-1t@feisty.vs19.net>
2015-05-28  7:31 ` [PATCH v6] perf: __kmod_path__parse: deal " Wangnan (F)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150528083347.GA3712@krava.redhat.com \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=pi3orama@163.com \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.