All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: John Garry <john.garry@huawei.com>,
	James Clark <james.clark@arm.com>,
	Andrew Kilroy <andrew.kilroy@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-perf-users@vger.kernel.org"
	<linux-perf-users@vger.kernel.org>,
	"acme@kernel.org" <acme@kernel.org>
Cc: Will Deacon <will@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json
Date: Thu, 7 Oct 2021 17:03:25 -0700	[thread overview]
Message-ID: <b24041af-1c92-3855-9659-133e73a0c241@linux.intel.com> (raw)
In-Reply-To: <83767166-e379-a352-d920-ad8b6e923800@huawei.com>


On 10/6/2021 9:26 AM, John Garry wrote:
> On 06/10/2021 09:43, James Clark wrote:
>
> + Andi
>
>>
>>
>> On 05/10/2021 11:10, John Garry wrote:
>>> On 04/10/2021 17:00, Andrew Kilroy wrote:
>>>> There are some syntactical mistakes in the json files for the 
>>>> Cortex A76
>>>> N1 (Neoverse N1).  This was obstructing parsing from an external tool.
>>>
>>> If the trailing comma is not allowed by standard, then maybe we 
>>> should fix our parsing tool to not allow it also. However maybe 
>>> there is a good reason why we allow it..
>>
>> It would be nice to do, because I have also made similar fixes 
>> before. We looked at the STRICT option
>> in the parser (https://github.com/zserge/jsmn), but even then it 
>> seems to allow trailing commas.
>>
>> Trailing commas are not allowed in the json standard, but there is a 
>> split between parsers
>> where some allow it and others don't. Specifically the Python parser 
>> doesn't allow it, and Python
>> can easily be involved in some workflow that parses these files.
>>
>> The only way forwards I can think of is either getting a change 
>> accepted upstream to the parser
>> and then updating it in perf, switching to a different parser, or 
>> doing some hack to add an extra
>> step in perf to look for commas. None of which sound ideal.
>>
>
> Looking at the license in jsmn.c, we seem to be ok to modify it (to 
> error on non-standard trailing ',') - that parser has already 
> apparently been modified in mainline.
>
> If we do that then I hope that there are not to many violations in out 
> JSONs, including downstream.


Sure we can modify the file. I already did some minor changes when I 
submitted it originally.

-Andi



WARNING: multiple messages have this Message-ID (diff)
From: Andi Kleen <ak@linux.intel.com>
To: John Garry <john.garry@huawei.com>,
	James Clark <james.clark@arm.com>,
	Andrew Kilroy <andrew.kilroy@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-perf-users@vger.kernel.org"
	<linux-perf-users@vger.kernel.org>,
	"acme@kernel.org" <acme@kernel.org>
Cc: Will Deacon <will@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Leo Yan <leo.yan@linaro.org>, Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json
Date: Thu, 7 Oct 2021 17:03:25 -0700	[thread overview]
Message-ID: <b24041af-1c92-3855-9659-133e73a0c241@linux.intel.com> (raw)
In-Reply-To: <83767166-e379-a352-d920-ad8b6e923800@huawei.com>


On 10/6/2021 9:26 AM, John Garry wrote:
> On 06/10/2021 09:43, James Clark wrote:
>
> + Andi
>
>>
>>
>> On 05/10/2021 11:10, John Garry wrote:
>>> On 04/10/2021 17:00, Andrew Kilroy wrote:
>>>> There are some syntactical mistakes in the json files for the 
>>>> Cortex A76
>>>> N1 (Neoverse N1).  This was obstructing parsing from an external tool.
>>>
>>> If the trailing comma is not allowed by standard, then maybe we 
>>> should fix our parsing tool to not allow it also. However maybe 
>>> there is a good reason why we allow it..
>>
>> It would be nice to do, because I have also made similar fixes 
>> before. We looked at the STRICT option
>> in the parser (https://github.com/zserge/jsmn), but even then it 
>> seems to allow trailing commas.
>>
>> Trailing commas are not allowed in the json standard, but there is a 
>> split between parsers
>> where some allow it and others don't. Specifically the Python parser 
>> doesn't allow it, and Python
>> can easily be involved in some workflow that parses these files.
>>
>> The only way forwards I can think of is either getting a change 
>> accepted upstream to the parser
>> and then updating it in perf, switching to a different parser, or 
>> doing some hack to add an extra
>> step in perf to look for commas. None of which sound ideal.
>>
>
> Looking at the license in jsmn.c, we seem to be ok to modify it (to 
> error on non-standard trailing ',') - that parser has already 
> apparently been modified in mainline.
>
> If we do that then I hope that there are not to many violations in out 
> JSONs, including downstream.


Sure we can modify the file. I already did some minor changes when I 
submitted it originally.

-Andi



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-10-08  0:03 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04 16:00 [PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json Andrew Kilroy
2021-10-04 16:00 ` Andrew Kilroy
2021-10-04 16:00 ` [PATCH 2/4] perf vendor events: Add new armv8 pmu events Andrew Kilroy
2021-10-04 16:00   ` Andrew Kilroy
2021-10-05 10:01   ` John Garry
2021-10-05 10:01     ` John Garry
2021-10-04 16:00 ` [PATCH 3/4] perf vendor events: Categorise the Neoverse V1 counters Andrew Kilroy
2021-10-04 16:00   ` Andrew Kilroy
2021-10-04 16:00 ` [PATCH 4/4] perf vendor events: Add the Neoverse V1 to arm64 mapfile Andrew Kilroy
2021-10-04 16:00   ` Andrew Kilroy
2021-10-05  9:33   ` John Garry
2021-10-05  9:33     ` John Garry
2021-10-05 17:58     ` Arnaldo Carvalho de Melo
2021-10-05 17:58       ` Arnaldo Carvalho de Melo
2021-10-06  8:20       ` Andrew Kilroy
2021-10-06  8:20         ` Andrew Kilroy
2021-10-06  8:29         ` Andrew Kilroy
2021-10-06  8:29           ` Andrew Kilroy
2021-10-05 10:10 ` [PATCH 1/4] perf vendor events: Syntax corrections in Neoverse N1 json John Garry
2021-10-05 10:10   ` John Garry
2021-10-06  8:43   ` James Clark
2021-10-06  8:43     ` James Clark
2021-10-06 16:26     ` John Garry
2021-10-06 16:26       ` John Garry
2021-10-08  0:03       ` Andi Kleen [this message]
2021-10-08  0:03         ` Andi Kleen
2021-10-08  2:59         ` Ian Rogers
2021-10-08  2:59           ` Ian Rogers
2021-10-08 10:34           ` James Clark
2021-10-08 10:34             ` James Clark

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=b24041af-1c92-3855-9659-133e73a0c241@linux.intel.com \
    --to=ak@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=andrew.kilroy@arm.com \
    --cc=james.clark@arm.com \
    --cc=john.garry@huawei.com \
    --cc=jolsa@redhat.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=namhyung@kernel.org \
    --cc=will@kernel.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.