* Re: [PATCH v13 3/3] selftests: Add selftests for cachestat
[not found] ` <20230503013608.2431726-4-nphamcs@gmail.com>
@ 2023-05-11 3:21 ` Michael Ellerman
2023-05-11 19:33 ` Nhat Pham
0 siblings, 1 reply; 2+ messages in thread
From: Michael Ellerman @ 2023-05-11 3:21 UTC (permalink / raw)
To: Nhat Pham, akpm
Cc: linux-api, bfoster, kernel-team, linux-kernel, willy, linux-mm,
hannes, linuxppc-dev
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:
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v13 3/3] selftests: Add selftests for cachestat
2023-05-11 3:21 ` [PATCH v13 3/3] selftests: Add selftests for cachestat Michael Ellerman
@ 2023-05-11 19:33 ` Nhat Pham
0 siblings, 0 replies; 2+ messages in thread
From: Nhat Pham @ 2023-05-11 19:33 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-api, bfoster, kernel-team, linux-kernel, willy, linux-mm,
hannes, akpm, linuxppc-dev
On Wed, May 10, 2023 at 8:21 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> 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:
>
Oh this is nice! I had to make a similar fix in another test
of mine, but forgot about it in this context.
LGTM. For verification, I have applied the diff and test on
my own local setup. Things still pass.
Acked-by: Nhat Pham <nphamcs@gmail.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-05-11 19:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20230503013608.2431726-1-nphamcs@gmail.com>
[not found] ` <20230503013608.2431726-4-nphamcs@gmail.com>
2023-05-11 3:21 ` [PATCH v13 3/3] selftests: Add selftests for cachestat Michael Ellerman
2023-05-11 19:33 ` Nhat Pham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).