* [PATCH V4 1/2] selftests: vm: bring common functions to a new file
@ 2022-03-15 8:50 Muhammad Usama Anjum
2022-03-15 8:50 ` [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit Muhammad Usama Anjum
2022-03-16 8:55 ` [PATCH V4 1/2] selftests: vm: bring common functions to a new file David Hildenbrand
0 siblings, 2 replies; 6+ messages in thread
From: Muhammad Usama Anjum @ 2022-03-15 8:50 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Muhammad Usama Anjum, kernel, linux-kernel, linux-mm, linux-kselftest
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@collabora.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
+
ifeq ($(MACHINE),x86_64)
BINARIES_32 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_32))
BINARIES_64 := $(patsubst %,$(OUTPUT)/%,$(BINARIES_64))
diff --git a/tools/testing/selftests/vm/madv_populate.c b/tools/testing/selftests/vm/madv_populate.c
index 3ee0e82756002..715a42e8e2cdb 100644
--- a/tools/testing/selftests/vm/madv_populate.c
+++ b/tools/testing/selftests/vm/madv_populate.c
@@ -18,6 +18,7 @@
#include <sys/mman.h>
#include "../kselftest.h"
+#include "vm_util.h"
/*
* For now, we're using 2 MiB of private anonymous memory for all tests.
@@ -26,18 +27,6 @@
static size_t pagesize;
-static uint64_t pagemap_get_entry(int fd, char *start)
-{
- const unsigned long pfn = (unsigned long)start / pagesize;
- 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;
-}
-
static bool pagemap_is_populated(int fd, char *start)
{
uint64_t entry = pagemap_get_entry(fd, start);
@@ -46,13 +35,6 @@ static bool pagemap_is_populated(int fd, char *start)
return entry & 0xc000000000000000ull;
}
-static bool pagemap_is_softdirty(int fd, char *start)
-{
- uint64_t entry = pagemap_get_entry(fd, start);
-
- return entry & 0x0080000000000000ull;
-}
-
static void sense_support(void)
{
char *addr;
@@ -258,20 +240,6 @@ static bool range_is_not_softdirty(char *start, ssize_t size)
return ret;
}
-static void clear_softdirty(void)
-{
- int fd = open("/proc/self/clear_refs", O_WRONLY);
- const char *ctrl = "4";
- int ret;
-
- if (fd < 0)
- ksft_exit_fail_msg("opening clear_refs failed\n");
- ret = write(fd, ctrl, strlen(ctrl));
- if (ret != strlen(ctrl))
- ksft_exit_fail_msg("writing clear_refs failed\n");
- close(fd);
-}
-
static void test_softdirty(void)
{
char *addr;
diff --git a/tools/testing/selftests/vm/split_huge_page_test.c b/tools/testing/selftests/vm/split_huge_page_test.c
index 52497b7b9f1db..b6b381611fb6d 100644
--- a/tools/testing/selftests/vm/split_huge_page_test.c
+++ b/tools/testing/selftests/vm/split_huge_page_test.c
@@ -16,6 +16,7 @@
#include <sys/mount.h>
#include <malloc.h>
#include <stdbool.h>
+#include "vm_util.h"
uint64_t pagesize;
unsigned int pageshift;
@@ -51,30 +52,6 @@ int is_backed_by_thp(char *vaddr, int pagemap_file, int kpageflags_file)
return 0;
}
-
-static uint64_t read_pmd_pagesize(void)
-{
- int fd;
- char buf[20];
- ssize_t num_read;
-
- fd = open(PMD_SIZE_PATH, O_RDONLY);
- if (fd == -1) {
- perror("Open hpage_pmd_size failed");
- exit(EXIT_FAILURE);
- }
- num_read = read(fd, buf, 19);
- if (num_read < 1) {
- close(fd);
- perror("Read hpage_pmd_size failed");
- exit(EXIT_FAILURE);
- }
- buf[num_read] = '\0';
- close(fd);
-
- return strtoul(buf, NULL, 10);
-}
-
static int write_file(const char *path, const char *buf, size_t buflen)
{
int fd;
@@ -113,58 +90,6 @@ static void write_debugfs(const char *fmt, ...)
}
}
-#define MAX_LINE_LENGTH 500
-
-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;
-}
-
-static 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) {
- printf("%s: Pattern is too long\n", __func__);
- exit(EXIT_FAILURE);
- }
-
-
- fp = fopen(SMAP_PATH, "r");
- if (!fp) {
- printf("%s: Failed to open file %s\n", __func__, SMAP_PATH);
- exit(EXIT_FAILURE);
- }
- 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) {
- printf("Reading smap error\n");
- exit(EXIT_FAILURE);
- }
-
-err_out:
- fclose(fp);
- return thp;
-}
-
void split_pmd_thp(void)
{
char *one_page;
diff --git a/tools/testing/selftests/vm/vm_util.c b/tools/testing/selftests/vm/vm_util.c
new file mode 100644
index 0000000000000..c946e04df9236
--- /dev/null
+++ b/tools/testing/selftests/vm/vm_util.c
@@ -0,0 +1,103 @@
+// 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);
+}
+
+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"
+#define SMAP "/proc/self/smaps"
+#define DIRTY_BIT_LOCATION 55
+#define MAX_LINE_LENGTH 512
+
+uint64_t pagemap_get_entry(int fd, char *start);
+bool pagemap_is_softdirty(int fd, char *start);
+void clear_softdirty(void);
+uint64_t read_pmd_pagesize(void);
+uint64_t check_huge(void *addr);
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit
2022-03-15 8:50 [PATCH V4 1/2] selftests: vm: bring common functions to a new file Muhammad Usama Anjum
@ 2022-03-15 8:50 ` Muhammad Usama Anjum
2022-03-15 20:53 ` Gabriel Krisman Bertazi
2022-03-16 8:55 ` [PATCH V4 1/2] selftests: vm: bring common functions to a new file David Hildenbrand
1 sibling, 1 reply; 6+ messages in thread
From: Muhammad Usama Anjum @ 2022-03-15 8:50 UTC (permalink / raw)
To: Andrew Morton, Shuah Khan
Cc: Gabriel Krisman Bertazi, usama.anjum, kernel, Will Deacon,
linux-kernel, linux-mm, linux-kselftest
From: Gabriel Krisman Bertazi <krisman@collabora.com>
This introduces three tests:
1) Sanity check soft dirty basic semantics: allocate area, clean, dirty,
check if the SD bit is flipped.
2) Check VMA reuse: validate the VM_SOFTDIRTY usage
3) Check soft-dirty on huge pages
This was motivated by Will Deacon's fix commit 912efa17e512 ("mm: proc:
Invalidate TLB after clearing soft-dirty page state"). I was tracking the
same issue that he fixed, and this test would have caught it.
CC: Will Deacon <will@kernel.org>
Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
V3 of this patch is in Andrew's tree. Please drop that.
Changes in V4:
Cosmetic changes
Removed global variables
Replaced ksft_print_msg with ksft_exit_fail_msg to exit the program at
once
Some other minor changes
Correct the authorship of the patch
Tests of soft dirty bit in this patch and in madv_populate.c are
non-overlapping. madv_populate.c has only one soft-dirty bit test in the
context of different advise (MADV_POPULATE_READ and
MADV_POPULATE_WRITE). This new test adds more tests.
Tab width of 8 has been used to align the macros. This alignment may look
odd in shell or email. But it looks alright in editors.
Test output:
TAP version 13
1..5
ok 1 Test test_simple
ok 2 Test test_vma_reuse reused memory location
ok 3 Test test_vma_reuse dirty bit of previous page
ok 4 Test test_hugepage huge page allocation
ok 5 Test test_hugepage huge page dirty bit
# Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0
Or
TAP version 13
1..5
ok 1 Test test_simple
ok 2 Test test_vma_reuse reused memory location
ok 3 Test test_vma_reuse dirty bit of previous page
ok 4 # SKIP Test test_hugepage huge page allocation
ok 5 # SKIP Test test_hugepage huge page dirty bit
# Totals: pass:3 fail:0 xfail:0 xpass:0 skip:2 error:0
Changes in V3:
Move test to selftests/vm
Use kselftest macros
Minor updates to make code more maintainable
Add configurations in config file
V2 of this patch:
https://lore.kernel.org/lkml/20210603151518.2437813-1-krisman@collabora.com/
---
tools/testing/selftests/vm/.gitignore | 1 +
tools/testing/selftests/vm/Makefile | 2 +
tools/testing/selftests/vm/config | 2 +
tools/testing/selftests/vm/soft-dirty.c | 146 ++++++++++++++++++++++++
4 files changed, 151 insertions(+)
create mode 100644 tools/testing/selftests/vm/soft-dirty.c
diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore
index d7507f3c7c76a..3cb4fa771ec2a 100644
--- a/tools/testing/selftests/vm/.gitignore
+++ b/tools/testing/selftests/vm/.gitignore
@@ -29,5 +29,6 @@ write_to_hugetlbfs
hmm-tests
memfd_secret
local_config.*
+soft-dirty
split_huge_page_test
ksm_tests
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 4e68edb26d6b6..f25eb30b5f0cb 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -47,6 +47,7 @@ TEST_GEN_FILES += on-fault-limit
TEST_GEN_FILES += thuge-gen
TEST_GEN_FILES += transhuge-stress
TEST_GEN_FILES += userfaultfd
+TEST_GEN_PROGS += soft-dirty
TEST_GEN_PROGS += split_huge_page_test
TEST_GEN_FILES += ksm_tests
@@ -92,6 +93,7 @@ KSFT_KHDR_INSTALL := 1
include ../lib.mk
$(OUTPUT)/madv_populate: vm_util.c
+$(OUTPUT)/soft-dirty: vm_util.c
$(OUTPUT)/split_huge_page_test: vm_util.c
ifeq ($(MACHINE),x86_64)
diff --git a/tools/testing/selftests/vm/config b/tools/testing/selftests/vm/config
index 60e82da0de850..be087c4bc3961 100644
--- a/tools/testing/selftests/vm/config
+++ b/tools/testing/selftests/vm/config
@@ -4,3 +4,5 @@ CONFIG_TEST_VMALLOC=m
CONFIG_DEVICE_PRIVATE=y
CONFIG_TEST_HMM=m
CONFIG_GUP_TEST=y
+CONFIG_TRANSPARENT_HUGEPAGE=y
+CONFIG_MEM_SOFT_DIRTY=y
diff --git a/tools/testing/selftests/vm/soft-dirty.c b/tools/testing/selftests/vm/soft-dirty.c
new file mode 100644
index 0000000000000..2d50ed3472206
--- /dev/null
+++ b/tools/testing/selftests/vm/soft-dirty.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <string.h>
+#include <stdbool.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <malloc.h>
+#include <sys/mman.h>
+#include "../kselftest.h"
+#include "vm_util.h"
+
+#define PAGEMAP "/proc/self/pagemap"
+#define CLEAR_REFS "/proc/self/clear_refs"
+#define MAX_LINE_LENGTH 512
+#define TEST_ITERATIONS 10000
+
+static void test_simple(int pagemap_fd, int pagesize)
+{
+ int i;
+ char *map;
+
+ map = aligned_alloc(pagesize, pagesize);
+ if (!map)
+ ksft_exit_fail_msg("mmap failed\n");
+
+ clear_softdirty();
+
+ for (i = 0 ; i < TEST_ITERATIONS; i++) {
+ if (pagemap_is_softdirty(pagemap_fd, map) == 1) {
+ ksft_print_msg("dirty bit was 1, but should be 0 (i=%d)\n", i);
+ break;
+ }
+
+ clear_softdirty();
+ map[0]++;
+
+ if (pagemap_is_softdirty(pagemap_fd, map) == 0) {
+ ksft_print_msg("dirty bit was 0, but should be 1 (i=%d)\n", i);
+ break;
+ }
+
+ clear_softdirty();
+ }
+ free(map);
+
+ ksft_test_result(i == TEST_ITERATIONS, "Test %s\n", __func__);
+}
+
+static void test_vma_reuse(int pagemap_fd, int pagesize)
+{
+ char *map, *map2;
+
+ map = mmap(NULL, pagesize, (PROT_READ | PROT_WRITE), (MAP_PRIVATE | MAP_ANON), -1, 0);
+ if (map == MAP_FAILED)
+ ksft_exit_fail_msg("mmap failed");
+
+ clear_softdirty();
+
+ /* Write to the page before unmapping and map the same size region again to check
+ * if same memory region is gotten next time and if dirty bit is preserved across
+ * this type of allocations.
+ */
+ map[0]++;
+
+ munmap(map, pagesize);
+
+ map2 = mmap(NULL, pagesize, (PROT_READ | PROT_WRITE), (MAP_PRIVATE | MAP_ANON), -1, 0);
+ if (map2 == MAP_FAILED)
+ ksft_exit_fail_msg("mmap failed");
+
+ ksft_test_result(map == map2, "Test %s reused memory location\n", __func__);
+
+ ksft_test_result(pagemap_is_softdirty(pagemap_fd, map) != 0,
+ "Test %s dirty bit of previous page\n", __func__);
+
+ munmap(map2, pagesize);
+}
+
+static void test_hugepage(int pagemap_fd, int pagesize)
+{
+ char *map;
+ int i, ret;
+ size_t hpage_len = read_pmd_pagesize();
+
+ map = memalign(hpage_len, hpage_len);
+ if (!map)
+ ksft_exit_fail_msg("memalign failed\n");
+
+ ret = madvise(map, hpage_len, MADV_HUGEPAGE);
+ if (ret)
+ ksft_exit_fail_msg("madvise failed %d\n", ret);
+
+ for (i = 0; i < hpage_len; i++)
+ map[i] = (char)i;
+
+ if (check_huge(map)) {
+ ksft_test_result_pass("Test %s huge page allocation\n", __func__);
+
+ clear_softdirty();
+ for (i = 0 ; i < TEST_ITERATIONS ; i++) {
+ if (pagemap_is_softdirty(pagemap_fd, map) == 1) {
+ ksft_print_msg("dirty bit was 1, but should be 0 (i=%d)\n", i);
+ break;
+ }
+
+ clear_softdirty();
+ map[0]++;
+
+ if (pagemap_is_softdirty(pagemap_fd, map) == 0) {
+ ksft_print_msg("dirty bit was 0, but should be 1 (i=%d)\n", i);
+ break;
+ }
+ clear_softdirty();
+ }
+
+ ksft_test_result(i == TEST_ITERATIONS, "Test %s huge page dirty bit\n", __func__);
+ } else {
+ // hugepage allocation failed. skip these tests
+ ksft_test_result_skip("Test %s huge page allocation\n", __func__);
+ ksft_test_result_skip("Test %s huge page dirty bit\n", __func__);
+ }
+ free(map);
+}
+
+int main(int argc, char **argv)
+{
+ int pagemap_fd;
+ int pagesize;
+
+ ksft_print_header();
+ ksft_set_plan(5);
+
+ pagemap_fd = open(PAGEMAP, O_RDONLY);
+ if (pagemap_fd < 0)
+ ksft_exit_fail_msg("Failed to open %s\n", PAGEMAP);
+
+ pagesize = getpagesize();
+
+ test_simple(pagemap_fd, pagesize);
+ test_vma_reuse(pagemap_fd, pagesize);
+ test_hugepage(pagemap_fd, pagesize);
+
+ close(pagemap_fd);
+
+ return ksft_exit_pass();
+}
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit
2022-03-15 8:50 ` [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit Muhammad Usama Anjum
@ 2022-03-15 20:53 ` Gabriel Krisman Bertazi
2022-03-16 17:36 ` Muhammad Usama Anjum
0 siblings, 1 reply; 6+ messages in thread
From: Gabriel Krisman Bertazi @ 2022-03-15 20:53 UTC (permalink / raw)
To: Muhammad Usama Anjum
Cc: Andrew Morton, Shuah Khan, kernel, Will Deacon, linux-kernel,
linux-mm, linux-kselftest
Muhammad Usama Anjum <usama.anjum@collabora.com> writes:
> From: Gabriel Krisman Bertazi <krisman@collabora.com>
Hi Usama,
Please, cc me on the whole thread. I didn't get the patch 1/2 or the
cover letter.
> This introduces three tests:
> 1) Sanity check soft dirty basic semantics: allocate area, clean, dirty,
> check if the SD bit is flipped.
> 2) Check VMA reuse: validate the VM_SOFTDIRTY usage
> 3) Check soft-dirty on huge pages
>
> This was motivated by Will Deacon's fix commit 912efa17e512 ("mm: proc:
> Invalidate TLB after clearing soft-dirty page state"). I was tracking the
> same issue that he fixed, and this test would have caught it.
>
> CC: Will Deacon <will@kernel.org>
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> V3 of this patch is in Andrew's tree. Please drop that.
v3 is still in linux-next and this note is quite hidden in the middle of
the commit message.
>
> Changes in V4:
> Cosmetic changes
> Removed global variables
> Replaced ksft_print_msg with ksft_exit_fail_msg to exit the program at
> once
> Some other minor changes
> Correct the authorship of the patch
>
> Tests of soft dirty bit in this patch and in madv_populate.c are
> non-overlapping. madv_populate.c has only one soft-dirty bit test in the
> context of different advise (MADV_POPULATE_READ and
> MADV_POPULATE_WRITE). This new test adds more tests.
>
> Tab width of 8 has been used to align the macros. This alignment may look
> odd in shell or email. But it looks alright in editors.
I'm curious if you tested reverting 912efa17e512. Did the new versions
of this patch still catch the original issue?
> Test output:
> TAP version 13
> 1..5
> ok 1 Test test_simple
> ok 2 Test test_vma_reuse reused memory location
> ok 3 Test test_vma_reuse dirty bit of previous page
> ok 4 Test test_hugepage huge page allocation
> ok 5 Test test_hugepage huge page dirty bit
> # Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0
>
> Or
>
> TAP version 13
> 1..5
> ok 1 Test test_simple
> ok 2 Test test_vma_reuse reused memory location
> ok 3 Test test_vma_reuse dirty bit of previous page
> ok 4 # SKIP Test test_hugepage huge page allocation
> ok 5 # SKIP Test test_hugepage huge page dirty bit
> # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:2 error:0
>
> Changes in V3:
> Move test to selftests/vm
> Use kselftest macros
> Minor updates to make code more maintainable
> Add configurations in config file
>
> V2 of this patch:
> https://lore.kernel.org/lkml/20210603151518.2437813-1-krisman@collabora.com/
> ---
> tools/testing/selftests/vm/.gitignore | 1 +
> tools/testing/selftests/vm/Makefile | 2 +
> tools/testing/selftests/vm/config | 2 +
> tools/testing/selftests/vm/soft-dirty.c | 146 ++++++++++++++++++++++++
> 4 files changed, 151 insertions(+)
> create mode 100644 tools/testing/selftests/vm/soft-dirty.c
>
> diff --git a/tools/testing/selftests/vm/.gitignore b/tools/testing/selftests/vm/.gitignore
> index d7507f3c7c76a..3cb4fa771ec2a 100644
> --- a/tools/testing/selftests/vm/.gitignore
> +++ b/tools/testing/selftests/vm/.gitignore
> @@ -29,5 +29,6 @@ write_to_hugetlbfs
> hmm-tests
> memfd_secret
> local_config.*
> +soft-dirty
> split_huge_page_test
> ksm_tests
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index 4e68edb26d6b6..f25eb30b5f0cb 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -47,6 +47,7 @@ TEST_GEN_FILES += on-fault-limit
> TEST_GEN_FILES += thuge-gen
> TEST_GEN_FILES += transhuge-stress
> TEST_GEN_FILES += userfaultfd
> +TEST_GEN_PROGS += soft-dirty
> TEST_GEN_PROGS += split_huge_page_test
> TEST_GEN_FILES += ksm_tests
>
> @@ -92,6 +93,7 @@ KSFT_KHDR_INSTALL := 1
> include ../lib.mk
>
> $(OUTPUT)/madv_populate: vm_util.c
> +$(OUTPUT)/soft-dirty: vm_util.c
> $(OUTPUT)/split_huge_page_test: vm_util.c
>
> ifeq ($(MACHINE),x86_64)
> diff --git a/tools/testing/selftests/vm/config b/tools/testing/selftests/vm/config
> index 60e82da0de850..be087c4bc3961 100644
> --- a/tools/testing/selftests/vm/config
> +++ b/tools/testing/selftests/vm/config
> @@ -4,3 +4,5 @@ CONFIG_TEST_VMALLOC=m
> CONFIG_DEVICE_PRIVATE=y
> CONFIG_TEST_HMM=m
> CONFIG_GUP_TEST=y
> +CONFIG_TRANSPARENT_HUGEPAGE=y
> +CONFIG_MEM_SOFT_DIRTY=y
> diff --git a/tools/testing/selftests/vm/soft-dirty.c b/tools/testing/selftests/vm/soft-dirty.c
> new file mode 100644
> index 0000000000000..2d50ed3472206
> --- /dev/null
> +++ b/tools/testing/selftests/vm/soft-dirty.c
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdbool.h>
> +#include <fcntl.h>
> +#include <stdint.h>
> +#include <malloc.h>
> +#include <sys/mman.h>
> +#include "../kselftest.h"
> +#include "vm_util.h"
> +
> +#define PAGEMAP "/proc/self/pagemap"
> +#define CLEAR_REFS "/proc/self/clear_refs"
> +#define MAX_LINE_LENGTH 512
MAX_LINE_LENGTH is no longer used after check_for_pattern was dropped.
Can't the previous defines and file handling functions also go the
vm_util.h?
> +#define TEST_ITERATIONS 10000
> +
> +static void test_simple(int pagemap_fd, int pagesize)
> +{
> + int i;
> + char *map;
> +
> + map = aligned_alloc(pagesize, pagesize);
> + if (!map)
> + ksft_exit_fail_msg("mmap failed\n");
> +
> + clear_softdirty();
> +
> + for (i = 0 ; i < TEST_ITERATIONS; i++) {
> + if (pagemap_is_softdirty(pagemap_fd, map) == 1) {
> + ksft_print_msg("dirty bit was 1, but should be 0 (i=%d)\n", i);
> + break;
> + }
> +
> + clear_softdirty();
> + map[0]++;
This will overflow several times during TEST_ITERATIONS. While it is
not broken, since we care about causing the page fault, it is not
obvious. Can you add a comment or do something like this instead?
map[0] = !map[0];
--
Gabriel Krisman Bertazi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 1/2] selftests: vm: bring common functions to a new file
2022-03-15 8:50 [PATCH V4 1/2] selftests: vm: bring common functions to a new file Muhammad Usama Anjum
2022-03-15 8:50 ` [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit Muhammad Usama Anjum
@ 2022-03-16 8:55 ` David Hildenbrand
2022-03-16 16:47 ` Muhammad Usama Anjum
1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2022-03-16 8:55 UTC (permalink / raw)
To: Muhammad Usama Anjum, Andrew Morton, Shuah Khan
Cc: kernel, linux-kernel, linux-mm, linux-kselftest
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@collabora.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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 1/2] selftests: vm: bring common functions to a new file
2022-03-16 8:55 ` [PATCH V4 1/2] selftests: vm: bring common functions to a new file David Hildenbrand
@ 2022-03-16 16:47 ` Muhammad Usama Anjum
0 siblings, 0 replies; 6+ messages in thread
From: Muhammad Usama Anjum @ 2022-03-16 16:47 UTC (permalink / raw)
To: David Hildenbrand
Cc: usama.anjum, kernel, linux-kernel, linux-mm, linux-kselftest,
Andrew Morton, Gabriel Krisman Bertazi, Shuah Khan
Hi David,
Thank you for the review. I'll correct everything mentioned here in the
next iteration.
+[Gabriel]
On 3/16/22 1:55 PM, David Hildenbrand wrote:
> 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@collabora.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.
>
>
>
--
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit
2022-03-15 20:53 ` Gabriel Krisman Bertazi
@ 2022-03-16 17:36 ` Muhammad Usama Anjum
0 siblings, 0 replies; 6+ messages in thread
From: Muhammad Usama Anjum @ 2022-03-16 17:36 UTC (permalink / raw)
To: Gabriel Krisman Bertazi
Cc: usama.anjum, Andrew Morton, Shuah Khan, kernel, Will Deacon,
linux-kernel, linux-mm, linux-kselftest
On 3/16/22 1:53 AM, Gabriel Krisman Bertazi wrote:
> Muhammad Usama Anjum <usama.anjum@collabora.com> writes:
>
>> From: Gabriel Krisman Bertazi <krisman@collabora.com>
>
> Hi Usama,
>
> Please, cc me on the whole thread. I didn't get the patch 1/2 or the
> cover letter.
>
Sorry, I'll correct it.
>> This introduces three tests:
>> 1) Sanity check soft dirty basic semantics: allocate area, clean, dirty,
>> check if the SD bit is flipped.
>> 2) Check VMA reuse: validate the VM_SOFTDIRTY usage
>> 3) Check soft-dirty on huge pages
>>
>> This was motivated by Will Deacon's fix commit 912efa17e512 ("mm: proc:
>> Invalidate TLB after clearing soft-dirty page state"). I was tracking the
>> same issue that he fixed, and this test would have caught it.
>>
>> CC: Will Deacon <will@kernel.org>
>> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> V3 of this patch is in Andrew's tree. Please drop that.
>
> v3 is still in linux-next and this note is quite hidden in the middle of
> the commit message.
I've tried to put this message at the top of the changelog. I can add
"Note" in the start of it. What can be some other way to highlight this
kind of important message?
>>
>> Changes in V4:
>> Cosmetic changes
>> Removed global variables
>> Replaced ksft_print_msg with ksft_exit_fail_msg to exit the program at
>> once
>> Some other minor changes
>> Correct the authorship of the patch
>>
>> Tests of soft dirty bit in this patch and in madv_populate.c are
>> non-overlapping. madv_populate.c has only one soft-dirty bit test in the
>> context of different advise (MADV_POPULATE_READ and
>> MADV_POPULATE_WRITE). This new test adds more tests.
>>
>> Tab width of 8 has been used to align the macros. This alignment may look
>> odd in shell or email. But it looks alright in editors.
>
> I'm curious if you tested reverting 912efa17e512. Did the new versions
> of this patch still catch the original issue?
Yeah, it did after I reverted the patch and fixed build errors because
of some function's signature change and one test failed and hence issue
is caught:
TAP version 13
1..5
# dirty bit was 0, but should be 1 (i=1)
not ok 1 Test test_simple
ok 2 Test test_vma_reuse reused memory location
ok 3 Test test_vma_reuse dirty bit of previous page
ok 4 # SKIP Test test_hugepage huge page allocation
ok 5 # SKIP Test test_hugepage huge page dirty bit
# Totals: pass:2 fail:1 xfail:0 xpass:0 skip:2 error:0
>> Test output:
>> TAP version 13
>> 1..5
>> ok 1 Test test_simple
>> ok 2 Test test_vma_reuse reused memory location
>> ok 3 Test test_vma_reuse dirty bit of previous page
>> ok 4 Test test_hugepage huge page allocation
>> ok 5 Test test_hugepage huge page dirty bit
>> # Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0
>>
>> Or
>>
>> TAP version 13
>> 1..5
>> ok 1 Test test_simple
>> ok 2 Test test_vma_reuse reused memory location
>> ok 3 Test test_vma_reuse dirty bit of previous page
>> ok 4 # SKIP Test test_hugepage huge page allocation
>> ok 5 # SKIP Test test_hugepage huge page dirty bit
>> # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:2 error:0
[..]
>> +
>> +#define PAGEMAP "/proc/self/pagemap"
>> +#define CLEAR_REFS "/proc/self/clear_refs"
>> +#define MAX_LINE_LENGTH 512
>
> MAX_LINE_LENGTH is no longer used after check_for_pattern was dropped.
>
> Can't the previous defines and file handling functions also go the
> vm_util.h?
>
I don't want to make changes in other two tests. I just want to move
some functions which we need for this test into vm_util.h while keeping
changes less.
>> +#define TEST_ITERATIONS 10000
>> +
>> +static void test_simple(int pagemap_fd, int pagesize)
>> +{
>> + int i;
>> + char *map;
>> +
>> + map = aligned_alloc(pagesize, pagesize);
>> + if (!map)
>> + ksft_exit_fail_msg("mmap failed\n");
>> +
>> + clear_softdirty();
>> +
>> + for (i = 0 ; i < TEST_ITERATIONS; i++) {
>> + if (pagemap_is_softdirty(pagemap_fd, map) == 1) {
>> + ksft_print_msg("dirty bit was 1, but should be 0 (i=%d)\n", i);
>> + break;
>> + }
>> +
>> + clear_softdirty();
>> + map[0]++;
>
>
> This will overflow several times during TEST_ITERATIONS. While it is
> not broken, since we care about causing the page fault, it is not
> obvious. Can you add a comment or do something like this instead?
>
> map[0] = !map[0];
Yeah, it is less obvious. I'll add a comment
--
Muhammad Usama Anjum
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-16 17:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-15 8:50 [PATCH V4 1/2] selftests: vm: bring common functions to a new file Muhammad Usama Anjum
2022-03-15 8:50 ` [PATCH V4 2/2] selftests: vm: Add test for Soft-Dirty PTE bit Muhammad Usama Anjum
2022-03-15 20:53 ` Gabriel Krisman Bertazi
2022-03-16 17:36 ` Muhammad Usama Anjum
2022-03-16 8:55 ` [PATCH V4 1/2] selftests: vm: bring common functions to a new file David Hildenbrand
2022-03-16 16:47 ` Muhammad Usama Anjum
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).