[PATCH 0/3] EDITME: cover title for b4/kunit-user-alloc

[PATCH 0/3] EDITME: cover title for b4/kunit-user-alloc

[Brendan Jackman]

kunit_attach_mm() leaks an mm_struct (verified with dumb printf
debugging). Fix that. In the process, add a new kthread mm operation,
and clean up some nearby cleanup code in the KUnit lib.

---
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.

---
Brendan Jackman (3):
      kunit: test: Delete pointless resource API usage
      kthread: Add kthread_take_mm()
      kunit: test: fix mm_struct leak in kunit_attach_mm()

 include/linux/kthread.h |  1 +
 kernel/fork.c           |  3 +-
 kernel/kthread.c        | 36 +++++++++++++++++------
 lib/kunit/user_alloc.c  | 78 +++++--------------------------------------------
 4 files changed, 37 insertions(+), 81 deletions(-)
---
base-commit: 9448598b22c50c8a5bb77a9103e2d49f134c9578
change-id: 20251223-b4-kunit-user-alloc-6ae8df0b8a92

Best regards,
-- 
Brendan Jackman <jackmanb@google.com>

[PATCH 1/3] kunit: test: Delete pointless resource API usage

[Brendan Jackman]

This code uses the low-level resource API to track parameters of the
vm_mmap call, but it doesn't do anything with them, because the mm
teardown code takes care of tearing down the mmaps. Delete it.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 lib/kunit/user_alloc.c | 76 ++++----------------------------------------------
 1 file changed, 6 insertions(+), 70 deletions(-)

diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c
index b8cac765e6204..564f5566641d5 100644
--- a/lib/kunit/user_alloc.c
+++ b/lib/kunit/user_alloc.c
@@ -7,21 +7,6 @@
 #include <linux/kthread.h>
 #include <linux/mm.h>
 
-struct kunit_vm_mmap_resource {
-	unsigned long addr;
-	size_t size;
-};
-
-/* vm_mmap() arguments */
-struct kunit_vm_mmap_params {
-	struct file *file;
-	unsigned long addr;
-	unsigned long len;
-	unsigned long prot;
-	unsigned long flag;
-	unsigned long offset;
-};
-
 int kunit_attach_mm(void)
 {
 	struct mm_struct *mm;
@@ -50,67 +35,18 @@ int kunit_attach_mm(void)
 }
 EXPORT_SYMBOL_GPL(kunit_attach_mm);
 
