Skip to content

Log and document "hidden" feature to turn off adaptive trace at run-time #5154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
lgirdwood merged 4 commits into from
Jan 10, 2022
Merged

Log and document "hidden" feature to turn off adaptive trace at run-time #5154

lgirdwood merged 4 commits into from
Jan 10, 2022

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Jan 5, 2022 •
edited
Loading

+ other cleanups, comments and clarifications, see commit messages.

Zero functional change.

marc-hb added 3 commits January 5, 2022 22:59
@marc-hb
Replace perf_*_trace() macros with functions.
d1cd75e 
The fewer macros, the better.

Commit 1758279 ("perf: make trace part of perf counting
configurable") provided no specific rationale for using macros. It did
not explain why it passed some ignored arguments either - renamed
accordingly.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
trace.c: minor #ifdef simplification in trace_log_filtered()
ffc9e77 
Group two #ifdef into one.

Top of block variables do not require C99.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
trace: log "hidden" feature turning off adaptive filtering at run-time
b808167 
Also clarify related Kconfig help strings.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 6, 2022

The Ethernet error [ 811.432681] kernel: e1000e 0000:00:1f.6 eno1: Hardware Error in https://sof-ci.01.org/sofpr/PR5154/build11502/devicetest/?model=TGLU_UP_HDA_ZEPHYR&testcase=check-suspend-resume-5 is cleared unrelated to this PR. Everything else is green.

The new message INFO Adaptive filtering disabled by user appears in the FW logs.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jan 6, 2022 •
edited
Loading

@kkarask could you find someone to help with this?
https://quickbuild.igk.intel.com/build/8260870

master > Check if PR is approved @ ptlpmssmain01:8842 
Composite step 'Check if PR is approved' failed due to unsatisfied success condition.
master > Check if PR is approved > Check if PR author is authorized @ ptlpmssmain01:8842 
java.lang.IllegalArgumentException: Text must not be null or empty

Demoting to draft until QB can run.

UPDATE: https://quickbuild.igk.intel.com/build/8262346 is now green, dunno what happened. Actual tests still haven't run.

@marc-hb marc-hb marked this pull request as ready for review January 6, 2022 00:09
@marc-hb marc-hb marked this pull request as draft January 6, 2022 01:19
@marc-hb
trace.c: rename emit_recent_entry() to emit_suppressed_entry() + comm…
1c963e5 
…ents

This makes much clearer why this function is not called when the burst
is short enough.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb force-pushed the adaptive-filter-cleanup branch from f47a512 to 1c963e5 Compare January 6, 2022 07:09
@marc-hb marc-hb mentioned this pull request Jan 6, 2022
Merged
@marc-hb marc-hb marked this pull request as ready for review January 6, 2022 07:26
lgirdwood
lgirdwood reviewed Jan 6, 2022
View reviewed changes
src/lib/agent.c

DECLARE_TR_CTX(sa_tr, SOF_UUID(sa_uuid), LOG_LEVEL_INFO);

#define perf_sa_trace(pcd, sa) \
Copy link
Member

@lgirdwood lgirdwood Jan 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a macro preserve __func__ and __LINE__. Can you check the output is correct as before with this change ?

Copy link
Collaborator Author

@marc-hb marc-hb Jan 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this. It is still possible to use macros, I did not remove that feature. I even made that clearer, see the very first hunk in perf_cnt.h. However I replaced these two specific macros because each is local to a single .c file and invoked only once in that .c file. So in these two particular cases __LINE__ would be completely useless because git grep is more than enough for these two.

BTW we can't use __func__ because it's not compatible with the dictionary. __func__ is not a static string that can be concatenated and only 32bits parameters are supported.

@marc-hb marc-hb requested a review from kv2019i January 7, 2022 00:37
@kkarask
Copy link

kkarask commented Jan 10, 2022

@marc-hb CI tests already passed.

@lgirdwood lgirdwood merged commit e64a6cf into thesofproject:main Jan 10, 2022
@marc-hb marc-hb deleted the adaptive-filter-cleanup branch January 10, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lyakh lyakh lyakh approved these changes

@lgirdwood lgirdwood lgirdwood approved these changes

@akloniex akloniex Awaiting requested review from akloniex akloniex is a code owner

@dbaluta dbaluta Awaiting requested review from dbaluta dbaluta is a code owner

@lbetlej lbetlej Awaiting requested review from lbetlej lbetlej is a code owner

@libinyang libinyang Awaiting requested review from libinyang

@mmaka1 mmaka1 Awaiting requested review from mmaka1 mmaka1 is a code owner

@mrajwa mrajwa Awaiting requested review from mrajwa

@plbossart plbossart Awaiting requested review from plbossart plbossart is a code owner

@kv2019i kv2019i Awaiting requested review from kv2019i kv2019i is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@marc-hb @kkarask @lyakh @lgirdwood