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

[Roman Gushchin]

On Wed, Jun 01, 2022 at 02:23:51PM -0700, Andrew Morton wrote:
> 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.

I agree.

> 
> > +	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?

Good catch! No, of course it wasn't intentional.

Below is an updated version of this patch.

Thank you for taking a look!

--