-static int kunit_vm_mmap_init(struct kunit_resource *res, void *context)
-{
-	struct kunit_vm_mmap_params *p = context;
-	struct kunit_vm_mmap_resource vres;
-	int ret;
-
-	ret = kunit_attach_mm();
-	if (ret)
-		return ret;
-
-	vres.size = p->len;
-	vres.addr = vm_mmap(p->file, p->addr, p->len, p->prot, p->flag, p->offset);
-	if (!vres.addr)
-		return -ENOMEM;
-	res->data = kmemdup(&vres, sizeof(vres), GFP_KERNEL);
-	if (!res->data) {
-		vm_munmap(vres.addr, vres.size);
-		return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static void kunit_vm_mmap_free(struct kunit_resource *res)
-{
-	struct kunit_vm_mmap_resource *vres = res->data;
-
-	/*
-	 * Since this is executed from the test monitoring process,
-	 * the test's mm has already been torn down. We don't need
-	 * to run vm_munmap(vres->addr, vres->size), only clean up
-	 * the vres.
-	 */
-
-	kfree(vres);
-	res->data = NULL;
-}
-
 unsigned long kunit_vm_mmap(struct kunit *test, struct file *file,
 			    unsigned long addr, unsigned long len,
 			    unsigned long prot, unsigned long flag,
 			    unsigned long offset)
 {
-	struct kunit_vm_mmap_params params = {
-		.file = file,
-		.addr = addr,
-		.len = len,
-		.prot = prot,
-		.flag = flag,
-		.offset = offset,
-	};
-	struct kunit_vm_mmap_resource *vres;
+	int err;
 
-	vres = kunit_alloc_resource(test,
-				    kunit_vm_mmap_init,
-				    kunit_vm_mmap_free,
-				    GFP_KERNEL,
-				    &params);
-	if (vres)
-		return vres->addr;
-	return 0;
+	err = kunit_attach_mm();
+	if (err)
+		return err;
+
+	return vm_mmap(file, addr, len, prot, flag, offset);
 }
 EXPORT_SYMBOL_GPL(kunit_vm_mmap);
 

-- 
2.51.2

[PATCH 2/3] kthread: Add kthread_take_mm()

[Brendan Jackman]

lib/kunit/user_alloc.c currently uses kthread_use_mm() without a
corresponding kthread_unuse_mm(). This is a bug, but fixing it in KUnit
makes writing tests that use mms more difficult, because of KUnit's
resource/try-catch model.

Therefore, introduce a new operation that does what kunit_attach_mm()
wants, namely an unbalanced call with cleanup deferred to
kthread_exit().

This is actually just the same as kthread_use_mm() but without taking a
reference on the mm_struct.

While adding this, clarify the reference returned by mm_alloc(), since
that is what kthread_take_mm() is gonna be paired with, in practice.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 include/linux/kthread.h |  1 +
 kernel/fork.c           |  3 ++-
 kernel/kthread.c        | 36 +++++++++++++++++++++++++++---------
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 8d27403888ce9..2e6244d8ff1a3 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -259,6 +259,7 @@ bool kthread_cancel_delayed_work_sync(struct kthread_delayed_work *work);
 
 void kthread_destroy_worker(struct kthread_worker *worker);
 
+void kthread_take_mm(struct mm_struct *mm);
 void kthread_use_mm(struct mm_struct *mm);
 void kthread_unuse_mm(struct mm_struct *mm);
 
diff --git a/kernel/fork.c b/kernel/fork.c
index b1f3915d5f8ec..761e6232ea75a 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1147,7 +1147,8 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 }
 
 /*
- * Allocate and initialize an mm_struct.
+ * Allocate and initialize an mm_struct. The caller gets a single reference to
+ * the mm's address space, which should be released with a call to mmput().
  */
 struct mm_struct *mm_alloc(void)
 {
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 99a3808d086f0..c660c04a1b627 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -1589,10 +1589,16 @@ void kthread_destroy_worker(struct kthread_worker *worker)
 EXPORT_SYMBOL(kthread_destroy_worker);
 
 /**
- * kthread_use_mm - make the calling kthread operate on an address space
+ * kthread_take_mm - make the calling kthread own an address space.
+ *
+ * Unlike kthread_use_mm(), this doesn't have a cleanup, instead that happens
+ * automatically on kthread exit. Correspondingly, it does not take any
+ * references, by calling this function you donate your reference to the address
+ * space (from mmget()/mm_users).
+ *
  * @mm: address space to operate on
  */
-void kthread_use_mm(struct mm_struct *mm)
+void kthread_take_mm(struct mm_struct *mm)
 {
 	struct mm_struct *active_mm;
 	struct task_struct *tsk = current;
@@ -1600,13 +1606,6 @@ void kthread_use_mm(struct mm_struct *mm)
 	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
 	WARN_ON_ONCE(tsk->mm);
 
-	/*
-	 * It is possible for mm to be the same as tsk->active_mm, but
-	 * we must still mmgrab(mm) and mmdrop_lazy_tlb(active_mm),
-	 * because these references are not equivalent.
-	 */
-	mmgrab(mm);
-
 	task_lock(tsk);
 	/* Hold off tlb flush IPIs while switching mm's */
 	local_irq_disable();
@@ -1632,6 +1631,25 @@ void kthread_use_mm(struct mm_struct *mm)
 	 */
 	mmdrop_lazy_tlb(active_mm);
 }
+EXPORT_SYMBOL_GPL(kthread_take_mm);
+
+/**
+ * kthread_use_mm - make the calling kthread operate on an address space.
+ *
+ * This must be paired with a call to kthread_unuse_mm().
+ *
+ * @mm: address space to operate on
+ */
+void kthread_use_mm(struct mm_struct *mm)
+{
+	/*
+	 * It is possible for mm to be the same as tsk->active_mm, but we must
+	 * still mmgrab(mm) and mmdrop_lazy_tlb(active_mm) (in
+	 * kthread_take_mm()), because these references are not equivalent.
+	 */
+	mmgrab(mm);
+	kthread_take_mm(mm);
+}
 EXPORT_SYMBOL_GPL(kthread_use_mm);
 
 /**

-- 
2.51.2

[PATCH 3/3] kunit: test: fix mm_struct leak in kunit_attach_mm()

[Brendan Jackman]

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>
---
 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, &current->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

Re: [PATCH 0/3] EDITME: cover title for b4/kunit-user-alloc

[Brendan Jackman]

> Subject: Re: [PATCH 0/3] EDITME: cover title for b4/kunit-user-alloc

I hope everyone is suitably impressed by this. My talent for discovering
new ways of screwing up LKML submissions is simply unparalleled.

This should say "fix mm_struct leak in kunit_attach_mm()".

Re: [PATCH 1/3] kunit: test: Delete pointless resource API usage

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 3769 bytes --]

On Wed, 24 Dec 2025 at 00:18, Brendan Jackman <jackmanb@google.com> wrote:
>
> This code uses the low-level resource API to track parameters of the
> vm_mmap call, but it doesn't do anything with them, because the mm
> teardown code takes care of tearing down the mmaps. Delete it.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---

This makes sense. Maybe there's a case where tracking mmaps as
resources could be useful in the future, but I can't think of any off
the top of my head, so this is just wasteful for now.

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David



>  lib/kunit/user_alloc.c | 76 ++++----------------------------------------------
>  1 file changed, 6 insertions(+), 70 deletions(-)
>
> diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c
> index b8cac765e6204..564f5566641d5 100644
> --- a/lib/kunit/user_alloc.c
> +++ b/lib/kunit/user_alloc.c
> @@ -7,21 +7,6 @@
>  #include <linux/kthread.h>
>  #include <linux/mm.h>
>
> -struct kunit_vm_mmap_resource {
> -       unsigned long addr;
> -       size_t size;
> -};
> -
> -/* vm_mmap() arguments */
> -struct kunit_vm_mmap_params {
> -       struct file *file;
> -       unsigned long addr;
> -       unsigned long len;
> -       unsigned long prot;
> -       unsigned long flag;
> -       unsigned long offset;
> -};
> -
>  int kunit_attach_mm(void)
>  {
>         struct mm_struct *mm;
> @@ -50,67 +35,18 @@ int kunit_attach_mm(void)
>  }
>  EXPORT_SYMBOL_GPL(kunit_attach_mm);
>
> -static int kunit_vm_mmap_init(struct kunit_resource *res, void *context)
> -{
> -       struct kunit_vm_mmap_params *p = context;
> -       struct kunit_vm_mmap_resource vres;
> -       int ret;
> -
> -       ret = kunit_attach_mm();
> -       if (ret)
> -               return ret;
> -
> -       vres.size = p->len;
> -       vres.addr = vm_mmap(p->file, p->addr, p->len, p->prot, p->flag, p->offset);
> -       if (!vres.addr)
> -               return -ENOMEM;
> -       res->data = kmemdup(&vres, sizeof(vres), GFP_KERNEL);
> -       if (!res->data) {
> -               vm_munmap(vres.addr, vres.size);
> -               return -ENOMEM;
> -       }
> -
> -       return 0;
> -}
> -
> -static void kunit_vm_mmap_free(struct kunit_resource *res)
> -{
> -       struct kunit_vm_mmap_resource *vres = res->data;
> -
> -       /*
> -        * Since this is executed from the test monitoring process,
> -        * the test's mm has already been torn down. We don't need
> -        * to run vm_munmap(vres->addr, vres->size), only clean up
> -        * the vres.
> -        */
> -
> -       kfree(vres);
> -       res->data = NULL;
> -}
> -
>  unsigned long kunit_vm_mmap(struct kunit *test, struct file *file,
>                             unsigned long addr, unsigned long len,
>                             unsigned long prot, unsigned long flag,
>                             unsigned long offset)
>  {
> -       struct kunit_vm_mmap_params params = {
> -               .file = file,
> -               .addr = addr,
> -               .len = len,
> -               .prot = prot,
> -               .flag = flag,
> -               .offset = offset,
> -       };
> -       struct kunit_vm_mmap_resource *vres;
> +       int err;
>
> -       vres = kunit_alloc_resource(test,
> -                                   kunit_vm_mmap_init,
> -                                   kunit_vm_mmap_free,
> -                                   GFP_KERNEL,
> -                                   &params);
> -       if (vres)
> -               return vres->addr;
> -       return 0;
> +       err = kunit_attach_mm();
> +       if (err)
> +               return err;
> +
> +       return vm_mmap(file, addr, len, prot, flag, offset);
>  }
>  EXPORT_SYMBOL_GPL(kunit_vm_mmap);
>
>
> --
> 2.51.2
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 2/3] kthread: Add kthread_take_mm()

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 5106 bytes --]

