On Wed, 24 Dec 2025 at 00:18, Brendan Jackman jackmanb@google.com wrote:
Here's how I understand mm refcounts:
funcs | counter | manages lifecycle of...
mmgrab()/mmdrop() | mm_count | mm_struct and PGD
mmget()/mmput() | mm_users | userspace address space
All mm_users references share a single reference to the mm_struct.mm_alloc() returns the mm with a single reference to the user address space, i.e. with mm_users=1, mm_count=1.
kunit_attach_mm() then passes the mm to kthread_use_mm(). It does not call kthread_unuse_mm(), instead it relies on the kthread exit path to release the relevant resources. It does this because KUnit's resource cleanup logic works by running cleanups in a different kthread from the test. You can't have cleanups that operate on the kthread, because the kthread is already gone by the time the cleanup is called.
The kthread exit path will indeed drop the reference to the address space, i.e. it will call mmput(task->mm), decrementing mm_users. However, it does not release the reference taken on the mm_struct when kthread_use_mm() called mmgrab().
To fix this, use the new kthread_take_mm() which provides the API KUnit needs.
Signed-off-by: Brendan Jackman jackmanb@google.com
This seems right. We did have the ability to run cleanups on the test kthread, but we couldn't guarantee it (because the test thread could be aborted early, due to test failures, timeouts, etc).
I'm assuming that we can't just throw an mmdrop() in a resource cleanup function (e.g. kunit_add_action()), since we can't do a full kthread_unuse_mm()? I gave it a quick try and nothing crashed, but that's not exactly a ringing endorsement that it's correct:
diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c index 3fca4ae223f6..5177537f592b 100644 --- a/lib/kunit/user_alloc.c +++ b/lib/kunit/user_alloc.c @@ -7,6 +7,8 @@ #include <linux/kthread.h> #include <linux/mm.h>
+KUNIT_DEFINE_ACTION_WRAPPER(kunit_mmdrop_action, mmdrop, struct mm_struct*); + int kunit_attach_mm(void) { struct mm_struct *mm; @@ -29,7 +31,9 @@ int kunit_attach_mm(void) arch_pick_mmap_layout(mm, ¤t->signal->rlim[RLIMIT_STACK]);
/* Attach the mm. It will be cleaned up when the process dies. */ - kthread_take_mm(mm); + kthread_use_mm(mm); + + kunit_add_action(current->kunit_test, kunit_mmdrop_action, mm);
return 0; }
That being said, if mm folks are okay with the kthread_take_mm() approach, that certainly makes things easier on our end, so I'm happy with this as an option.
Reviewed-by: David Gow davidgow@google.com
Cheers, -- David
lib/kunit/user_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c index 564f5566641d5..3fca4ae223f67 100644 --- a/lib/kunit/user_alloc.c +++ b/lib/kunit/user_alloc.c @@ -29,7 +29,7 @@ int kunit_attach_mm(void) arch_pick_mmap_layout(mm, ¤t->signal->rlim[RLIMIT_STACK]);
/* Attach the mm. It will be cleaned up when the process dies. */
kthread_use_mm(mm);
kthread_take_mm(mm); return 0;}
-- 2.51.2