* [patch 1/2] userfaultfd/selftests: fix hugetlb area allocations
2021-12-31 4:12 incoming Andrew Morton
@ 2021-12-31 4:12 ` Andrew Morton
2021-12-31 4:12 ` [patch 2/2] mm/damon/dbgfs: fix 'struct pid' leaks in 'dbgfs_target_ids_write()' Andrew Morton
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Morton @ 2021-12-31 4:12 UTC (permalink / raw)
To: aarcange, akpm, almasrymina, axelrasmussen, linux-mm,
mike.kravetz, mm-commits, peterx, shuah, torvalds
From: Mike Kravetz <mike.kravetz@oracle.com>
Subject: userfaultfd/selftests: fix hugetlb area allocations
Currently, userfaultfd selftest for hugetlb as run from run_vmtests.sh or
any environment where there are 'just enough' hugetlb pages will always
fail with:
testing events (fork, remap, remove):
ERROR: UFFDIO_COPY error: -12 (errno=12, line=616)
The ENOMEM error code implies there are not enough hugetlb pages.
However, there are free hugetlb pages but they are all reserved. There is
a basic problem with the way the test allocates hugetlb pages which has
existed since the test was originally written. Due to the way 'cleanup'
was done between different phases of the test, this issue was masked until
recently. The issue was uncovered by commit 8ba6e8640844
("userfaultfd/selftests: reinitialize test context in each test").
For the hugetlb test, src and dst areas are allocated as PRIVATE mappings
of a hugetlb file. This means that at mmap time, pages are reserved for
the src and dst areas. At the start of event testing (and other tests)
the src area is populated which results in allocation of huge pages to
fill the area and consumption of reserves associated with the area. Then,
a child is forked to fault in the dst area. Note that the dst area was
allocated in the parent and hence the parent owns the reserves associated
with the mapping. The child has normal access to the dst area, but can
not use the reserves created/owned by the parent. Thus, if there are no
other huge pages available allocation of a page for the dst by the child
will fail.
Fix by not creating reserves for the dst area. In this way the child can
use free (non-reserved) pages.
Also, MAP_PRIVATE of a file only makes sense if you are interested in the
contents of the file before making a COW copy. The test does not do this.
So, just use MAP_ANONYMOUS | MAP_HUGETLB to create an anonymous hugetlb
mapping. There is no need to create a hugetlb file in the non-shared
case.
Link: https://lkml.kernel.org/r/20211217172919.7861-1-mike.kravetz@oracle.com
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Axel Rasmussen <axelrasmussen@google.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Mina Almasry <almasrymina@google.com>
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
tools/testing/selftests/vm/userfaultfd.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
--- a/tools/testing/selftests/vm/userfaultfd.c~userfaultfd-selftests-fix-hugetlb-area-allocations
+++ a/tools/testing/selftests/vm/userfaultfd.c
@@ -87,7 +87,7 @@ static bool test_uffdio_minor = false;
static bool map_shared;
static int shm_fd;
-static int huge_fd;
+static int huge_fd = -1; /* only used for hugetlb_shared test */
static char *huge_fd_off0;
static unsigned long long *count_verify;
static int uffd = -1;
@@ -223,6 +223,9 @@ static void noop_alias_mapping(__u64 *st
static void hugetlb_release_pages(char *rel_area)
{
+ if (huge_fd == -1)
+ return;
+
if (fallocate(huge_fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
rel_area == huge_fd_off0 ? 0 : nr_pages * page_size,
nr_pages * page_size))
@@ -235,16 +238,17 @@ static void hugetlb_allocate_area(void *
char **alloc_area_alias;
*alloc_area = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
- (map_shared ? MAP_SHARED : MAP_PRIVATE) |
- MAP_HUGETLB,
- huge_fd, *alloc_area == area_src ? 0 :
- nr_pages * page_size);
+ map_shared ? MAP_SHARED :
+ MAP_PRIVATE | MAP_HUGETLB |
+ (*alloc_area == area_src ? 0 : MAP_NORESERVE),
+ huge_fd,
+ *alloc_area == area_src ? 0 : nr_pages * page_size);
if (*alloc_area == MAP_FAILED)
err("mmap of hugetlbfs file failed");
if (map_shared) {
area_alias = mmap(NULL, nr_pages * page_size, PROT_READ | PROT_WRITE,
- MAP_SHARED | MAP_HUGETLB,
+ MAP_SHARED,
huge_fd, *alloc_area == area_src ? 0 :
nr_pages * page_size);
if (area_alias == MAP_FAILED)
_
^ permalink raw reply [flat|nested] 3+ messages in thread* [patch 2/2] mm/damon/dbgfs: fix 'struct pid' leaks in 'dbgfs_target_ids_write()'
2021-12-31 4:12 incoming Andrew Morton
2021-12-31 4:12 ` [patch 1/2] userfaultfd/selftests: fix hugetlb area allocations Andrew Morton
@ 2021-12-31 4:12 ` Andrew Morton
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Morton @ 2021-12-31 4:12 UTC (permalink / raw)
To: akpm, linux-mm, mm-commits, sj, stable, torvalds
From: SeongJae Park <sj@kernel.org>
Subject: mm/damon/dbgfs: fix 'struct pid' leaks in 'dbgfs_target_ids_write()'
DAMON debugfs interface increases the reference counts of 'struct pid's
for targets from the 'target_ids' file write callback
('dbgfs_target_ids_write()'), but decreases the counts only in DAMON
monitoring termination callback ('dbgfs_before_terminate()').
Therefore, when 'target_ids' file is repeatedly written without DAMON
monitoring start/termination, the reference count is not decreased and
therefore memory for the 'struct pid' cannot be freed. This commit fixes
this issue by decreasing the reference counts when 'target_ids' is
written.
Link: https://lkml.kernel.org/r/20211229124029.23348-1-sj@kernel.org
Fixes: 4bc05954d007 ("mm/damon: implement a debugfs-based user space interface")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org> [5.15+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
mm/damon/dbgfs.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--- a/mm/damon/dbgfs.c~mm-damon-dbgfs-fix-struct-pid-leaks-in-dbgfs_target_ids_write
+++ a/mm/damon/dbgfs.c
@@ -353,6 +353,7 @@ static ssize_t dbgfs_target_ids_write(st
const char __user *buf, size_t count, loff_t *ppos)
{
struct damon_ctx *ctx = file->private_data;
+ struct damon_target *t, *next_t;
bool id_is_pid = true;
char *kbuf, *nrs;
unsigned long *targets;
@@ -397,8 +398,12 @@ static ssize_t dbgfs_target_ids_write(st
goto unlock_out;
}
- /* remove targets with previously-set primitive */
- damon_set_targets(ctx, NULL, 0);
+ /* remove previously set targets */
+ damon_for_each_target_safe(t, next_t, ctx) {
+ if (targetid_is_pid(ctx))
+ put_pid((struct pid *)t->id);
+ damon_destroy_target(t);
+ }
/* Configure the context for the address space type */
if (id_is_pid)
_
^ permalink raw reply [flat|nested] 3+ messages in thread