On Wed, 24 Dec 2025 at 00:18, Brendan Jackman <jackmanb@google.com> wrote:
>
> lib/kunit/user_alloc.c currently uses kthread_use_mm() without a
> corresponding kthread_unuse_mm(). This is a bug, but fixing it in KUnit
> makes writing tests that use mms more difficult, because of KUnit's
> resource/try-catch model.
>
> Therefore, introduce a new operation that does what kunit_attach_mm()
> wants, namely an unbalanced call with cleanup deferred to
> kthread_exit().
>
> This is actually just the same as kthread_use_mm() but without taking a
> reference on the mm_struct.
>
> While adding this, clarify the reference returned by mm_alloc(), since
> that is what kthread_take_mm() is gonna be paired with, in practice.
>
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---

This makes some sense to me from the KUnit side, though it'd probably
be nicer to have a way of actually triggering kunit_unuse_mm() at the
right spot. I'm not sure if we'll want to have tests spawn additional
threads sharing the same mm in the future, too, which this shouldn't
make impossible, particularly if we have a requirement that those
threads don't outlast the original test thread.

Otherwise, Is there a reason we can't mmdrop() from another kthread
instead of trying to kthread_unuse_mm()? I wouldn't be surprised (it
doesn't _seem_ right), but seems to work here.

