Re: [RFC PATCH v3 1/4] mm/damon/core: add code borrowed from report-based monitoring work

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] Stack bloat / stack overflow risk from `cpumask_t` allocation on the stack.
- [Medium] Data race and potential torn reads from lockless assignment of `cpumask_t`.
--

commit 9c32a9c6100e762865883bc9bf4b2272ccb6dedb
Author: Akinobu Mita <akinobu.mita@gmail.com>

mm/damon/core: add code borrowed from report-based monitoring work
    
This commit borrows code from the report-based monitoring infrastructure
to support perf event-based monitoring. It brings in struct
damon_access_report, an access_reported field in damon_region, and the
sample/primitives sysfs directory structure.
    
Link: https://lore.kernel.org/linux-mm/20251208062943.68824-6-sj@kernel.org/T/

> diff --git a/mm/damon/sysfs.c b/mm/damon/sysfs.c
> index eefa959aa30ae..c19556f2af3bb 100644
> --- a/mm/damon/sysfs.c
> +++ b/mm/damon/sysfs.c
[ ... ]
> +static ssize_t cpumask_store(struct kobject *kobj, struct kobj_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct damon_sysfs_sample_filter *filter = container_of(kobj,
> +			struct damon_sysfs_sample_filter, kobj);
> +	cpumask_t cpumask;

Is it safe to allocate cpumask_t on the stack here?

The size of cpumask_t scales with CONFIG_NR_CPUS. On systems with a large
number of CPUs, this could consume a significant amount of the kernel stack
and potentially risk a stack overflow. Should this use alloc_cpumask_var()
instead to dynamically allocate the temporary mask?

> +	int err = cpulist_parse(buf, &cpumask);
> +
> +	if (err)
> +		return err;
> +	filter->cpumask = cpumask;

Can this lockless assignment cause a data race or torn reads?

Because cpumask_t can be larger than a single word, this assignment may
compile into a non-atomic multi-word copy like memcpy(). If a concurrent
sysfs read in cpumask_show() or a DAMON commit operation accesses
filter->cpumask while this copy is ongoing, could it read a partially
updated, torn mask?

Should this be protected by explicit synchronization, such as
damon_sysfs_lock?

> +	return count;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423004211.7037-1-akinobu.mita@gmail.com?part=1