diff mbox series

[v1,4/5] perf pmu-events: Remove duplicated riscv firmware event

Message ID 20240805194424.597244-4-irogers@google.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v1,1/5] perf test: Add build test for JEVENTS_ARCH=all | expand

Checks

Context Check Description
conchuod/vmtest-fixes-PR fail merge-conflict

Commit Message

Ian Rogers Aug. 5, 2024, 7:44 p.m. UTC
FW_SFENCE_VMA_RECEIVED is repeated twice in the file which will break
invariants in perf list as discussed in this thread:
https://lore.kernel.org/linux-perf-users/20240719081651.24853-1-eric.lin@sifive.com/

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json      | 3 ---
 tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json      | 3 ---
 .../perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json | 3 ---
 .../perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json | 3 ---
 4 files changed, 12 deletions(-)

Comments

Eric Lin Aug. 6, 2024, 3:54 a.m. UTC | #1
Hi Ian,

I've sent a patch to fix it and the patch already merged. Thanks.
https://lore.kernel.org/all/20240719115018.27356-1-eric.lin@sifive.com/

Best regards,
Eric Lin


On Tue, Aug 6, 2024 at 3:44 AM Ian Rogers <irogers@google.com> wrote:
>
> FW_SFENCE_VMA_RECEIVED is repeated twice in the file which will break
> invariants in perf list as discussed in this thread:
> https://lore.kernel.org/linux-perf-users/20240719081651.24853-1-eric.lin@sifive.com/
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json      | 3 ---
>  tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json      | 3 ---
>  .../perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json | 3 ---
>  .../perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json | 3 ---
>  4 files changed, 12 deletions(-)
>
> diff --git a/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json b/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
> index 9b4a032186a7..5a996fa4b837 100644
> --- a/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
> +++ b/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
> @@ -35,9 +35,6 @@
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
>    },
> -  {
> -    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
> -  },
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
>    },
> diff --git a/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json b/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json
> index 9b4a032186a7..5a996fa4b837 100644
> --- a/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json
> +++ b/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json
> @@ -35,9 +35,6 @@
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
>    },
> -  {
> -    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
> -  },
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
>    },
> diff --git a/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json b/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json
> index 9b4a032186a7..5a996fa4b837 100644
> --- a/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json
> +++ b/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json
> @@ -35,9 +35,6 @@
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
>    },
> -  {
> -    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
> -  },
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
>    },
> diff --git a/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json b/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json
> index 9b4a032186a7..5a996fa4b837 100644
> --- a/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json
> +++ b/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json
> @@ -35,9 +35,6 @@
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
>    },
> -  {
> -    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
> -  },
>    {
>      "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
>    },
> --
> 2.46.0.rc2.264.g509ed76dc8-goog
>
Ian Rogers Aug. 6, 2024, 4:05 a.m. UTC | #2
On Mon, Aug 5, 2024 at 8:54 PM Eric Lin <eric.lin@sifive.com> wrote:
>
> Hi Ian,
>
> I've sent a patch to fix it and the patch already merged. Thanks.
> https://lore.kernel.org/all/20240719115018.27356-1-eric.lin@sifive.com/

Hi Eric/Arnaldo,

Right, I already commented this should have gone through the
perf-tools-next tree:
https://lore.kernel.org/all/CAP-5=fV3NXkKsCP1WH0_qLRNpL+WuP8S3h1=cHaUMH5MFkVHQg@mail.gmail.com/
Arnaldo, please take Eric's patch in preference to this one.

Thanks,
Ian
Arnaldo Carvalho de Melo Aug. 6, 2024, 1:38 p.m. UTC | #3
On Mon, Aug 05, 2024 at 09:05:26PM -0700, Ian Rogers wrote:
> On Mon, Aug 5, 2024 at 8:54 PM Eric Lin <eric.lin@sifive.com> wrote:
> >
> > Hi Ian,
> >
> > I've sent a patch to fix it and the patch already merged. Thanks.
> > https://lore.kernel.org/all/20240719115018.27356-1-eric.lin@sifive.com/
> 
> Hi Eric/Arnaldo,
> 
> Right, I already commented this should have gone through the
> perf-tools-next tree:
> https://lore.kernel.org/all/CAP-5=fV3NXkKsCP1WH0_qLRNpL+WuP8S3h1=cHaUMH5MFkVHQg@mail.gmail.com/
> Arnaldo, please take Eric's patch in preference to this one.

So I removed your version from perf-tools-next, eventually we'll get
this merged upstream, I'm just checking this isn't going to get in our
way of testing what we have in perf-tools-next...


