| Message ID | 1567612484-195727-1-git-send-email-john.garry@huawei.com |
|---|---|
| Headers | show |
| Series | HiSilicon hip08 uncore PMU events additions | expand |
On 04/09/2019 16:54, John Garry wrote: > This patchset adds some missing uncore PMU events for the hip08 arm64 > platform. > > The missing events were originally mentioned in > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > It also includes a fix for a DDRC eventname. Hi guys, Could I get these JSON updates picked up please? Maybe they were missed earlier. Let me know if I should re-post. Thanks in advance, John > > John Garry (4): > perf jevents: Fix Hisi hip08 DDRC PMU eventname > perf jevents: Add some missing events for Hisi hip08 DDRC PMU > perf jevents: Add some missing events for Hisi hip08 L3C PMU > perf jevents: Add some missing events for Hisi hip08 HHA PMU > > .../arm64/hisilicon/hip08/uncore-ddrc.json | 16 +++++- > .../arm64/hisilicon/hip08/uncore-hha.json | 23 +++++++- > .../arm64/hisilicon/hip08/uncore-l3c.json | 56 +++++++++++++++++++ > 3 files changed, 93 insertions(+), 2 deletions(-) >
Em Fri, Oct 04, 2019 at 03:30:07PM +0100, John Garry escreveu: > On 04/09/2019 16:54, John Garry wrote: > > This patchset adds some missing uncore PMU events for the hip08 arm64 > > platform. > > > > The missing events were originally mentioned in > > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > > > It also includes a fix for a DDRC eventname. > > Hi guys, > > Could I get these JSON updates picked up please? Maybe they were missed > earlier. Let me know if I should re-post. Looking at them now. - Arnaldo > Thanks in advance, > John > > > > > John Garry (4): > > perf jevents: Fix Hisi hip08 DDRC PMU eventname > > perf jevents: Add some missing events for Hisi hip08 DDRC PMU > > perf jevents: Add some missing events for Hisi hip08 L3C PMU > > perf jevents: Add some missing events for Hisi hip08 HHA PMU > > > > .../arm64/hisilicon/hip08/uncore-ddrc.json | 16 +++++- > > .../arm64/hisilicon/hip08/uncore-hha.json | 23 +++++++- > > .../arm64/hisilicon/hip08/uncore-l3c.json | 56 +++++++++++++++++++ > > 3 files changed, 93 insertions(+), 2 deletions(-) > > > -- - Arnaldo
Em Fri, Oct 04, 2019 at 11:36:58AM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Oct 04, 2019 at 03:30:07PM +0100, John Garry escreveu: > > On 04/09/2019 16:54, John Garry wrote: > > > This patchset adds some missing uncore PMU events for the hip08 arm64 > > > platform. > > > > > > The missing events were originally mentioned in > > > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > > > > > It also includes a fix for a DDRC eventname. > > > > Hi guys, > > > > Could I get these JSON updates picked up please? Maybe they were missed > > earlier. Let me know if I should re-post. > > Looking at them now. It would be really good if somehow we managed to have someone from the ARM community to check and provide a Reviewed-by for those, i.e. someone else than the poster to look at it and check that its ok, would that be possible? - Arnaldo > - Arnaldo > > > Thanks in advance, > > John > > > > > > > > John Garry (4): > > > perf jevents: Fix Hisi hip08 DDRC PMU eventname > > > perf jevents: Add some missing events for Hisi hip08 DDRC PMU > > > perf jevents: Add some missing events for Hisi hip08 L3C PMU > > > perf jevents: Add some missing events for Hisi hip08 HHA PMU > > > > > > .../arm64/hisilicon/hip08/uncore-ddrc.json | 16 +++++- > > > .../arm64/hisilicon/hip08/uncore-hha.json | 23 +++++++- > > > .../arm64/hisilicon/hip08/uncore-l3c.json | 56 +++++++++++++++++++ > > > 3 files changed, 93 insertions(+), 2 deletions(-) > > > > > > > -- > > - Arnaldo -- - Arnaldo
On Fri, Oct 04, 2019 at 11:38:35AM -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Oct 04, 2019 at 11:36:58AM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Fri, Oct 04, 2019 at 03:30:07PM +0100, John Garry escreveu: > > > On 04/09/2019 16:54, John Garry wrote: > > > > This patchset adds some missing uncore PMU events for the hip08 arm64 > > > > platform. > > > > > > > > The missing events were originally mentioned in > > > > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > > > > > > > It also includes a fix for a DDRC eventname. > > > > > > Hi guys, > > > > > > Could I get these JSON updates picked up please? Maybe they were missed > > > earlier. Let me know if I should re-post. > > > > Looking at them now. > > It would be really good if somehow we managed to have someone from the > ARM community to check and provide a Reviewed-by for those, i.e. someone > else than the poster to look at it and check that its ok, would that be > possible? The patches look fine to me, but the IP being supported here is designed by Hisilicon so my Arm knowledge isn't very helpful as it's outside the scope of the architecture. Will
On 04/10/2019 15:38, Arnaldo Carvalho de Melo wrote: > Em Fri, Oct 04, 2019 at 11:36:58AM -0300, Arnaldo Carvalho de Melo escreveu: >> Em Fri, Oct 04, 2019 at 03:30:07PM +0100, John Garry escreveu: >>> On 04/09/2019 16:54, John Garry wrote: >>>> This patchset adds some missing uncore PMU events for the hip08 arm64 >>>> platform. >>>> >>>> The missing events were originally mentioned in >>>> https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. >>>> >>>> It also includes a fix for a DDRC eventname. >>> >>> Hi guys, >>> >>> Could I get these JSON updates picked up please? Maybe they were missed >>> earlier. Let me know if I should re-post. >> >> Looking at them now. > > It would be really good if somehow we managed to have someone from the > ARM community to check and provide a Reviewed-by for those, i.e. someone > else than the poster to look at it and check that its ok, would that be > possible? Hi Arnaldo, For this specific case, I'm not sure how much traction or value there would be since we're just adding some missing events for custom IP. But I do agree that more review of JSONs from the community is required - as I brought up here regarding a recent addition: https://lore.kernel.org/lkml/749a0b8e-2bfd-28f6-b34d-dc72ef3d3a74@huawei.com/ Can we enforce that at least linux-arm-kernel@lists.infradead.org and/or get_maintainer.pl results is cc'ed on anything ARM specific as a start? Cheers, John > > - Arnaldo > >> - Arnaldo >> >>> Thanks in advance, >>> John >>> >>>> >>>> John Garry (4): >>>> perf jevents: Fix Hisi hip08 DDRC PMU eventname >>>> perf jevents: Add some missing events for Hisi hip08 DDRC PMU >>>> perf jevents: Add some missing events for Hisi hip08 L3C PMU >>>> perf jevents: Add some missing events for Hisi hip08 HHA PMU >>>> >>>> .../arm64/hisilicon/hip08/uncore-ddrc.json | 16 +++++- >>>> .../arm64/hisilicon/hip08/uncore-hha.json | 23 +++++++- >>>> .../arm64/hisilicon/hip08/uncore-l3c.json | 56 +++++++++++++++++++ >>>> 3 files changed, 93 insertions(+), 2 deletions(-) >>>> >>> >> >> -- >> >> - Arnaldo >
Em Fri, Oct 04, 2019 at 03:59:44PM +0100, John Garry escreveu: > On 04/10/2019 15:38, Arnaldo Carvalho de Melo wrote: > > Em Fri, Oct 04, 2019 at 11:36:58AM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Fri, Oct 04, 2019 at 03:30:07PM +0100, John Garry escreveu: > > > > On 04/09/2019 16:54, John Garry wrote: > > > > > This patchset adds some missing uncore PMU events for the hip08 arm64 > > > > > platform. > > > > > The missing events were originally mentioned in > > > > > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > > > > It also includes a fix for a DDRC eventname. > > > > Could I get these JSON updates picked up please? Maybe they were missed > > > > earlier. Let me know if I should re-post. > > > Looking at them now. > > It would be really good if somehow we managed to have someone from the > > ARM community to check and provide a Reviewed-by for those, i.e. someone > > else than the poster to look at it and check that its ok, would that be > > possible? > For this specific case, I'm not sure how much traction or value there would > be since we're just adding some missing events for custom IP. Someone else inside your organization? If nobody is available, then so be it, let that be clear in the JSON file (see below) and then I wouldn't be waiting for acks/reviewed-by messages for such cases. > But I do agree that more review of JSONs from the community is required - as > I brought up here regarding a recent addition: > https://lore.kernel.org/lkml/749a0b8e-2bfd-28f6-b34d-dc72ef3d3a74@huawei.com/ > > Can we enforce that at least linux-arm-kernel@lists.infradead.org and/or > get_maintainer.pl results is cc'ed on anything ARM specific as a start? I think this should be the case, would you be willing to add a note to that effect at the top of the JSON files? And an extra note at tools/perf/pmu-events/README telling users to look at the json files to figure out what Reviewed-by tags are required for something to get merged? One extra Reviewed-by would be ok? Who would be the reviewers for each arch? Would that be at the top of the JSON file? - Arnaldo
> >>>>>> The missing events were originally mentioned in >>>>>> https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > >>>>>> It also includes a fix for a DDRC eventname. > >>>>> Could I get these JSON updates picked up please? Maybe they were missed >>>>> earlier. Let me know if I should re-post. > >>>> Looking at them now. > >>> It would be really good if somehow we managed to have someone from the >>> ARM community to check and provide a Reviewed-by for those, i.e. someone >>> else than the poster to look at it and check that its ok, would that be >>> possible? > >> For this specific case, I'm not sure how much traction or value there would >> be since we're just adding some missing events for custom IP. > > Someone else inside your organization? For this, sure. Colleague Shaokun (cc'ed) provided me the metadata for these JSON additions, so when he returns from national vacation I can ask him to provide necessary tags. If nobody is available, then so > be it, let that be clear in the JSON file (see below) and then I > wouldn't be waiting for acks/reviewed-by messages for such cases. > >> But I do agree that more review of JSONs from the community is required - as >> I brought up here regarding a recent addition: >> https://lore.kernel.org/lkml/749a0b8e-2bfd-28f6-b34d-dc72ef3d3a74@huawei.com/ >> >> Can we enforce that at least linux-arm-kernel@lists.infradead.org and/or >> get_maintainer.pl results is cc'ed on anything ARM specific as a start? > > I think this should be the case, would you be willing to add a note to > that effect at the top of the JSON files? Adding notes to JSONs would be painful unless the parser is updated to to filter them out. And, as I understand, the x86 JSONs are autogenerated, so that tooling would need to handle this also. > > And an extra note at tools/perf/pmu-events/README telling users to look > at the json files to figure out what Reviewed-by tags are required for > something to get merged? One extra Reviewed-by would be ok?Who would be > the reviewers for each arch? Would that be at the top of the JSON file? There is no per-arch JSON, and, in addition, I think that would be hard to formulate such formal rules. As an alternative, how about just add a maintainers entry for reviewers per arch? As a start, I don't mind being added there for arm64: --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12767,6 +12767,10 @@ F: arch/*/events/* F: arch/*/events/*/* F: tools/perf/ +PERFORMANCE EVENTS SUBSYSTEM ARM64 PMU EVENTS +R: John Garry <john.garry@huawei.com> +F: tools/perf/pmu-events/arch/arm64 + Patches per-arch should have some nod/tag from a member of the respective list. Or at very least be cc'ed :) Thanks, John > > - Arnaldo > > . >
Em Fri, Oct 04, 2019 at 05:30:46PM +0100, John Garry escreveu: > > > > > > > > > The missing events were originally mentioned in > > > > > > > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > > > > > > > > It also includes a fix for a DDRC eventname. > > > > > > > > Could I get these JSON updates picked up please? Maybe they were missed > > > > > > earlier. Let me know if I should re-post. > > > > > > > Looking at them now. > > > > > > It would be really good if somehow we managed to have someone from the > > > > ARM community to check and provide a Reviewed-by for those, i.e. someone > > > > else than the poster to look at it and check that its ok, would that be > > > > possible? > > > > > For this specific case, I'm not sure how much traction or value there would > > > be since we're just adding some missing events for custom IP. > > > > Someone else inside your organization? > > For this, sure. Colleague Shaokun (cc'ed) provided me the metadata for these > JSON additions, so when he returns from national vacation I can ask him to > provide necessary tags. Ok > If nobody is available, then so > > be it, let that be clear in the JSON file (see below) and then I > > wouldn't be waiting for acks/reviewed-by messages for such cases. > > > > > But I do agree that more review of JSONs from the community is required - as > > > I brought up here regarding a recent addition: > > > https://lore.kernel.org/lkml/749a0b8e-2bfd-28f6-b34d-dc72ef3d3a74@huawei.com/ > > > > > > Can we enforce that at least linux-arm-kernel@lists.infradead.org and/or > > > get_maintainer.pl results is cc'ed on anything ARM specific as a start? > > > > I think this should be the case, would you be willing to add a note to > > that effect at the top of the JSON files? > > Adding notes to JSONs would be painful unless the parser is updated to to > filter them out. And, as I understand, the x86 JSONs are autogenerated, so > that tooling would need to handle this also. Ok > > > > And an extra note at tools/perf/pmu-events/README telling users to look > > at the json files to figure out what Reviewed-by tags are required for > > something to get merged? One extra Reviewed-by would be ok?Who would be > > the reviewers for each arch? Would that be at the top of the JSON file? > > There is no per-arch JSON, and, in addition, I think that would be hard to > formulate such formal rules. Ok > As an alternative, how about just add a maintainers entry for reviewers per > arch? As a start, I don't mind being added there for arm64: > > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12767,6 +12767,10 @@ F: arch/*/events/* > F: arch/*/events/*/* > F: tools/perf/ > > +PERFORMANCE EVENTS SUBSYSTEM ARM64 PMU EVENTS > +R: John Garry <john.garry@huawei.com> > +F: tools/perf/pmu-events/arch/arm64 > + > > Patches per-arch should have some nod/tag from a member of the respective > list. Or at very least be cc'ed :) Another Ok, please send a formal patch, and it would be really nice if the other ARM folks would... Ack that ;-) :-) And provide extra entries for the other pmu-events directories or even for specific files, which is a possibility, right? On my side I'll script a bit more and make sure that a post commit hook warns me if the right tag is not present. - Arnaldo
> >> As an alternative, how about just add a maintainers entry for reviewers per >> arch? As a start, I don't mind being added there for arm64: >> >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -12767,6 +12767,10 @@ F: arch/*/events/* >> F: arch/*/events/*/* >> F: tools/perf/ >> >> +PERFORMANCE EVENTS SUBSYSTEM ARM64 PMU EVENTS >> +R: John Garry <john.garry@huawei.com> >> +F: tools/perf/pmu-events/arch/arm64 >> + >> >> Patches per-arch should have some nod/tag from a member of the respective >> list. Or at very least be cc'ed :) > > Another Ok, please send a formal patch, and it would be really nice if > the other ARM folks would... Ack that ;-) :-) And provide extra entries > for the other pmu-events directories or even for specific files, which > is a possibility, right? ok, can do. Will has kindly agreed to have his name added to the MAINTAINERS entry (thanks). Other Cc's from ARM community may also be interested to be included - shout if so. So I'll just include tools/perf/pmu-events/arch/arm64 for now. The code in tools/perf/pmu-events/. should be generic for all architectures, while I'd say tools/perf/arch/arm64 is not strictly related. Thanks, John > > On my side I'll script a bit more and make sure that a post commit hook > warns me if the right tag is not present. > > - Arnaldo > > . >
Hi John, Thanks for your nice work, these are useful for performance profiling if anyone is unfamiliar with the uncore PMU events on hip08. For this patchset, please feel free to add Reviewed-by: Shaokun Zhang <zhangshaokun@hisilicon.com> Thanks, Shaokun On 2019/9/4 23:54, John Garry wrote: > This patchset adds some missing uncore PMU events for the hip08 arm64 > platform. > > The missing events were originally mentioned in > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > It also includes a fix for a DDRC eventname. > > John Garry (4): > perf jevents: Fix Hisi hip08 DDRC PMU eventname > perf jevents: Add some missing events for Hisi hip08 DDRC PMU > perf jevents: Add some missing events for Hisi hip08 L3C PMU > perf jevents: Add some missing events for Hisi hip08 HHA PMU > > .../arm64/hisilicon/hip08/uncore-ddrc.json | 16 +++++- > .../arm64/hisilicon/hip08/uncore-hha.json | 23 +++++++- > .../arm64/hisilicon/hip08/uncore-l3c.json | 56 +++++++++++++++++++ > 3 files changed, 93 insertions(+), 2 deletions(-) >
Em Wed, Oct 09, 2019 at 09:14:45AM +0800, Shaokun Zhang escreveu: > Hi John, > > Thanks for your nice work, these are useful for performance profiling > if anyone is unfamiliar with the uncore PMU events on hip08. > For this patchset, please feel free to add > Reviewed-by: Shaokun Zhang <zhangshaokun@hisilicon.com> Thanks, added and applied, - Arnaldo > Thanks, > Shaokun > > On 2019/9/4 23:54, John Garry wrote: > > This patchset adds some missing uncore PMU events for the hip08 arm64 > > platform. > > > > The missing events were originally mentioned in > > https://lkml.org/lkml/2019/6/14/645, when upstreaming the JSONs initially. > > > > It also includes a fix for a DDRC eventname. > > > > John Garry (4): > > perf jevents: Fix Hisi hip08 DDRC PMU eventname > > perf jevents: Add some missing events for Hisi hip08 DDRC PMU > > perf jevents: Add some missing events for Hisi hip08 L3C PMU > > perf jevents: Add some missing events for Hisi hip08 HHA PMU > > > > .../arm64/hisilicon/hip08/uncore-ddrc.json | 16 +++++- > > .../arm64/hisilicon/hip08/uncore-hha.json | 23 +++++++- > > .../arm64/hisilicon/hip08/uncore-l3c.json | 56 +++++++++++++++++++ > > 3 files changed, 93 insertions(+), 2 deletions(-) > > -- - Arnaldo