Re: [PATCH v5 6/6] mm: shrinkers: add scan interface for shrinker debugfs

[Andrew Morton]

On Tue, 31 May 2022 20:22:27 -0700 Roman Gushchin <roman.gushchin@xxxxxxxxx> wrote:

> Add a scan interface which allows to trigger scanning of a particular
> shrinker and specify memcg and numa node. It's useful for testing,
> debugging and profiling of a specific scan_objects() callback.
> Unlike alternatives (creating a real memory pressure and dropping
> caches via /proc/sys/vm/drop_caches) this interface allows to interact
> with only one shrinker at once. Also, if a shrinker is misreporting
> the number of objects (as some do), it doesn't affect scanning.
> 
> ..
>
> --- a/mm/shrinker_debug.c
> +++ b/mm/shrinker_debug.c
> @@ -99,6 +99,78 @@ static int shrinker_debugfs_count_show(struct seq_file *m, void *v)
>  }
>  DEFINE_SHOW_ATTRIBUTE(shrinker_debugfs_count);
>  
> +static int shrinker_debugfs_scan_open(struct inode *inode, struct file *file)
> +{
> +	file->private_data = inode->i_private;
> +	return nonseekable_open(inode, file);
> +}
> +
> +static ssize_t shrinker_debugfs_scan_write(struct file *file,
> +					   const char __user *buf,
> +					   size_t size, loff_t *pos)
> +{
> +	struct shrinker *shrinker = file->private_data;
> +	unsigned long nr_to_scan = 0, ino;
> +	struct shrink_control sc = {
> +		.gfp_mask = GFP_KERNEL,
> +	};
> +	struct mem_cgroup *memcg = NULL;
> +	int nid;
> +	char kbuf[72];
> +	int read_len = size < (sizeof(kbuf) - 1) ? size : (sizeof(kbuf) - 1);

size_t or ulong would be more appropriate.

> +	ssize_t ret;
> +
> +	if (copy_from_user(kbuf, buf, read_len))
> +		return -EFAULT;
> +	kbuf[read_len] = '\0';
> +
> +	if (sscanf(kbuf, "%lu %d %lu", &ino, &nid, &nr_to_scan) < 2)

Was it intentional to permit more than three args?

> +		return -EINVAL;
> +
> +	if (nid < 0 || nid >= nr_node_ids)
> +		return -EINVAL;
> +
> +	if (nr_to_scan == 0)
> +		return size;
> +
> +	if (shrinker->flags & SHRINKER_MEMCG_AWARE) {
> +		memcg = mem_cgroup_get_from_ino(ino);
> +		if (!memcg || IS_ERR(memcg))
> +			return -ENOENT;
> +
> +		if (!mem_cgroup_online(memcg)) {
> +			mem_cgroup_put(memcg);
> +			return -ENOENT;
> +		}
> +	} else if (ino != 0) {
> +		return -EINVAL;
> +	}
> +
> +	ret = down_read_killable(&shrinker_rwsem);
> +	if (ret) {
> +		mem_cgroup_put(memcg);
> +		return ret;
> +	}
> +
> +	sc.nid = nid;
> +	sc.memcg = memcg;
> +	sc.nr_to_scan = nr_to_scan;
> +	sc.nr_scanned = nr_to_scan;
> +
> +	shrinker->scan_objects(shrinker, &sc);
> +
> +	up_read(&shrinker_rwsem);
> +	mem_cgroup_put(memcg);
> +
> +	return size;
> +}