- Arnaldo
Arnaldo Carvalho de Melo Aug. 6, 2024, 1:50 p.m. UTC | #4
On Tue, Aug 06, 2024 at 10:38:07AM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Aug 05, 2024 at 09:05:26PM -0700, Ian Rogers wrote:
> > On Mon, Aug 5, 2024 at 8:54 PM Eric Lin <eric.lin@sifive.com> wrote:
> > > I've sent a patch to fix it and the patch already merged. Thanks.
> > > https://lore.kernel.org/all/20240719115018.27356-1-eric.lin@sifive.com/

> > Right, I already commented this should have gone through the
> > perf-tools-next tree:
> > https://lore.kernel.org/all/CAP-5=fV3NXkKsCP1WH0_qLRNpL+WuP8S3h1=cHaUMH5MFkVHQg@mail.gmail.com/
> > Arnaldo, please take Eric's patch in preference to this one.

> So I removed your version from perf-tools-next, eventually we'll get
> this merged upstream, I'm just checking this isn't going to get in our
> way of testing what we have in perf-tools-next...

As expected:

⬢[acme@toolbox perf-tools-next]$ git log --oneline -1 ; time make -C tools/perf build-test
<SNIP>
make_no_libdw_dwarf_unwind_O: cd . && make NO_LIBDW_DWARF_UNWIND=1 FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j28 O=/tmp/tmp.1xBTPCPJh1 DESTDIR=/tmp/tmp.6upZRBhUpN
          make_jevents_all_O: cd . && make JEVENTS_ARCH=all FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j28 O=/tmp/tmp.ncjKsS9Ddm DESTDIR=/tmp/tmp.z2lxV44jzL
cd . && make JEVENTS_ARCH=all FEATURES_DUMP=/home/acme/git/perf-tools-next/tools/perf/BUILD_TEST_FEATURE_DUMP -j28 O=/tmp/tmp.ncjKsS9Ddm DESTDIR=/tmp/tmp.z2lxV44jzL
  BUILD:   Doing 'make -j28' parallel build
Warning: Kernel ABI header differences:
  diff -u tools/include/uapi/drm/i915_drm.h include/uapi/drm/i915_drm.h
<SNIP>
  LD      /tmp/tmp.ncjKsS9Ddm/util/perf-util-in.o
  LD      /tmp/tmp.ncjKsS9Ddm/perf-util-in.o
  AR      /tmp/tmp.ncjKsS9Ddm/libperf-util.a
Traceback (most recent call last):
  File "/home/acme/git/perf-tools-next/tools/perf/pmu-events/jevents.py", line 1317, in <module>
    main()
  File "/home/acme/git/perf-tools-next/tools/perf/pmu-events/jevents.py", line 1308, in main
    ftw(arch_path, [], process_one_file)
  File "/home/acme/git/perf-tools-next/tools/perf/pmu-events/jevents.py", line 1245, in ftw
    ftw(item.path, parents + [item.name], action)
  File "/home/acme/git/perf-tools-next/tools/perf/pmu-events/jevents.py", line 1243, in ftw
    action(parents, item)
  File "/home/acme/git/perf-tools-next/tools/perf/pmu-events/jevents.py", line 646, in process_one_file
    print_pending_events()
  File "/home/acme/git/perf-tools-next/tools/perf/pmu-events/jevents.py", line 510, in print_pending_events
    assert event.name != last_name, f"Duplicate event: {last_pmu}/{last_name}/ in {_pending_events_tblname}"
           ^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Duplicate event: default_core/fw_sfence_vma_received/ in pmu_events__andes_ax45
make[5]: *** [pmu-events/Build:46: /tmp/tmp.ncjKsS9Ddm/pmu-events/pmu-events.c] Error 1
make[5]: *** Deleting file '/tmp/tmp.ncjKsS9Ddm/pmu-events/pmu-events.c'
make[4]: *** [Makefile.perf:763: /tmp/tmp.ncjKsS9Ddm/pmu-events/pmu-events-in.o] Error 2

So I'll have to cherry pick that patch and have it in perf-tools-next,
we'll get notified at some point that linux-next has the same patch,
will ack that, etc.

Please send patches for this area in the future to the perf-tools-next
tree.

- Arnaldo
Arnaldo Carvalho de Melo Aug. 6, 2024, 1:51 p.m. UTC | #5
On Tue, Aug 06, 2024 at 10:38:07AM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Aug 05, 2024 at 09:05:26PM -0700, Ian Rogers wrote:
> > On Mon, Aug 5, 2024 at 8:54 PM Eric Lin <eric.lin@sifive.com> wrote:
> > >
> > > Hi Ian,
> > >
> > > I've sent a patch to fix it and the patch already merged. Thanks.

Also, already merged where? Upstream? In the riscv kernel tree? where?

- Arnaldo

