Re: [LTP] [PATCH v4] high_freq_hwp_cap_cppc.c: new test

[Andrea Cervesato via ltp <ltp@lists.linux.it>]

Hi Piotr,

A few issues below.

> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*
> + * Copyright (C) 2026 Intel - http://www.intel.com/

The blank line between the SPDX identifier and the copyright block is
non-standard. They should be adjacent (no blank line between them).

> +	nproc = tst_ncpus();

tst_ncpus() returns _SC_NPROCESSORS_ONLN — the *count* of online CPUs,
not the highest CPU number. On a system where CPUs 0, 1, 3 are online
and CPU 2 is offline, tst_ncpus() returns 3, and the loop below iterates
over CPUs 0, 1, 2, accessing the wrong set of sysfs paths.

Use tst_ncpus_conf() as the upper bound and skip offline CPUs
individually, or iterate over the actual online CPU numbers from
/sys/devices/system/cpu/online.

> +		snprintf(path, sizeof(path), "/sys/devices/system/cpu/cpu%d/acpi_cppc/highest_perf", i);
> +		SAFE_FILE_SCANF(path, "%llu", &sysfs_highest_perf);

CONFIG_ACPI_CPPC_LIB=y does not guarantee the acpi_cppc/ sysfs
directory exists at runtime — it only appears when the CPU firmware
exposes the CPPC _CPC ACPI method. If the path is absent, SAFE_FILE_SCANF
aborts with TBROK. Check for existence and return TCONF instead:

  if (access(path, F_OK)) {
      tst_res(TCONF, "CPPC sysfs not available, skipping");
      return;
  }

> +		int fd = SAFE_OPEN(path, O_RDONLY);
> +
> +		SAFE_PREAD(1, fd, &msr_highest_perf, sizeof(msr_highest_perf), 0x771);
> +		msr_highest_perf &= (1ULL << 8) - 1;
> +		tst_res(TDEBUG, "%s: %llu", path, msr_highest_perf);
> +
> +		if (msr_highest_perf != sysfs_highest_perf)
> +			status = false;

Two issues:

1. fd is never closed. Opening it once per CPU iteration leaks one
   file descriptor per CPU. Add SAFE_CLOSE(fd) at the end of the loop
   body.

2. When the values differ, status is set to false but there is no
   per-CPU message. The single TFAIL at the end gives no indication of
   which CPU triggered the failure. Add a TINFO or TFAIL per CPU:

     tst_res(TFAIL, "cpu%d: sysfs=%llu MSR=%llu",
             i, sysfs_highest_perf, msr_highest_perf);


Regards,
--
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp