Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Michael Ellerman <mpe@ellerman.id.au>
To:     Nhat Pham <nphamcs@gmail.com>, akpm@linux-foundation.org
Cc:     hannes@cmpxchg.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, bfoster@redhat.com,
        willy@infradead.org, linux-api@vger.kernel.org,
        kernel-team@meta.com, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v13 3/3] selftests: Add selftests for cachestat
In-Reply-To: <20230503013608.2431726-4-nphamcs@gmail.com>
References: <20230503013608.2431726-1-nphamcs@gmail.com>
 <20230503013608.2431726-4-nphamcs@gmail.com>
Date:   Thu, 11 May 2023 13:21:28 +1000
Message-ID: <877ctfa6yv.fsf@mail.lhotse>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Nhat Pham <nphamcs@gmail.com> writes:
> Test cachestat on a newly created file, /dev/ files, and /proc/ files.
> Also test on a shmem file (which can also be tested with huge pages
> since tmpfs supports huge pages).
>
> Signed-off-by: Nhat Pham <nphamcs@gmail.com>
...
> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
> new file mode 100644
> index 000000000000..c3823b809c25
> --- /dev/null
> +++ b/tools/testing/selftests/cachestat/test_cachestat.c
> @@ -0,0 +1,258 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +
> +#include <stdio.h>
> +#include <stdbool.h>
> +#include <linux/kernel.h>
> +#include <linux/mman.h>
> +#include <sys/mman.h>
> +#include <sys/shm.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +
> +#include "../kselftest.h"
> +
> +static const char * const dev_files[] = {
> +	"/dev/zero", "/dev/null", "/dev/urandom",
> +	"/proc/version", "/proc"
> +};
> +static const int cachestat_nr = 451;
> +
> +void print_cachestat(struct cachestat *cs)
> +{
> +	ksft_print_msg(
> +	"Using cachestat: Cached: %lu, Dirty: %lu, Writeback: %lu, Evicted: %lu, Recently Evicted: %lu\n",
> +	cs->nr_cache, cs->nr_dirty, cs->nr_writeback,
> +	cs->nr_evicted, cs->nr_recently_evicted);
> +}
> +
> +bool write_exactly(int fd, size_t filesize)
> +{
> +	char data[filesize];

On kernels with 64K pages (powerpc at least), this tries to allocate
64MB on the stack which segfaults.

Allocating data with malloc avoids the problem and allows the test to
pass.

Looks like this commit is still in mm-unstable, so maybe Andrew can
squash the incremental diff below in, if it looks OK to you. The diff is
a bit big because I unindented the body of the function.

cheers


diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index 9be2262e5c17..54d09b820ed4 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -31,48 +31,59 @@ void print_cachestat(struct cachestat *cs)
 
 bool write_exactly(int fd, size_t filesize)
 {
-	char data[filesize];
-	bool ret = true;
 	int random_fd = open("/dev/urandom", O_RDONLY);
+	char *cursor, *data;
+	int remained;
+	bool ret;
 
 	if (random_fd < 0) {
 		ksft_print_msg("Unable to access urandom.\n");
 		ret = false;
 		goto out;
-	} else {
-		int remained = filesize;
-		char *cursor = data;
+	}
 
-		while (remained) {
-			ssize_t read_len = read(random_fd, cursor, remained);
+	data = malloc(filesize);
+	if (!data) {
+		ksft_print_msg("Unable to allocate data.\n");
+		ret = false;
+		goto close_random_fd;
+	}
 
-			if (read_len <= 0) {
-				ksft_print_msg("Unable to read from urandom.\n");
-				ret = false;
-				goto close_random_fd;
-			}
+	remained = filesize;
+	cursor = data;
 
-			remained -= read_len;
-			cursor += read_len;
+	while (remained) {
+		ssize_t read_len = read(random_fd, cursor, remained);
+
+		if (read_len <= 0) {
+			ksft_print_msg("Unable to read from urandom.\n");
+			ret = false;
+			goto out_free_data;
 		}
 
-		/* write random data to fd */
-		remained = filesize;
-		cursor = data;
-		while (remained) {
-			ssize_t write_len = write(fd, cursor, remained);
+		remained -= read_len;
+		cursor += read_len;
+	}
 
-			if (write_len <= 0) {
-				ksft_print_msg("Unable write random data to file.\n");
-				ret = false;
-				goto close_random_fd;
-			}
+	/* write random data to fd */
+	remained = filesize;
+	cursor = data;
+	while (remained) {
+		ssize_t write_len = write(fd, cursor, remained);
 
-			remained -= write_len;
-			cursor += write_len;
+		if (write_len <= 0) {
+			ksft_print_msg("Unable write random data to file.\n");
+			ret = false;
+			goto out_free_data;
 		}
+
+		remained -= write_len;
+		cursor += write_len;
 	}
 
+	ret = true;
+out_free_data:
+	free(data);
 close_random_fd:
 	close(random_fd);
 out: