diff mbox series

[1/4] selftests/resctrl: Change initialize_llc_perf() return type to void

Message ID 20230208094026.22529-2-ilpo.jarvinen@linux.intel.com (mailing list archive)
State Accepted
Commit 9ce29d23a13399b44b8e52505464c5d0365ce086
Headers show
Series selftests/resctrl: Small cleanups | expand

Commit Message

Ilpo Järvinen Feb. 8, 2023, 9:40 a.m. UTC 
initialize_llc_perf() unconditionally does return 0 so no point in
having it's return type as int. Hence, change it's return type from int
to void.

Co-developed-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 tools/testing/selftests/resctrl/cache.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

Comments

Reinette Chatre Feb. 14, 2023, 12:59 a.m. UTC | #1 
Hi Ilpo,

On 2/8/2023 1:40 AM, Ilpo Järvinen wrote:
> initialize_llc_perf() unconditionally does return 0 so no point in
> having it's return type as int. Hence, change it's return type from int
> to void.
> 

Thank you very much for contributing to resctrl. As a new resctrl
contributor I would like to share that resctrl follows the x86
style guidance and to be consistent this is for the most part
true for the resctrl selftest area.

To that point, changelogs are easier to read if the context, problem,
and solution are clearly separated by placing them in separate
paragraphs. See "Changelog" in
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/maintainer-tip.rst

Please compare to a changelog as follows:

"
initialize_llc_perf() unconditionally returns 0.

initialize_llc_perf() performs memory initialization, none of
which can fail.

Change the return type from int to void to accurately
reflect that there is no checking of return value needed.
"

For such a small change as this, the changelog could possibly be
simplified but the context, problem, and solution should always
be clear to the reader. This may be significant changelog feedback
for such a small change. This is because it is your first patch to
this area and my goal is to point out the style that will help
your future resctrl contributions to have the pattern that
x86 maintainers expect.

Reinette
diff mbox series

Patch

diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c
index 68ff856d36f0..4b8ee81aedae 100644
--- a/tools/testing/selftests/resctrl/cache.c
+++ b/tools/testing/selftests/resctrl/cache.c
@@ -48,7 +48,7 @@  static int perf_event_open_llc_miss(pid_t pid, int cpu_no)
 	return 0;
 }
 
-static int initialize_llc_perf(void)
+static void initialize_llc_perf(void)
 {
 	memset(&pea_llc_miss, 0, sizeof(struct perf_event_attr));
 	memset(&rf_cqm, 0, sizeof(struct read_format));
@@ -59,8 +59,6 @@  static int initialize_llc_perf(void)
 	pea_llc_miss.config = PERF_COUNT_HW_CACHE_MISSES;
 
 	rf_cqm.nr = 1;
-
-	return 0;
 }
 
 static int reset_enable_llc_perf(pid_t pid, int cpu_no)
@@ -234,11 +232,8 @@  int cat_val(struct resctrl_val_param *param)
 	if (ret)
 		return ret;
 
-	if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR))) {
-		ret = initialize_llc_perf();
-		if (ret)
-			return ret;
-	}
+	if (!strncmp(resctrl_val, CAT_STR, sizeof(CAT_STR)))
+		initialize_llc_perf();
 
 	/* Test runs until the callback setup() tells the test to stop. */
 	while (1) {