Consistently prefer sysfs/json events #878
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Upstream branch: ba5ea59 |
Factor out the case of an event or PMU name followed by a slash based term list. This is with a view to sharing the code with new legacy hardware parsing. Use early return to reduce indentation in the code. Make parse_events_add_pmu static now it doesn't need sharing with parse-events.y. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Avoid passing the name of a PMU then finding it again, just directly pass the PMU. parse_events_multi_pmu_add_or_add_pmu is the only version that needs to find a PMU, so move the find there. Remove the error message as parse_events_multi_pmu_add_or_add_pmu will given an error at the end when a name isn't either a PMU name or event name. Without the error message being created the location in the input parameter (loc) can be removed. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
In parse_events_add_pmu, delay copying the list of terms until it is known the list contains terms. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Move all implementation to pmu code. Don't allocate a fnmatch wildcard pattern, matching ignoring the suffix already handles this, and only use fnmatch if the given PMU name has a '*' in it. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Switch from cache-references to branches in test as Intel has a sysfs event for cache-references and changing the priority for sysfs over legacy causes the test to fail. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Prior behavior is to not look for legacy cache names in sysfs/json and to create events on all core PMUs. New behavior is to look for sysfs/json events first on all PMUs, for core PMUs add a legacy event if the sysfs/json event isn't present. This is done so that there is consistency with how event names in terms are handled and their prioritization of sysfs/json over legacy. It may make sense to use a legacy cache event name as an event name on a non-core PMU so we should allow it. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Avoid duplicate logic for name_or_raw and PE_TERM_HW by having a rule to turn PE_TERM_HW into a name_or_raw. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Allow the term list to be const so that other functions can pass const term lists. Add const as necessary to called functions. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
It was requested that RISC-V be able to add events to the perf tool so the PMU driver didn't need to map legacy events to config encodings: https://lore.kernel.org/lkml/20240217005738.3744121-1-atishp@rivosinc.com/ This change makes the priority of events specified without a PMU the same as those specified with a PMU, namely sysfs and json events are checked first before using the legacy encoding. The hw_term is made more generic as a hardware_event that encodes a pair of string and int value, allowing parse_events_multi_pmu_add to fall back on a known encoding when the sysfs/json adding fails for core events. As this covers PE_VALUE_SYM_HW, that token is removed and related code simplified. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
The helper function just wraps a splice and free. Making the free inline removes a comment, so then it just wraps a splice which we can make inline too. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Use the error handler from the parse_state to give a more informative
error message.
Before:
```
$ perf stat -e 'cycles/period=99999999999999999999/' true
event syntax error: 'cycles/period=99999999999999999999/'
\___ parser error
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
After:
```
$ perf stat -e 'cycles/period=99999999999999999999/' true
event syntax error: 'cycles/period=99999999999999999999/'
\___ parser error
event syntax error: '..les/period=99999999999999999999/'
\___ Bad base 10 number "99999999999999999999"
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Inline parse_events_evlist_error that is only used in
parse_events_error. Modify parse_events_error to not report a parser
error unless errors haven't already been reported. Make it clearer
that the latter case only happens for unrecognized input.
Before:
```
$ perf stat -e 'cycles/period=99999999999999999999/' true
event syntax error: 'cycles/period=99999999999999999999/'
\___ parser error
event syntax error: '..les/period=99999999999999999999/'
\___ Bad base 10 number "99999999999999999999"
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
$ perf stat -e 'cycles:xyz' true
event syntax error: 'cycles:xyz'
\___ parser error
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
After:
```
$ perf stat -e 'cycles/period=99999999999999999999/xyz' true
event syntax error: '..les/period=99999999999999999999/xyz'
\___ Bad base 10 number "99999999999999999999"
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
$ perf stat -e 'cycles:xyz' true
event syntax error: 'cycles:xyz'
\___ Unrecognized input
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
```
Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Use a struct/bitmap rather than a copied string from lexer.
In lexer give improved error message when too many precise flags are
given or repeated modifiers.
Before:
```
$ perf stat -e 'cycles:kuk' true
event syntax error: 'cycles:kuk'
\___ Bad modifier
...
$ perf stat -e 'cycles:pppp' true
event syntax error: 'cycles:pppp'
\___ Bad modifier
...
$ perf stat -e '{instructions:p,cycles:pp}:pp' -a true
event syntax error: '..cycles:pp}:pp'
\___ Bad modifier
...
```
After:
```
$ perf stat -e 'cycles:kuk' true
event syntax error: 'cycles:kuk'
\___ Duplicate modifier 'k' (kernel)
...
$ perf stat -e 'cycles:pppp' true
event syntax error: 'cycles:pppp'
\___ Maximum precise value is 3
...
$ perf stat -e '{instructions:p,cycles:pp}:pp' true
event syntax error: '..cycles:pp}:pp'
\___ Maximum combined precise value is 3, adding precision to "cycles:pp"
...
```
Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Moves 352 bytes from .data to .data.rel.ro. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Add comments. Ensure leader->group_name is freed before overwriting it. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
Add comments. Pass ownership of the event name to save on a strdup. Signed-off-by: Ian Rogers <irogers@google.com> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
|
Upstream branch: ba5ea59 |
bjoto
force-pushed
the
series/844479=>for-next
branch
from
d9fce9e
to
15a8cde
Compare
|
At least one diff in series https://patchwork.kernel.org/project/linux-riscv/list/?series=844930 irrelevant now. Closing PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: Consistently prefer sysfs/json events
version: 1
url: https://patchwork.kernel.org/project/linux-riscv/list/?series=844479