Re: [LTP] [PATCH v4] hugetlb/hugemmap: add hugemmap33 to test hugetlbfs quota accounting

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> +static void verify_quota_stat(long tot, long free, long avail)
> +{
> +	struct statfs s;
> +
> +	SAFE_STATFS(MNTPOINT, &s);
> +
> +	if ((long)s.f_blocks != tot || (long)s.f_bfree != free || (long)s.f_bavail != avail) {
> +		tst_res(TFAIL, "Bad quota counters: total=%li free=%li avail=%li, expected %li %li %li",
> +			(long)s.f_blocks, (long)s.f_bfree, (long)s.f_bavail,
> +			tot, free, avail);
> +		return;
> +	}
> +	tst_res(TPASS, "Quota counters are correct: total=%li free=%li avail=%li",
> +		tot, free, avail);
> +}
> +
> +static void do_map(unsigned long size, int mmap_flags, int action_flags,
> +		   int expected)
> +{
> +	int fd;
> +	char *a = MAP_FAILED, *b, *c = MAP_FAILED;
> +	char path[PATH_MAX];
> +
> +	snprintf(path, sizeof(path), "%s/test_file_%d", MNTPOINT, getpid());
> +	fd = SAFE_OPEN(path, O_CREAT | O_RDWR, 0600);
> +	SAFE_UNLINK(path);
> +
> +	TESTPTR(mmap(NULL, size, PROT_READ | PROT_WRITE, mmap_flags, fd, 0));
> +	a = TST_RET_PTR;
> +	if (a == MAP_FAILED) {
> +		if (expected == QUOTA_FAIL && TST_ERR == ENOMEM) {
> +			tst_res(TPASS | TERRNO, "mmap failed as expected due to quota");
> +			SAFE_CLOSE(fd);
> +			return;
> +		}
> +		tst_res(TFAIL | TERRNO, "mmap failed unexpectedly");
> +		SAFE_CLOSE(fd);
> +		return;
> +	}
> +
> +	if (expected == QUOTA_FAIL) {
> +		tst_res(TFAIL, "mmap succeeded but quota exhaustion was expected");
> +		goto cleanup_a;
> +	}
> +
> +	if (action_flags & ACTION_TOUCH) {
> +		for (b = a; b < a + size; b += hpage_size)
> +			*b = 1;
> +	}
> +
> +	if (action_flags & ACTION_COW) {
> +		TESTPTR(mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0));
> +		c = TST_RET_PTR;
> +		if (c == MAP_FAILED) {
> +			if (expected == QUOTA_COW_FAIL && TST_ERR == ENOMEM) {
> +				tst_res(TPASS | TERRNO, "COW mmap failed as expected due to quota");
> +				goto cleanup_a;
> +			}
> +			tst_res(TFAIL | TERRNO, "COW mmap failed unexpectedly");
> +			goto cleanup_a;
> +		}
> +
> +		if (expected == QUOTA_COW_FAIL) {
> +			tst_res(TFAIL, "COW mmap succeeded but quota exhaustion was expected");
> +			goto cleanup_c;
> +		}
> +
> +		if (*c != 1) {
> +			tst_res(TFAIL, "Data mismatch when setting up COW");
> +			goto cleanup_c;
> +		}
> +		*c = 0;
> +		SAFE_MUNMAP(c, size);
> +	}
> +
> +	if (expected == QUOTA_OK)
> +		tst_res(TPASS, "Quota test passed as expected");
> +
> +	SAFE_MUNMAP(a, size);
> +	SAFE_CLOSE(fd);
> +	return;
> +
> +cleanup_c:
> +	SAFE_MUNMAP(c, size);
> +cleanup_a:
> +	SAFE_MUNMAP(a, size);
> +	SAFE_CLOSE(fd);
> +}

I think that it would be much more readable if this was split into
several functions. It would make sense to split this into three
different functions one function verify_quota_ok() verify_quota_fail()
and verify_quota_cow_fail().

-- 
Cyril Hrubis
chrubis@suse.cz

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