Re: [PATCH 1/3] selftests/resctrl: Add L3_CAT_OCCUP test to verify CAT bounds occupancy

[Richard Cheng <icheng@nvidia.com>]

On Wed, Aug 05, 2026 at 03:18:40PM +0800, Reinette Chatre wrote:
> Hi Richard,
>

Hi Reinette,

Thanks for the review !
 
> On 6/8/26 4:06 AM, Richard Cheng wrote:
> > L3_CAT needs a CPU-exclusive cache portion, so it's skipped when MPAM
> > reports every CBM bit as shareable, leaving L3 allocation untested. CMT
> > only checks occupancy accuracy, not that a CBM actually limits it.
> 
> hmmm ... CMT test ensures that the LLC occupancy is within a % of the size
> of the cache allocation. To me this implies that it indeed tests that
> the CBM limits the allocation, no?
> 

Yes you're correct on this. I based the patch on older version of the CMT test.
After rebasing, I see the current CMT already uses a workload larger than the allocation
and configures the root group with the complementary CBM.

It now covers what I wanted to test.

I think we're good to drop this patch in v2.


> > 
> > L3_CAT_OCCUP gives a group a small CBM, run a workload spanning the
> > whole cache, and check every occupancy sample stays within the
> > allocation. An unenforced CBM would instead let occupancy grow to the
> > full cache.
> > 
> > Move CON_MON_LCC_OCCUP_PATH to resctrl.h to share it with CMT.
> > 
> > Signed-off-by: Richard Cheng <icheng@nvidia.com>
> > ---
> >  tools/testing/selftests/resctrl/cat_test.c    | 201 ++++++++++++++++++
> >  tools/testing/selftests/resctrl/cmt_test.c    |   3 -
> >  tools/testing/selftests/resctrl/resctrl.h     |   4 +
> >  .../testing/selftests/resctrl/resctrl_tests.c |   1 +
> >  4 files changed, 206 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
> > index f00b622c1460..16a947f1ed16 100644
> > --- a/tools/testing/selftests/resctrl/cat_test.c
> > +++ b/tools/testing/selftests/resctrl/cat_test.c
> > @@ -402,3 +402,204 @@ struct resctrl_test l2_noncont_cat_test = {
> >  	.feature_check = noncont_cat_feature_check,
> >  	.run_test = noncont_cat_run_test,
> >  };
> > +
> > +/*
> > + * L3_CAT_OCCUP - Verify that a CAT allocation bounds cache occupancy.
> > + *
> > + * Unlike L3_CAT (which measures interference between groups and needs an
> > + * exclusive cache portion), this test gives a control group a strict subset
> 
> Please let comment just refer to what this test does. These comments are unlikely
> to be updated if L3_CAT ever changes.
> 
> > + * of the CBM, then runs a benchmark whose buffer spans the *whole* cache -
> > + * i.e. much larger than the allocation. With CAT enforced, the group can
> > + * only keep its allocated portion resident, so llc_occupancy settles near
> > + * the allocation size. Without enforcement occupancy would instead climb
> > + * towards the full cache. This works even when all CBM bits are shareable
> > + * (where L3_CAT is skipped).
> 
> hmmm ... these statements state as fact what really depends on system load and
> interference that this test make no attempt to avoid.
> 
> > + */
> > +#define CAT_OCCUP_RESULT_FILE		"result_cat_occup"
> > +#define CAT_OCCUP_NUM_OF_RUNS		5
> > +
> > +static int cat_occup_cpu;
> > +
> > +static int cat_occup_init(const struct resctrl_val_param *param, int domain_id)
> 
> Please take a look at recent resctrl selftest changes that provides more
> data to the init() that will avoid the cat_occup_cpu global.
> Even so, why not just use cmt_init() that reduces interference from rest of
> system to improve chances of workload's cache occupancy to match its
> cache allocation?
> 
> > +{
> > +	char schemata[64];
> > +
> > +	sprintf(llc_occup_path, CON_MON_LCC_OCCUP_PATH, RESCTRL_PATH,
> > +		param->ctrlgrp, domain_id);
> > +
> > +	/*
> > +	 * Confine the benchmark to the allocated portion *before* it starts
> > +	 * filling (resctrl_val() calls init() before forking the benchmark),
> > +	 * so occupancy reflects the restricted CBM from the first sample.
> > +	 */
> > +	snprintf(schemata, sizeof(schemata), "%lx", param->mask);
> > +
> > +	return write_schemata(param->ctrlgrp, schemata, cat_occup_cpu, "L3");
> > +}
> > +
> > +static int cat_occup_setup(const struct resctrl_test *test,
> > +			   const struct user_params *uparams,
> > +			   struct resctrl_val_param *p)
> > +{
> > +	if (p->num_of_runs >= CAT_OCCUP_NUM_OF_RUNS)
> > +		return END_OF_TESTS;
> > +
> > +	p->num_of_runs++;
> > +
> > +	return 0;
> > +}
> 
> cmt_setup()?
> 
> > +
> > +static int cat_occup_measure(const struct user_params *uparams,
> > +			     struct resctrl_val_param *param, pid_t bm_pid)
> > +{
> > +	sleep(1);
> > +	return measure_llc_resctrl(param->filename, bm_pid);
> > +}
> 
> 
> cmt_measure()?
> 
> This test has a lot in common with the existing CMT test since it 
> duplicates cmt_setup(), cmt_measure(), and cmt_feature_check(). From what I 
> can tell it can use cmt_init() also. Could the implementation be simplified
> by instead considering it a CMT test, move code to cmt_test.c, and avoid
> all this duplication?
> 
> > +
> > +static int cat_occup_check_results(struct resctrl_val_param *param,
> > +				   size_t alloc_span, size_t cache_size,
> > +				   int no_of_bits)
> > +{
> > +	char *token_array[8], temp[512];
> > +	unsigned long occu, max_occu = 0, ceiling, floor;
> 
> Please follow kernel coding style (throughout this series) by, in this example,
> using reverse-fir tree ordering.
> 
> > +	int runs = 0;
> > +	int fail = 0;
> > +	FILE *fp;
> > +
> > +	/*
> > +	 * Check every sample, not an average: CAT is a hard limit, so a single
> 
> hmmm ... "is a hard limit" does not match what the code does
> 
> > +	 * sample above the allocation is a real violation that an average
> > +	 * could mask.
> > +	 */
> > +	ceiling = alloc_span + (cache_size - alloc_span) / 2;
> > +	floor = alloc_span / 2;
> > +
> > +	ksft_print_msg("Checking for pass/fail\n");
> > +	fp = fopen(param->filename, "r");
> > +	if (!fp) {
> > +		ksft_perror("Error in opening file");
> > +
> > +		return -1;
> > +	}
> > +
> > +	while (fgets(temp, sizeof(temp), fp)) {
> > +		char *token = strtok(temp, ":\t");
> > +		int fields = 0;
> > +
> > +		while (token) {
> > +			token_array[fields++] = token;
> > +			token = strtok(NULL, ":\t");
> > +		}
> > +
> > +		/* Field 3 is the resctrl-reported llc_occupancy value. */
> > +		occu = strtoul(token_array[3], NULL, 0);
> > +		runs++;
> > +
> > +		if (occu > max_occu)
> > +			max_occu = occu;
> > +
> > +		if (occu > ceiling) {
> > +			ksft_print_msg("Fail: run %d occupancy %lu exceeds ceiling %lu\n",
> > +				       runs, occu, ceiling);
> > +			fail = 1;
> 
> KSFT_FAIL is available to avoid using magic numbers.
> 
> > +		}
> > +	}
> > +	fclose(fp);
> > +
> > +	if (!runs) {
> > +		ksft_print_msg("No occupancy samples collected\n");
> > +		return -1;
> > +	}
> > +
> > +	if (max_occu < floor) {
> > +		ksft_print_msg("Fail: peak occupancy %lu never reached floor %lu\n",
> > +			       max_occu, floor);
> 
> I think this test should be dropped. We cannot control the environments in which
> the tests are run and legitimate interference may cause this test to fail without
> it meaning that there is a bug in resctrl. 
> 
> 
> > +		fail = 1;
> > +	}
> > +
> > +	ksft_print_msg("%s CAT confines occupancy to the allocated %d-bit portion\n",
> > +		       fail ? "Fail:" : "Pass:", no_of_bits);
> > +	ksft_print_msg("occupancy=%lu alloc=%zu full=%zu ceiling=%lu floor=%lu\n",
> > +		       max_occu, alloc_span, cache_size, ceiling, floor);
> > +
> > +	return fail;
> > +}
> > +
> > +static void cat_occup_test_cleanup(void)
> > +{
> > +	remove(CAT_OCCUP_RESULT_FILE);
> > +}
> > +
> > +static int cat_occup_run_test(const struct resctrl_test *test,
> > +			      const struct user_params *uparams)
> > +{
> > +	struct fill_buf_param fill_buf = {};
> > +	unsigned long cache_total_size = 0;
> > +	unsigned long full_mask;
> > +	int count_of_bits;
> > +	size_t alloc_span;
> > +	int n, ret;
> > +
> > +	ret = get_full_cbm(test->resource, &full_mask);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = get_cache_size(uparams->cpu, test->resource, &cache_total_size);
> > +	if (ret)
> > +		return ret;
> > +	ksft_print_msg("Cache size :%lu\n", cache_total_size);
> > +
> > +	count_of_bits = count_bits(full_mask);
> > +
> > +	/*
> > +	 * Allocate a strict subset of the cache so the benchmark buffer
> > +	 * is larger than the allocation and CAT has something to enforce.
> > +	 */
> > +	n = uparams->bits ? : count_of_bits / 2;
> > +	if (n < 1 || n >= count_of_bits) {
> > +		ksft_print_msg("Invalid number of CBM bits %d, expected 1 to %d\n",
> > +			       n, count_of_bits - 1);
> > +		return -1;
> > +	}
> > +
> > +	struct resctrl_val_param param = {
> > +		.ctrlgrp	= "c1",
> > +		.filename	= CAT_OCCUP_RESULT_FILE,
> > +		.mask		= ~(full_mask << n) & full_mask,
> > +		.num_of_runs	= 0,
> > +		.init		= cat_occup_init,
> > +		.setup		= cat_occup_setup,
> > +		.measure	= cat_occup_measure,
> > +	};
> > +
> > +	alloc_span = cache_portion_size(cache_total_size, param.mask, full_mask);
> > +
> > +	/* Benchmark buffer spans the full cache: larger than the allocation. */
> > +	fill_buf.buf_size = cache_total_size;
> > +	fill_buf.memflush = uparams->fill_buf ? uparams->fill_buf->memflush : true;
> > +	param.fill_buf = &fill_buf;
> 
> This prevents usage of user provided benchmark. Please compare with cmt_run_test()
> initialization. You can find more details about how the workload and parameters are
> communicated in e958c21e2ede ("selftests/resctrl: Make benchmark parameter passing robust")
> 
> > +	cat_occup_cpu = uparams->cpu;
> > +
> > +	remove(param.filename);
> > +
> > +	ret = resctrl_val(test, uparams, &param);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return cat_occup_check_results(&param, alloc_span, cache_total_size, n);
> > +}
> 
> Fundamentally this looks like a duplicate of cmt_run_test()? Only differences I see
> is the size of the buffer and how the test results are checked for pass/fail. Looking
> at the pass/fail I do not see a big difference with what the CMT test tests. I do not
> see what value this tests add beyond what the CMT test already provides.
> 
> 
> > +
> > +static bool cat_occup_feature_check(const struct resctrl_test *test)
> > +{
> > +	return test_resource_feature_check(test) &&
> > +	       resctrl_mon_feature_exists("L3_MON", "llc_occupancy");
> > +}
> > +
> > +struct resctrl_test l3_cat_occup_test = {
> > +	.name = "L3_CAT_OCCUP",
> > +	.group = "CAT",
> > +	.resource = "L3",
> > +	.feature_check = cat_occup_feature_check,
> > +	.run_test = cat_occup_run_test,
> > +	.cleanup = cat_occup_test_cleanup,
> > +};
> > diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
> > index d09e693dc739..ef51daa8061a 100644
> > --- a/tools/testing/selftests/resctrl/cmt_test.c
> > +++ b/tools/testing/selftests/resctrl/cmt_test.c
> > @@ -16,9 +16,6 @@
> >  #define MAX_DIFF		2000000
> >  #define MAX_DIFF_PERCENT	15
> >  
> > -#define CON_MON_LCC_OCCUP_PATH		\
> > -	"%s/%s/mon_data/mon_L3_%02d/llc_occupancy"
> > -
> >  static int cmt_init(const struct resctrl_val_param *param, int domain_id)
> >  {
> >  	sprintf(llc_occup_path, CON_MON_LCC_OCCUP_PATH, RESCTRL_PATH,
> > diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> > index afe635b6e48d..ce3abf0bdac2 100644
> > --- a/tools/testing/selftests/resctrl/resctrl.h
> > +++ b/tools/testing/selftests/resctrl/resctrl.h
> > @@ -31,6 +31,9 @@
> >  #define PHYS_ID_PATH		"/sys/devices/system/cpu/cpu"
> >  #define INFO_PATH		"/sys/fs/resctrl/info"
> >  
> > +#define CON_MON_LCC_OCCUP_PATH		\
> > +	"%s/%s/mon_data/mon_L3_%02d/llc_occupancy"
> > +
> >  /*
> >   * CPU vendor IDs
> >   *
> > @@ -244,6 +247,7 @@ extern struct resctrl_test mbm_test;
> >  extern struct resctrl_test mba_test;
> >  extern struct resctrl_test cmt_test;
> >  extern struct resctrl_test l3_cat_test;
> > +extern struct resctrl_test l3_cat_occup_test;
> >  extern struct resctrl_test l3_noncont_cat_test;
> >  extern struct resctrl_test l2_noncont_cat_test;
> >  
> > diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
> > index dbcd5eea9fbc..324a60818aa1 100644
> > --- a/tools/testing/selftests/resctrl/resctrl_tests.c
> > +++ b/tools/testing/selftests/resctrl/resctrl_tests.c
> > @@ -19,6 +19,7 @@ static struct resctrl_test *resctrl_tests[] = {
> >  	&mba_test,
> >  	&cmt_test,
> >  	&l3_cat_test,
> > +	&l3_cat_occup_test,
> >  	&l3_noncont_cat_test,
> >  	&l2_noncont_cat_test,
> >  };
> 
> Reinette

Agreed for the above comments, I'll drop this change for v2.
Thanks for pointing this out.

Best regards,
Richard Cheng.