All of lore.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Jiri Olsa <jolsa@redhat.com>, Simon Que <sque@chromium.org>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Stephane Eranian <eranian@google.com>,
	Sonny Rao <sonnyrao@chromium.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf: Get kernel start address by symbol name
Date: Mon, 16 Jun 2014 12:31:32 +0300	[thread overview]
Message-ID: <539EB974.9030904@intel.com> (raw)
In-Reply-To: <20140616080950.GA4632@krava.brq.redhat.com>

On 06/16/2014 11:09 AM, Jiri Olsa wrote:
> On Mon, Jun 16, 2014 at 10:06:49AM +0200, Jiri Olsa wrote:
>> On Fri, Jun 13, 2014 at 11:45:21AM -0700, Simon Que wrote:
>>> The function machine__get_kernel_start_addr() was taking the first symbol
>>> of kallsyms as the start address. This is incorrect in certain cases
>>> where the first symbol is something at 0, while the actual kernel
>>> functions begin at a later point (e.g. 0x80200000).
>>>
>>> This patch fixes machine__get_kernel_start_addr() to search for the
>>> symbol "_text" or "_stext", which marks the beginning of kernel mapping.
>>> This was already being done in machine__create_kernel_maps(). Thus, this
>>> patch is just a refactor, to move that code into
>>> machine__get_kernel_start_addr().
>>>
>>> Change-Id: I0c38c36f5e8b0f4fb92a6f57211fa45aabe545a6
>>> Signed-off-by: Simon Que <sque@chromium.org>
>>
>> hi,
>> looks good to me, adding Adrian to the loop

Yes, it looks fine except for the stuff below.

> 
> well, apart from this compile error ;-)
> 
> [jolsa@krava perf]$ make
>   BUILD:   Doing 'make -j4' parallel build
>   CC       util/machine.o
>   SUBDIR   /home/jolsa/kernel.org/linux-perf/tools/lib/traceevent/
> util/machine.c: In function ‘machine__get_kernel_start_addr’:
> util/machine.c:520:6: error: ‘addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   u64 addr;
>       ^
> util/machine.c: In function ‘machine__process_kernel_mmap_event’:
> util/machine.c:547:42: error: ‘addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    machine->vmlinux_maps[type] = map__new2(start, kernel, type);
>                                           ^
> util/machine.c:520:6: note: ‘addr’ was declared here
>   u64 addr;
>       ^
> cc1: all warnings being treated as errors
> make[1]: *** [util/machine.o] Error 1
> make: *** [all] Error 2
> 

There are checkpatch errors too:

ERROR: Remove Gerrit Change-Id's before submitting upstream.
#17:
Change-Id: I0c38c36f5e8b0f4fb92a6f57211fa45aabe545a6

ERROR: "foo** bar" should be "foo **bar"
#59: FILE: tools/perf/util/machine.c:515:
+                                         const char** symbol_name)

ERROR: "foo* bar" should be "foo *bar"
#64: FILE: tools/perf/util/machine.c:519:
+       const char* name;

total: 3 errors, 0 warnings, 0 checks, 90 lines checked



      reply	other threads:[~2014-06-16  9:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-13 18:45 [PATCH] perf: Get kernel start address by symbol name Simon Que
2014-06-16  8:06 ` Jiri Olsa
2014-06-16  8:09   ` Jiri Olsa
2014-06-16  9:31     ` Adrian Hunter [this message]

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=539EB974.9030904@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@ghostprotocols.net \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sonnyrao@chromium.org \
    --cc=sque@chromium.org \
    /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.