Re: [PATCH v3 1/6] cxl/test: Add test for module parameters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The function `have_multiple_modparms()` fails to check the `hmem_test` module parameter, allowing multiple test parameters to be activated simultaneously.
- [Low] The newly added function `have_multiple_modparms` is missing the `static` modifier, making it globally visible.
--

commit a298e853a144eb87f633b800d6fcf071d2088371
Author: Dave Jiang <dave.jiang@intel.com>

cxl/test: Add test for module parameters

This commit adds a test during module initialization to ensure that only
a single module parameter is activated at a time.

> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 296516eecfd6a..b00f5161b5f0c 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -1814,11 +1814,29 @@ static struct attribute *cxl_acpi_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(cxl_acpi);
>  
> +bool __init have_multiple_modparms(void)

[Severity: Low]
Should this function be declared static? It appears to only be used locally
within this file, and omitting the static keyword exports it to the global
namespace, which might trigger missing-prototype compiler warnings.

> +{
> +	int count = 0;
> +
> +	if (interleave_arithmetic)
> +		count++;
> +	if (extended_linear_cache)
> +		count++;
> +	if (fail_autoassemble)
> +		count++;

[Severity: Medium]
Does this check miss the hmem_test parameter? The cxl_test module is built
from both cxl.c and hmem_test.c, and hmem_test.c defines its own parameter.
It seems a user could load the module with hmem_test=1 and one of the local
parameters (like extended_linear_cache=1) concurrently, bypassing this check.

> +
> +	return count > 1;
> +}
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610222328.2272427-1-dave.jiang@intel.com?part=1