Regardless, I've tested this on a bunch of different KUnit configs
(UML, x86, arm, powerpc, m68k, etc) and nothing has gone wrong. But
I'm definitely not an mm expert, so someone who is probably should
look over this as well.

Reviewed-by: David Gow <davidgow@google.com> # For KUnit

Cheers,
-- David



>  include/linux/kthread.h |  1 +
>  kernel/fork.c           |  3 ++-
>  kernel/kthread.c        | 36 +++++++++++++++++++++++++++---------
>  3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> index 8d27403888ce9..2e6244d8ff1a3 100644
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
> @@ -259,6 +259,7 @@ bool kthread_cancel_delayed_work_sync(struct kthread_delayed_work *work);
>
>  void kthread_destroy_worker(struct kthread_worker *worker);
>
> +void kthread_take_mm(struct mm_struct *mm);
>  void kthread_use_mm(struct mm_struct *mm);
>  void kthread_unuse_mm(struct mm_struct *mm);
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b1f3915d5f8ec..761e6232ea75a 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1147,7 +1147,8 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
>  }
>
>  /*
> - * Allocate and initialize an mm_struct.
> + * Allocate and initialize an mm_struct. The caller gets a single reference to
> + * the mm's address space, which should be released with a call to mmput().
>   */
>  struct mm_struct *mm_alloc(void)
>  {
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 99a3808d086f0..c660c04a1b627 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -1589,10 +1589,16 @@ void kthread_destroy_worker(struct kthread_worker *worker)
>  EXPORT_SYMBOL(kthread_destroy_worker);
>
>  /**
> - * kthread_use_mm - make the calling kthread operate on an address space
> + * kthread_take_mm - make the calling kthread own an address space.
> + *
> + * Unlike kthread_use_mm(), this doesn't have a cleanup, instead that happens
> + * automatically on kthread exit. Correspondingly, it does not take any
> + * references, by calling this function you donate your reference to the address
> + * space (from mmget()/mm_users).
> + *
>   * @mm: address space to operate on
>   */
> -void kthread_use_mm(struct mm_struct *mm)
> +void kthread_take_mm(struct mm_struct *mm)
>  {
>         struct mm_struct *active_mm;
>         struct task_struct *tsk = current;
> @@ -1600,13 +1606,6 @@ void kthread_use_mm(struct mm_struct *mm)
>         WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
>         WARN_ON_ONCE(tsk->mm);
>
> -       /*
> -        * It is possible for mm to be the same as tsk->active_mm, but
> -        * we must still mmgrab(mm) and mmdrop_lazy_tlb(active_mm),
> -        * because these references are not equivalent.
> -        */
> -       mmgrab(mm);
> -
>         task_lock(tsk);
>         /* Hold off tlb flush IPIs while switching mm's */
>         local_irq_disable();
> @@ -1632,6 +1631,25 @@ void kthread_use_mm(struct mm_struct *mm)
>          */
>         mmdrop_lazy_tlb(active_mm);
>  }
> +EXPORT_SYMBOL_GPL(kthread_take_mm);
> +
> +/**
> + * kthread_use_mm - make the calling kthread operate on an address space.
> + *
> + * This must be paired with a call to kthread_unuse_mm().
> + *
> + * @mm: address space to operate on
> + */
> +void kthread_use_mm(struct mm_struct *mm)
> +{
> +       /*
> +        * It is possible for mm to be the same as tsk->active_mm, but we must
> +        * still mmgrab(mm) and mmdrop_lazy_tlb(active_mm) (in
> +        * kthread_take_mm()), because these references are not equivalent.
> +        */
> +       mmgrab(mm);
> +       kthread_take_mm(mm);
> +}
>  EXPORT_SYMBOL_GPL(kthread_use_mm);
>
>  /**
>
> --
> 2.51.2
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 3/3] kunit: test: fix mm_struct leak in kunit_attach_mm()

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 3374 bytes --]

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, &current->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, &current->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
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5281 bytes --]

Re: [PATCH 2/3] kthread: Add kthread_take_mm()

[Brendan Jackman]

On Mon Jan 5, 2026 at 9:56 AM UTC, David Gow wrote:
> On Wed, 24 Dec 2025 at 00:18, Brendan Jackman <jackmanb@google.com> wrote:
>>
>> lib/kunit/user_alloc.c currently uses kthread_use_mm() without a
>> corresponding kthread_unuse_mm(). This is a bug, but fixing it in KUnit
>> makes writing tests that use mms more difficult, because of KUnit's
>> resource/try-catch model.
>>
>> Therefore, introduce a new operation that does what kunit_attach_mm()
>> wants, namely an unbalanced call with cleanup deferred to
>> kthread_exit().
>>
>> This is actually just the same as kthread_use_mm() but without taking a
>> reference on the mm_struct.
>>
>> While adding this, clarify the reference returned by mm_alloc(), since
>> that is what kthread_take_mm() is gonna be paired with, in practice.
>>
>> Signed-off-by: Brendan Jackman <jackmanb@google.com>
>> ---
>
> This makes some sense to me from the KUnit side, though it'd probably
> be nicer to have a way of actually triggering kunit_unuse_mm() at the
> right spot. I'm not sure if we'll want to have tests spawn additional
> threads sharing the same mm in the future, too, which this shouldn't
> make impossible, particularly if we have a requirement that those
> threads don't outlast the original test thread.
>
> Otherwise, Is there a reason we can't mmdrop() from another kthread
> instead of trying to kthread_unuse_mm()? I wouldn't be surprised (it
> doesn't _seem_ right), but seems to work here.

No I think this works and it's actually how I originally wrote the
patch. 

However I think it's very messy, it depends very heavily on the
implementation of kthread_use_mm(), i.e. it is saying "I assume that
everything in kthread_use_mm() gets undone by kthread_exit(), except
that there's exactly one mmdrop() missing". This seems like a natural
conclusion when you've just spent half an hour staring at
kthread.c and drawing up a stupid little ASCII diagram to try and
drill this godforsaken refcount API into your head... But once you step
away from this patchset I think it would look completely bonkers. Here
I'm looking for a way to actually solve this with a proper API.

On the other hand, I'm now adding a weird special kthread API just to
solve this one little problem in KUnit, which people might reasonably
object to.

So yeah I probably should have laid out some other options in the cover
letter. The ones I can obviously see are:

1. The current proposal.

2. Just call mmdrop() from the other kthread and spray comments
   everywhere to try and make it make sense.

3. Find a way to call kthread_unuse_mm() before the kthread dies, with
   some sort of magic in the kunit_try_catch logic.
   But presumably to make that all work with faulting tests etc is gonna
   mean more special APIs, probably worse than kthread_take_mm(). (I did
   not explore this very carefully so it's possible this is easier than
   I guess).