Change read_file(), write_file(), read_num() and write_num() in vm_util.c to report failures to callers instead of exiting from the helper.
Make read_file() return a negative errno on failure instead of 0, so callers can distinguish a successful read from an I/O error. Also make read_num() reject negative and malformed values. Update callers to print diagnostics and fail wherever required. This patch prepares the helpers to be moved to tools/lib/mm without kselftest dependency. Signed-off-by: Sarthak Sharma <[email protected]> --- .../testing/selftests/mm/hugepage_settings.c | 97 ++++++++++++++--- tools/testing/selftests/mm/khugepaged.c | 14 ++- .../selftests/mm/split_huge_page_test.c | 5 +- tools/testing/selftests/mm/vm_util.c | 102 +++++++++++++----- tools/testing/selftests/mm/vm_util.h | 6 +- 5 files changed, 175 insertions(+), 49 deletions(-) diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c index 2eab2110ac6a..db0db8a3df7c 100644 --- a/tools/testing/selftests/mm/hugepage_settings.c +++ b/tools/testing/selftests/mm/hugepage_settings.c @@ -8,6 +8,7 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <errno.h> #include "vm_util.h" #include "hugepage_settings.h" @@ -61,8 +62,10 @@ int thp_read_string(const char *name, const char * const strings[]) exit(EXIT_FAILURE); } - if (!read_file(path, buf, sizeof(buf))) { - perror(path); + ret = read_file(path, buf, sizeof(buf)); + if (ret < 0) { + errno = -ret; + ksft_perror(path); exit(EXIT_FAILURE); } @@ -103,12 +106,18 @@ void thp_write_string(const char *name, const char *val) printf("%s: Pathname is too long\n", __func__); exit(EXIT_FAILURE); } - write_file(path, val, strlen(val) + 1); + ret = write_file(path, val, strlen(val) + 1); + if (ret < 0) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } } unsigned long thp_read_num(const char *name) { char path[PATH_MAX]; + unsigned long num; int ret; ret = snprintf(path, PATH_MAX, THP_SYSFS "%s", name); @@ -116,7 +125,14 @@ unsigned long thp_read_num(const char *name) printf("%s: Pathname is too long\n", __func__); exit(EXIT_FAILURE); } - return read_num(path); + ret = read_num(path, &num); + if (ret < 0) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } + + return num; } void thp_write_num(const char *name, unsigned long num) @@ -129,7 +145,12 @@ void thp_write_num(const char *name, unsigned long num) printf("%s: Pathname is too long\n", __func__); exit(EXIT_FAILURE); } - write_num(path, num); + ret = write_num(path, num); + if (ret < 0) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } } void thp_read_settings(struct thp_settings *settings) @@ -157,8 +178,16 @@ void thp_read_settings(struct thp_settings *settings) .max_ptes_shared = thp_read_num("khugepaged/max_ptes_shared"), .pages_to_scan = thp_read_num("khugepaged/pages_to_scan"), }; - if (dev_queue_read_ahead_path[0]) - settings->read_ahead_kb = read_num(dev_queue_read_ahead_path); + if (dev_queue_read_ahead_path[0]) { + int ret = read_num(dev_queue_read_ahead_path, + &settings->read_ahead_kb); + + if (ret < 0) { + errno = -ret; + ksft_perror(dev_queue_read_ahead_path); + exit(EXIT_FAILURE); + } + } for (i = 0; i < NR_ORDERS; i++) { if (!((1 << i) & orders)) { @@ -208,8 +237,16 @@ void thp_write_settings(struct thp_settings *settings) thp_write_num("khugepaged/max_ptes_shared", khugepaged->max_ptes_shared); thp_write_num("khugepaged/pages_to_scan", khugepaged->pages_to_scan); - if (dev_queue_read_ahead_path[0]) - write_num(dev_queue_read_ahead_path, settings->read_ahead_kb); + if (dev_queue_read_ahead_path[0]) { + int ret = write_num(dev_queue_read_ahead_path, + settings->read_ahead_kb); + + if (ret < 0) { + errno = -ret; + ksft_perror(dev_queue_read_ahead_path); + exit(EXIT_FAILURE); + } + } for (i = 0; i < NR_ORDERS; i++) { if (!((1 << i) & orders)) @@ -307,8 +344,16 @@ static unsigned long __thp_supported_orders(bool is_shmem) } ret = read_file(path, buf, sizeof(buf)); - if (ret) - orders |= 1UL << i; + if (ret < 0) { + if (ret != -ENOENT) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } + continue; + } + + orders |= 1UL << i; } return orders; @@ -425,28 +470,52 @@ static void hugetlb_sysfs_path(char *buf, size_t buflen, unsigned long hugetlb_nr_pages(unsigned long size) { char path[PATH_MAX]; + unsigned long nr; + int ret; hugetlb_sysfs_path(path, sizeof(path), size, "nr_hugepages"); - return read_num(path); + ret = read_num(path, &nr); + if (ret < 0) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } + + return nr; } void hugetlb_set_nr_pages(unsigned long size, unsigned long nr) { char path[PATH_MAX]; + int ret; hugetlb_sysfs_path(path, sizeof(path), size, "nr_hugepages"); - write_num(path, nr); + ret = write_num(path, nr); + if (ret < 0) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } } unsigned long hugetlb_free_pages(unsigned long size) { char path[PATH_MAX]; + unsigned long nr; + int ret; hugetlb_sysfs_path(path, sizeof(path), size, "free_hugepages"); - return read_num(path); + ret = read_num(path, &nr); + if (ret < 0) { + errno = -ret; + ksft_perror(path); + exit(EXIT_FAILURE); + } + + return nr; } static bool __hugetlb_setup(unsigned long size, unsigned long nr) diff --git a/tools/testing/selftests/mm/khugepaged.c b/tools/testing/selftests/mm/khugepaged.c index 10e8dedcb087..6a656bc6b266 100644 --- a/tools/testing/selftests/mm/khugepaged.c +++ b/tools/testing/selftests/mm/khugepaged.c @@ -119,6 +119,7 @@ static void get_finfo(const char *dir) char buf[1 << 10]; char path[PATH_MAX]; char *str, *end; + int ret; finfo.dir = dir; stat(finfo.dir, &path_stat); @@ -138,8 +139,9 @@ static void get_finfo(const char *dir) major(path_stat.st_dev), minor(path_stat.st_dev)) >= sizeof(path)) ksft_exit_fail_msg("%s: Pathname is too long\n", __func__); - if (read_file(path, buf, sizeof(buf)) < 0) - ksft_exit_fail_perror("read_file(read_num)"); + ret = read_file(path, buf, sizeof(buf)); + if (ret < 0) + ksft_exit_fail_msg("read_file(%s): %s\n", path, strerror(-ret)); if (strstr(buf, "DEVTYPE=disk")) { /* Found it */ if (snprintf(finfo.dev_queue_read_ahead_path, @@ -319,7 +321,7 @@ static void *file_setup_area_common(int nr_hpages, enum file_setup_ops setup) { const int open_opt = setup == FILE_SETUP_READ_ONLY_FS ? O_RDONLY : O_RDWR; const int mmap_prot = setup == FILE_SETUP_READ_ONLY_FS ? PROT_READ : (PROT_READ | PROT_WRITE); - int fd; + int fd, ret; void *p; unsigned long size; @@ -363,7 +365,11 @@ static void *file_setup_area_common(int nr_hpages, enum file_setup_ops setup) ksft_exit_fail_perror("mmap()"); /* Drop page cache */ - write_file("/proc/sys/vm/drop_caches", "3", 2); + ret = write_file("/proc/sys/vm/drop_caches", "3", 2); + if (ret < 0) + ksft_exit_fail_msg("write_file(drop_caches): %s\n", + strerror(-ret)); + success("OK"); return p; } diff --git a/tools/testing/selftests/mm/split_huge_page_test.c b/tools/testing/selftests/mm/split_huge_page_test.c index 32b991472f74..ef5ab96adc7e 100644 --- a/tools/testing/selftests/mm/split_huge_page_test.c +++ b/tools/testing/selftests/mm/split_huge_page_test.c @@ -269,7 +269,10 @@ static void write_debugfs(const char *fmt, ...) if (ret >= INPUT_MAX) ksft_exit_fail_msg("%s: Debugfs input is too long\n", __func__); - write_file(SPLIT_DEBUGFS, input, ret + 1); + ret = write_file(SPLIT_DEBUGFS, input, ret + 1); + if (ret < 0) + ksft_exit_fail_msg("write_file(%s): %s\n", SPLIT_DEBUGFS, + strerror(-ret)); } static char *allocate_zero_filled_hugepage(size_t len) diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c index 311fc5b4513e..e3ec077cda67 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -700,91 +700,139 @@ int unpoison_memory(unsigned long pfn) int read_file(const char *path, char *buf, size_t buflen) { - int fd; + int fd, err; ssize_t numread; fd = open(path, O_RDONLY); if (fd == -1) - return 0; + return -errno; numread = read(fd, buf, buflen - 1); if (numread < 1) { + err = numread ? errno : ENODATA; close(fd); - return 0; + return -err; } buf[numread] = '\0'; close(fd); - return (unsigned int) numread; + return (int)numread; } -void write_file(const char *path, const char *buf, size_t buflen) +int write_file(const char *path, const char *buf, size_t buflen) { int fd, saved_errno; ssize_t numwritten; if (buflen < 2) - ksft_exit_fail_msg("Incorrect buffer len: %zu\n", buflen); + return -EINVAL; fd = open(path, O_WRONLY); if (fd == -1) - ksft_exit_fail_msg("%s open failed: %s\n", path, strerror(errno)); + return -errno; numwritten = write(fd, buf, buflen - 1); saved_errno = errno; close(fd); - errno = saved_errno; + if (numwritten < 0) - ksft_exit_fail_msg("%s write(%.*s) failed: %s\n", path, (int)(buflen - 1), - buf, strerror(errno)); - if (numwritten != buflen - 1) - ksft_exit_fail_msg("%s write(%.*s) is truncated, expected %zu bytes, got %zd bytes\n", - path, (int)(buflen - 1), buf, buflen - 1, numwritten); + return -saved_errno; + + if (numwritten != (ssize_t)(buflen - 1)) + return -EIO; + + return 0; } -unsigned long read_num(const char *path) +int read_num(const char *path, unsigned long *num) { + unsigned long val; + int ret; char buf[21]; + char *end; - if (read_file(path, buf, sizeof(buf)) < 0) - ksft_exit_fail_perror("read_file()"); + if (!num) + return -EINVAL; - return strtoul(buf, NULL, 10); + ret = read_file(path, buf, sizeof(buf)); + if (ret < 0) + return ret; + + errno = 0; + val = strtoul(buf, &end, 10); + if (errno) + return -errno; + + if (end == buf || buf[0] == '-') + return -EINVAL; + + if (*end == '\n') + end++; + + if (*end != '\0') + return -EINVAL; + + *num = val; + return 0; } -void write_num(const char *path, unsigned long num) +int write_num(const char *path, unsigned long num) { char buf[21]; sprintf(buf, "%lu", num); - write_file(path, buf, strlen(buf) + 1); + return write_file(path, buf, strlen(buf) + 1); } static unsigned long shmall, shmmax; void __shm_limits_restore(void) { - if (shmmax) - write_num("/proc/sys/kernel/shmmax", shmmax); - if (shmall) - write_num("/proc/sys/kernel/shmall", shmall); + int ret; + + if (shmmax) { + ret = write_num("/proc/sys/kernel/shmmax", shmmax); + if (ret < 0) + ksft_exit_fail_msg("Failed to restore shmmax: %s\n", + strerror(-ret)); + } + if (shmall) { + ret = write_num("/proc/sys/kernel/shmall", shmall); + if (ret < 0) + ksft_exit_fail_msg("Failed to restore shmall: %s\n", + strerror(-ret)); + } } void shm_limits_prepare(unsigned long length) { unsigned long nr = length / psize(); unsigned long val; + int ret; + + ret = read_num("/proc/sys/kernel/shmmax", &val); + if (ret < 0) + ksft_exit_fail_msg("Failed to read /proc/sys/kernel/shmmax: %s\n", + strerror(-ret)); - val = read_num("/proc/sys/kernel/shmmax"); if (val < length) { - write_num("/proc/sys/kernel/shmmax", length); + ret = write_num("/proc/sys/kernel/shmmax", length); + if (ret < 0) + ksft_exit_fail_msg("Failed to write %lu to /proc/sys/kernel/shmmax: %s\n", + length, strerror(-ret)); shmmax = val; } - val = read_num("/proc/sys/kernel/shmall"); + ret = read_num("/proc/sys/kernel/shmall", &val); + if (ret < 0) + ksft_exit_fail_msg("Failed to read /proc/sys/kernel/shmall: %s\n", + strerror(-ret)); if (val < nr) { - write_num("/proc/sys/kernel/shmall", nr); + ret = write_num("/proc/sys/kernel/shmall", nr); + if (ret < 0) + ksft_exit_fail_msg("Failed to write %lu to /proc/sys/kernel/shmall: %s\n", + nr, strerror(-ret)); shmall = val; } } diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h index ea8fc8fdf0eb..28c3d7c1faed 100644 --- a/tools/testing/selftests/mm/vm_util.h +++ b/tools/testing/selftests/mm/vm_util.h @@ -164,10 +164,10 @@ int unpoison_memory(unsigned long pfn); #define PAGEMAP_PRESENT(ent) (((ent) & (1ull << 63)) != 0) #define PAGEMAP_PFN(ent) ((ent) & ((1ull << 55) - 1)) -void write_file(const char *path, const char *buf, size_t buflen); int read_file(const char *path, char *buf, size_t buflen); -unsigned long read_num(const char *path); -void write_num(const char *path, unsigned long num); +int write_file(const char *path, const char *buf, size_t buflen); +int read_num(const char *path, unsigned long *num); +int write_num(const char *path, unsigned long num); void shm_limits_prepare(unsigned long length); void __shm_limits_restore(void); -- 2.39.5
