| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b47a93fe-da50-d0d4-be8f-87071bf181f9@redhat.com>
Date: Wed, 16 Mar 2022 09:55:54 +0100
From: David Hildenbrand <david@...hat.com>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Shuah Khan <shuah@...nel.org>
Cc: kernel@...labora.com, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH V4 1/2] selftests: vm: bring common functions to a new
file
On 15.03.22 09:50, Muhammad Usama Anjum wrote:
> Bring common functions to a new file. These functions can be used in the
> new tests. This helps in code duplication.
>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> ---
> tools/testing/selftests/vm/Makefile | 7 +-
> tools/testing/selftests/vm/madv_populate.c | 34 +-----
> .../selftests/vm/split_huge_page_test.c | 77 +------------
> tools/testing/selftests/vm/vm_util.c | 103 ++++++++++++++++++
> tools/testing/selftests/vm/vm_util.h | 15 +++
> 5 files changed, 125 insertions(+), 111 deletions(-)
> create mode 100644 tools/testing/selftests/vm/vm_util.c
> create mode 100644 tools/testing/selftests/vm/vm_util.h
>
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index 5e43f072f5b76..4e68edb26d6b6 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -34,7 +34,7 @@ TEST_GEN_FILES += hugepage-mremap
> TEST_GEN_FILES += hugepage-shm
> TEST_GEN_FILES += hugepage-vmemmap
> TEST_GEN_FILES += khugepaged
> -TEST_GEN_FILES += madv_populate
> +TEST_GEN_PROGS = madv_populate
> TEST_GEN_FILES += map_fixed_noreplace
> TEST_GEN_FILES += map_hugetlb
> TEST_GEN_FILES += map_populate
> @@ -47,7 +47,7 @@ TEST_GEN_FILES += on-fault-limit
> TEST_GEN_FILES += thuge-gen
> TEST_GEN_FILES += transhuge-stress
> TEST_GEN_FILES += userfaultfd
> -TEST_GEN_FILES += split_huge_page_test
> +TEST_GEN_PROGS += split_huge_page_test
> TEST_GEN_FILES += ksm_tests
>
> ifeq ($(MACHINE),x86_64)
> @@ -91,6 +91,9 @@ TEST_FILES := test_vmalloc.sh
> KSFT_KHDR_INSTALL := 1
> include ../lib.mk
>
> +$(OUTPUT)/madv_populate: vm_util.c
> +$(OUTPUT)/split_huge_page_test: vm_util.c
> +
[...]
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdbool.h>
> +#include <string.h>
> +#include "vm_util.h"
> +
> +uint64_t pagemap_get_entry(int fd, char *start)
> +{
> + const unsigned long pfn = (unsigned long)start / getpagesize();
> + uint64_t entry;
> + int ret;
> +
> + ret = pread(fd, &entry, sizeof(entry), pfn * sizeof(entry));
> + if (ret != sizeof(entry))
> + ksft_exit_fail_msg("reading pagemap failed\n");
> + return entry;
> +}
> +
> +bool pagemap_is_softdirty(int fd, char *start)
> +{
> + uint64_t entry = pagemap_get_entry(fd, start);
> +
> + return ((entry >> DIRTY_BIT_LOCATION) & 1);
> +}
Please leave code you're moving around as untouched as possible to avoid
unrelated bugs that happen by mistake and are hard to review.
> +
> +void clear_softdirty(void)
> +{
> + int ret;
> + const char *ctrl = "4";
> + int fd = open("/proc/self/clear_refs", O_WRONLY);
> +
> + if (fd < 0)
> + ksft_exit_fail_msg("opening clear_refs failed\n");
> + ret = write(fd, ctrl, strlen(ctrl));
> + close(fd);
> + if (ret != strlen(ctrl))
> + ksft_exit_fail_msg("writing clear_refs failed\n");
> +}
> +
> +
> +static bool check_for_pattern(FILE *fp, const char *pattern, char *buf)
> +{
> + while (fgets(buf, MAX_LINE_LENGTH, fp) != NULL) {
> + if (!strncmp(buf, pattern, strlen(pattern)))
> + return true;
> + }
> + return false;
> +}
> +
> +uint64_t read_pmd_pagesize(void)
> +{
> + int fd;
> + char buf[20];
> + ssize_t num_read;
> +
> + fd = open(PMD_SIZE, O_RDONLY);
> + if (fd == -1)
> + ksft_exit_fail_msg("Open hpage_pmd_size failed\n");
> +
> + num_read = read(fd, buf, 19);
> + if (num_read < 1) {
> + close(fd);
> + ksft_exit_fail_msg("Read hpage_pmd_size failed\n");
> + }
> + buf[num_read] = '\0';
> + close(fd);
> +
> + return strtoul(buf, NULL, 10);
> +}
> +
> +uint64_t check_huge(void *addr)
> +{
> + uint64_t thp = 0;
> + int ret;
> + FILE *fp;
> + char buffer[MAX_LINE_LENGTH];
> + char addr_pattern[MAX_LINE_LENGTH];
> +
> + ret = snprintf(addr_pattern, MAX_LINE_LENGTH, "%08lx-",
> + (unsigned long) addr);
> + if (ret >= MAX_LINE_LENGTH)
> + ksft_exit_fail_msg("%s: Pattern is too long\n", __func__);
> +
> + fp = fopen(SMAP, "r");
> + if (!fp)
> + ksft_exit_fail_msg("%s: Failed to open file %s\n", __func__, SMAP);
> +
> + if (!check_for_pattern(fp, addr_pattern, buffer))
> + goto err_out;
> +
> + /*
> + * Fetch the AnonHugePages: in the same block and check the number of
> + * hugepages.
> + */
> + if (!check_for_pattern(fp, "AnonHugePages:", buffer))
> + goto err_out;
> +
> + if (sscanf(buffer, "AnonHugePages:%10ld kB", &thp) != 1)
> + ksft_exit_fail_msg("Reading smap error\n");
> +
> +err_out:
> + fclose(fp);
> + return thp;
> +}
> diff --git a/tools/testing/selftests/vm/vm_util.h b/tools/testing/selftests/vm/vm_util.h
> new file mode 100644
> index 0000000000000..7522dbb859f0f
> --- /dev/null
> +++ b/tools/testing/selftests/vm/vm_util.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <stdint.h>
> +#include <fcntl.h>
> +#include "../kselftest.h"
> +
> +#define PMD_SIZE "/sys/kernel/mm/transparent_hugepage/hpage_pmd_size"
Ehm no. PMD_SIZE_PATH at best -- just as it used to.
> +#define SMAP "/proc/self/smaps"
SMAPS_PATH
> +#define DIRTY_BIT_LOCATION 55
Please inline that just as it used to. There is no value in a magic
define without any proper namespace.
> +#define MAX_LINE_LENGTH 512
This used to be 500. Why the change?
Also: weird indentation and these all look like the should go into
vm_util.c. They are not used outside that file.
--
Thanks,
David / dhildenb
Powered by blists - more mailing lists