Re: [LTP] [PATCH v2 3/3] cgroups: Add first IO controller test

[Petr Vorel <pvorel@suse.cz>]

Hi Richie,

> +++ b/testcases/kernel/controllers/io/io_control01.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
nit: not sure if it was deliberate not adding it, but you may want to add your/SUSE copyright.

> +/*\
> + *
> + * [Description]
> + *
> + * Perform some I/O on a file and check if at least some of it is
> + * recorded by the I/O controller.
> + *
> + * The exact amount of I/O performed is dependent on the file system,
> + * page cache, scheduler and block driver. We call sync and drop the
> + * file's page cache to force reading and writing. We also write
> + * random data to try to prevent compression.
> + *
> + * The pagecache is a particular issue for reading. If the call to
> + * fadvise is ignored then the data may only be read from the
> + * cache. So that no I/O requests are made.
> + */
> +
...
> +static int read_io_stats(const char *const line, struct io_stats *const stat)
> +{
> +	return sscanf(line,
> +		      "%u:%u rbytes=%lu wbytes=%lu rios=%lu wios=%lu dbytes=%lu dios=%lu",
> +		      &stat->mjr, &stat->mnr,
> +		      &stat->rbytes, &stat->wbytes, &stat->rios, &stat->wios,
> +		      &stat->dbytes, &stat->dios);
> +}
checkpatch.pl false positive:
io_control01.c:40: WARNING: unchecked sscanf return value
Obviously perl parsing has some limitations as we check read_io_stats() return
value.

...
> +static void setup(void)
> +{
> +	char buf[PATH_MAX] = { 0 };
> +	char *path = SAFE_GETCWD(buf, PATH_MAX - sizeof("mnt") - 1);
> +	struct stat st;
> +
> +	strcpy(path + strlen(path), "/mnt");
> +
> +	tst_stat_mount_dev(path, &st);
> +	dev_major = major(st.st_rdev);
> +	dev_minor = minor(st.st_rdev);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.needs_device = 1,
nit: testcases/kernel/controllers/io/io_control01.c: useless tag: needs_device

Kind regards,
Petr

> +	.mntpoint = "mnt",
> +	.mount_device = 1,
> +	.all_filesystems = 1,
> +	.skip_filesystems = (const char *const[]){ "ntfs", "tmpfs", NULL },
> +	.needs_cgroup_ver = TST_CG_V2,
> +	.needs_cgroup_ctrls = (const char *const[]){ "io", NULL },
> +};

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp