Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1758159AbcJQNwp (ORCPT <rfc822;w@1wt.eu>);
        Mon, 17 Oct 2016 09:52:45 -0400
Received: from mail-yw0-f179.google.com ([209.85.161.179]:34218 "EHLO
        mail-yw0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1754734AbcJQNwh (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 17 Oct 2016 09:52:37 -0400
MIME-Version: 1.0
In-Reply-To: <20161014152005.GC12815@kernel.org>
References: <1476356383-30100-1-git-send-email-eranian@google.com>
 <1476356383-30100-2-git-send-email-eranian@google.com> <CACbG30--zoPwzFqda3Dns2sRibdMkPha+agNip82aCLBGoGjjQ@mail.gmail.com>
 <20161014111359.GB12815@kernel.org> <CABPqkBRGwCiMBnFoW+jj20RFcwJv3MHeAPhUE4KctrBTXheN+A@mail.gmail.com>
 <20161014152005.GC12815@kernel.org>
From: Stephane Eranian <eranian@google.com>
Date: Mon, 17 Oct 2016 06:52:36 -0700
Message-ID: <CABPqkBTVFVo1P4ukDBjq-0YJ+n0ZCxYY3R3wcF8VbcD5=tNpZA@mail.gmail.com>
Subject: Re: [PATCH 1/9] perf/jit: improve error messages from JVMTI
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Nilay Vaish <nilayvaish@gmail.com>,
        Linux Kernel list <linux-kernel@vger.kernel.org>,
        Jiri Olsa <jolsa@redhat.com>, Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>, Anton Blanchard <anton@ozlabs.org>,
        Namhyung Kim <namhyung@kernel.org>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1908
Lines: 44

On Fri, Oct 14, 2016 at 8:20 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Fri, Oct 14, 2016 at 05:57:25AM -0700, Stephane Eranian escreveu:
>> On Fri, Oct 14, 2016 at 4:13 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>> > Em Thu, Oct 13, 2016 at 03:05:40PM -0500, Nilay Vaish escreveu:
>> >> Do we not need to release the memory for err_msg if the condition for
>> >> the 'if' statement evaluates to false?  Is it that we are going to
>> >> kill the process, so no need to release the memory?
>
>> > I guess that print_error() is called only when an error was returned
>> > somewhere, that ret parameter, then if there was no error
>> > (JVMTI_ERROR_NONE) in translating that numeric code to an string,
>> > err_msg, it can then be used with warnx() (the main purpose of
>> > print_error()) and then deallocated.
>> >
>> > For err != JVMTI_ERROR_NONE it silently goes back to the caller that
>> > expected it to print something.
>> >
>> > I.e. probably it should have an else clause, something like:
>> >
>> >         if (err == JVMTI_ERROR_NONE) {
>> >                 warnx("%s failed with %s", msg, err_msg);
>> >                 (*jvmti)->Deallocate(jvmti, (unsigned char *)err_msg);
>> >         } else {
>> >                 warnx("%s failed with an unknown error %d", msg, (int)ret);
>> >         }
>> >
>> > Stephane?
>> I will fix all of the comments over the week-end. I am away from office today.
>
> I have almost all of them fixed, will fix this one too, if you agree
> with the analysis.
>
Ok, the proposed fix is correct.
Thanks for fixing issues.

Now, I am tracking down another issue with the injection of jitdump.
It seems something goes wrong with ordering and timestamps and some
valid jitdump samples are not symbolized. It is not clear what is
causing this yet. It may be the issue raised by Adrian a while ago
about the finished_round processing.


> - Arnaldo