> > > https://lore.kernel.org/all/20240719115018.27356-1-eric.lin@sifive.com/
> > 
> > Hi Eric/Arnaldo,
> > 
> > Right, I already commented this should have gone through the
> > perf-tools-next tree:
> > https://lore.kernel.org/all/CAP-5=fV3NXkKsCP1WH0_qLRNpL+WuP8S3h1=cHaUMH5MFkVHQg@mail.gmail.com/
> > Arnaldo, please take Eric's patch in preference to this one.
> 
> So I removed your version from perf-tools-next, eventually we'll get
> this merged upstream, I'm just checking this isn't going to get in our
> way of testing what we have in perf-tools-next...
> 
> 
> - Arnaldo
Eric Lin Aug. 6, 2024, 3:50 p.m. UTC | #6
Hi Arnaldo,

On Tue, Aug 6, 2024 at 9:51 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> On Tue, Aug 06, 2024 at 10:38:07AM -0300, Arnaldo Carvalho de Melo wrote:
> > On Mon, Aug 05, 2024 at 09:05:26PM -0700, Ian Rogers wrote:
> > > On Mon, Aug 5, 2024 at 8:54 PM Eric Lin <eric.lin@sifive.com> wrote:
> > > >
> > > > Hi Ian,
> > > >
> > > > I've sent a patch to fix it and the patch already merged. Thanks.
>
> Also, already merged where? Upstream? In the riscv kernel tree? where?
>

The patch already merged to Linux upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=63ba5b0fb4f54db256ec43b3062b2606b383055d

Thanks.

Best regards,
Eric Lin

> - Arnaldo
>
> > > > https://lore.kernel.org/all/20240719115018.27356-1-eric.lin@sifive.com/
> > >
> > > Hi Eric/Arnaldo,
> > >
> > > Right, I already commented this should have gone through the
> > > perf-tools-next tree:
> > > https://lore.kernel.org/all/CAP-5=fV3NXkKsCP1WH0_qLRNpL+WuP8S3h1=cHaUMH5MFkVHQg@mail.gmail.com/
> > > Arnaldo, please take Eric's patch in preference to this one.
> >
> > So I removed your version from perf-tools-next, eventually we'll get
> > this merged upstream, I'm just checking this isn't going to get in our
> > way of testing what we have in perf-tools-next...
> >
> >
> > - Arnaldo
Arnaldo Carvalho de Melo Aug. 6, 2024, 5:04 p.m. UTC | #7
On Tue, Aug 06, 2024 at 11:50:01PM +0800, Eric Lin wrote:
> On Tue, Aug 6, 2024 at 9:51 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> > On Tue, Aug 06, 2024 at 10:38:07AM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Mon, Aug 05, 2024 at 09:05:26PM -0700, Ian Rogers wrote:
> > > > On Mon, Aug 5, 2024 at 8:54 PM Eric Lin <eric.lin@sifive.com> wrote:

> > > > > I've sent a patch to fix it and the patch already merged. Thanks.

> > Also, already merged where? Upstream? In the riscv kernel tree? where?

> The patch already merged to Linux upstream:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=63ba5b0fb4f54db256ec43b3062b2606b383055d

Ok, I'm resolving this with removing Ian's patch and doing a merge with
upstream to pick this patch and keep perf-tools-next building.

- Arnaldo
diff mbox series

Patch

diff --git a/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json b/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
index 9b4a032186a7..5a996fa4b837 100644
--- a/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
+++ b/tools/perf/pmu-events/arch/riscv/andes/ax45/firmware.json
@@ -35,9 +35,6 @@ 
   {
     "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
   },
-  {
-    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
-  },
   {
     "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
   },
diff --git a/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json b/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json
index 9b4a032186a7..5a996fa4b837 100644
--- a/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json
+++ b/tools/perf/pmu-events/arch/riscv/sifive/u74/firmware.json
@@ -35,9 +35,6 @@ 
   {
     "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
   },
-  {
-    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
-  },
   {
     "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
   },
diff --git a/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json b/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json
index 9b4a032186a7..5a996fa4b837 100644
--- a/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json
+++ b/tools/perf/pmu-events/arch/riscv/starfive/dubhe-80/firmware.json
@@ -35,9 +35,6 @@ 
   {
     "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
   },
-  {
-    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
-  },
   {
     "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
   },
diff --git a/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json b/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json
index 9b4a032186a7..5a996fa4b837 100644
--- a/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json
+++ b/tools/perf/pmu-events/arch/riscv/thead/c900-legacy/firmware.json
@@ -35,9 +35,6 @@ 
   {
     "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
   },
-  {
-    "ArchStdEvent": "FW_SFENCE_VMA_RECEIVED"
-  },
   {
     "ArchStdEvent": "FW_SFENCE_VMA_ASID_RECEIVED"
   },