| Message ID | 20230301170918.69176-2-kjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [1/2] selftests/powerpc/pmu: Fix sample field check in the mmcra_thresh_marked_sample_test | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
On Wed, 2023-03-01 at 22:39 +0530, Kajol Jain wrote: > From: Madhavan Srinivasan <maddy@linux.ibm.com> > > event.h header already includes utlis.h. Avoid including > the same explicitly in the code when event.h included. > > Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> As I understand, transitive includes should not be depended upon. If you use a thing, and the thing is declared in a header, you should include _that_ header. Anything else is a recipe for weird include dependencies, ordering of the includes, etc. These files all use FAIL_IF, etc., which are declared in utils.h. So utils.h is a legitimate include. The fact that events.h also includes it (for u64) is a coincidence. If the u64 type def gets moved to, e.g., types.h, and utils.h is removed from events.h, suddenly all these files stop compiling.
On 3/2/23 3:35 AM, Benjamin Gray wrote: > On Wed, 2023-03-01 at 22:39 +0530, Kajol Jain wrote: >> From: Madhavan Srinivasan <maddy@linux.ibm.com> >> >> event.h header already includes utlis.h. Avoid including >> the same explicitly in the code when event.h included. >> >> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> > As I understand, transitive includes should not be depended upon. If > you use a thing, and the thing is declared in a header, you should > include _that_ header. Anything else is a recipe for weird include > dependencies, ordering of the includes, etc. > > These files all use FAIL_IF, etc., which are declared in utils.h. So > utils.h is a legitimate include. The fact that events.h also includes > it (for u64) is a coincidence. If the u64 type def gets moved to, e.g., > types.h, and utils.h is removed from events.h, suddenly all these files > stop compiling. thanks for the review. IIUC utils.h also carries the some test harness func declarations, also some of these tests does not use type defs anyway. I should have had a better commit message, my bad. But i will try out the suggested case. Maddy
On 3/2/23 8:49 AM, Madhavan Srinivasan wrote: > > On 3/2/23 3:35 AM, Benjamin Gray wrote: >> On Wed, 2023-03-01 at 22:39 +0530, Kajol Jain wrote: >>> From: Madhavan Srinivasan <maddy@linux.ibm.com> >>> >>> event.h header already includes utlis.h. Avoid including >>> the same explicitly in the code when event.h included. >>> >>> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com> >> As I understand, transitive includes should not be depended upon. If >> you use a thing, and the thing is declared in a header, you should >> include _that_ header. Anything else is a recipe for weird include >> dependencies, ordering of the includes, etc. >> >> These files all use FAIL_IF, etc., which are declared in utils.h. So >> utils.h is a legitimate include. The fact that events.h also includes >> it (for u64) is a coincidence. If the u64 type def gets moved to, e.g., >> types.h, and utils.h is removed from events.h, suddenly all these files >> stop compiling. > > thanks for the review. IIUC utils.h also carries the some test harness > func declarations, also some of these tests does not use type defs > anyway. I should have had a better commit message, my bad. But i will > try out the suggested case. yeah, "utils.h" included in the testcase files are for the tast_harness declarations. So we could get typedef moved from utils.h. Good catch. Thanks. Kajol, kindly drop this patch. Maddy > > Maddy
diff --git a/tools/testing/selftests/powerpc/pmu/count_instructions.c b/tools/testing/selftests/powerpc/pmu/count_instructions.c index a3984ef1e96a..a0f77065a1e0 100644 --- a/tools/testing/selftests/powerpc/pmu/count_instructions.c +++ b/tools/testing/selftests/powerpc/pmu/count_instructions.c @@ -11,7 +11,6 @@ #include <sys/prctl.h> #include "event.h" -#include "utils.h" #include "lib.h" extern void thirty_two_instruction_loop(u64 loops); diff --git a/tools/testing/selftests/powerpc/pmu/count_stcx_fail.c b/tools/testing/selftests/powerpc/pmu/count_stcx_fail.c index 2070a1e2b3a5..07ddac28d3cf 100644 --- a/tools/testing/selftests/powerpc/pmu/count_stcx_fail.c +++ b/tools/testing/selftests/powerpc/pmu/count_stcx_fail.c @@ -11,7 +11,6 @@ #include <sys/prctl.h> #include "event.h" -#include "utils.h" #include "lib.h" extern void thirty_two_instruction_loop_with_ll_sc(u64 loops, u64 *ll_sc_target); diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_cache_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_cache_test.c index f4be05aa3a3d..ad529b8a03ba 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_cache_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_cache_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* All L1 D cache load references counted at finish, gated by reject */ diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_l2l3_sel_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_l2l3_sel_test.c index 85a636886069..b2fa5fc54545 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_l2l3_sel_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_l2l3_sel_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* All successful D-side store dispatches for this thread */ diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_cmp_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_cmp_test.c index 9f1197104e8c..d77a730e00df 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_cmp_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_cmp_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_ctl_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_ctl_test.c index e0852ebc1671..f8b79953655a 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_ctl_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_ctl_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_sel_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_sel_test.c index 50a8cd843ce7..05eb1cf6c5f9 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_sel_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_thresh_sel_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_unit_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_unit_test.c index a2c18923dcec..151c0b1cff11 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_unit_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/group_constraint_unit_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* All successful D-side store dispatches for this thread with PMC 2 */ diff --git a/tools/testing/selftests/powerpc/pmu/event_code_tests/hw_cache_event_type_test.c b/tools/testing/selftests/powerpc/pmu/event_code_tests/hw_cache_event_type_test.c index a45b1da5b568..8020be598d9d 100644 --- a/tools/testing/selftests/powerpc/pmu/event_code_tests/hw_cache_event_type_test.c +++ b/tools/testing/selftests/powerpc/pmu/event_code_tests/hw_cache_event_type_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "../event.h" -#include "utils.h" #include "../sampling_tests/misc.h" /* diff --git a/tools/testing/selftests/powerpc/pmu/l3_bank_test.c b/tools/testing/selftests/powerpc/pmu/l3_bank_test.c index a5dfa9bf3b9f..29b3039f8fde 100644 --- a/tools/testing/selftests/powerpc/pmu/l3_bank_test.c +++ b/tools/testing/selftests/powerpc/pmu/l3_bank_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include "event.h" -#include "utils.h" #define MALLOC_SIZE (0x10000 * 10) /* Ought to be enough .. */ diff --git a/tools/testing/selftests/powerpc/pmu/per_event_excludes.c b/tools/testing/selftests/powerpc/pmu/per_event_excludes.c index ad32a09a6540..5d00220b62ba 100644 --- a/tools/testing/selftests/powerpc/pmu/per_event_excludes.c +++ b/tools/testing/selftests/powerpc/pmu/per_event_excludes.c @@ -14,7 +14,6 @@ #include "event.h" #include "lib.h" -#include "utils.h" /* * Test that per-event excludes work. diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_filter_map_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_filter_map_test.c index 3f43c315c666..6a639f255bfa 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_filter_map_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_filter_map_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* * A perf sampling test to check bhrb filter diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_no_crash_wo_pmu_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_no_crash_wo_pmu_test.c index 4644c6782974..a6deba5dea00 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_no_crash_wo_pmu_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/bhrb_no_crash_wo_pmu_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* * A perf sampling test for making sure diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/intr_regs_no_crash_wo_pmu_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/intr_regs_no_crash_wo_pmu_test.c index 839d2d225da0..a8c1197a577d 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/intr_regs_no_crash_wo_pmu_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/intr_regs_no_crash_wo_pmu_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* * A perf sampling test for making sure diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_cc56run_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_cc56run_test.c index ae4172f83817..c560073f0f2c 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_cc56run_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_cc56run_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_exceptionbits_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_exceptionbits_test.c index 982aa56d2171..f10e5de08302 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_exceptionbits_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_exceptionbits_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc1ce_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc1ce_test.c index 1c1813c182c0..02d32a5d81ef 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc1ce_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc1ce_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc56_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc56_test.c index 332d24b5ab9c..918f72c90e20 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc56_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_fc56_pmc56_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmccext_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmccext_test.c index dfd186cd8eec..26c5e3219d8b 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmccext_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmccext_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmcjce_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmcjce_test.c index fdd8ed9bf725..60584f64ed02 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmcjce_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr0_pmcjce_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_comb_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_comb_test.c index 5aea6499ee9a..0759a4efc050 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_comb_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_comb_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* All successful D-side store dispatches for this thread that were L2 Miss */ #define EventCode 0x46880 diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_sel_unit_cache_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_sel_unit_cache_test.c index f0c003282630..b1b34254018c 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_sel_unit_cache_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr1_sel_unit_cache_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" #define MALLOC_SIZE (0x10000 * 10) /* Ought to be enough .. */ diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c index 4e242fd61b25..ac3ee2b236c2 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_fcs_fch_test.c @@ -10,7 +10,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c index ceca597016b2..dd4e99dcb6b7 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr2_l2l3_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* All successful D-side store dispatches for this thread */ #define EventCode 0x010000046080 diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c index e154e2a4cc3a..b8792ee1004b 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcr3_src_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop_with_ll_sc(u64 loops, u64 *ll_sc_target); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_any_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_any_test.c index 14854694af62..e2d1c98602a3 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_any_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_any_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_cond_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_cond_test.c index 3e08176eb7f8..f502d52c0f5a 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_cond_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_cond_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_no_branch_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_no_branch_test.c index 488c865387e4..eae8634736c0 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_no_branch_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_no_branch_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_test.c index 186a853c0f62..73d66306c010 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_disable_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void thirty_two_instruction_loop(int loops); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_ind_call_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_ind_call_test.c index f0706730c099..c5905303e657 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_ind_call_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_bhrb_ind_call_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" extern void indirect_branch_loop(void); diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_cmp_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_cmp_test.c index 904362f172c9..ab2dc52766f6 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_cmp_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_cmp_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* * Primary PMU event used here is PM_MRK_INST_CMPL (0x401e0) diff --git a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c index 75527876ad3c..6f47e2211163 100644 --- a/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c +++ b/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c @@ -8,7 +8,6 @@ #include "../event.h" #include "misc.h" -#include "utils.h" /* * Primary PMU event used here is PM_MRK_INST_CMPL (0x401e0)