[PATCH] fork: clarify copy_mm() comment for kernel thread

[PATCH] fork: clarify copy_mm() comment for kernel thread

[Minu Jin]

The existing comment is misleading:

    1. "steal a active VM" suggests this code performs the active_mm
       borrowing, but the actual borrowing happens in context_switch()
       where 'next->active_mm = prev->active_mm' is assigned.
       This code simply returns 0 to skip mm copying for kernel threads.

    2. "active VM" is outdated terminology, the kernel uses "active_mm".

Update the comment to accurately describe what this code does.

Signed-off-by: Minu Jin <s9430939@naver.com>
---
 kernel/fork.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index b1f3915d5f8e..4bfd606004db 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1566,9 +1566,10 @@ static int copy_mm(u64 clone_flags, struct task_struct *tsk)
 	tsk->active_mm = NULL;
 
 	/*
-	 * Are we cloning a kernel thread?
+	 * Kernel threads have no 'mm' of their own as they borrow the
+	 * 'active_mm' from the previous process at context_switch().
 	 *
-	 * We need to steal a active VM for that..
+	 * Return 0 directly to skip the copy_mm process.
 	 */
 	oldmm = current->mm;
 	if (!oldmm)
-- 
2.43.0

Re: [PATCH] fork: clarify copy_mm() comment for kernel thread

[Lorenzo Stoakes]

On Tue, Jan 27, 2026 at 02:53:21PM +0900, Minu Jin wrote:
> The existing comment is misleading:
>
>     1. "steal a active VM" suggests this code performs the active_mm
>        borrowing, but the actual borrowing happens in context_switch()
>        where 'next->active_mm = prev->active_mm' is assigned.
>        This code simply returns 0 to skip mm copying for kernel threads.

Yeah but the comment as it stands implies that you are doing that elsewhere
so I don't think this is a problem.

Referring to a specific other function in a comment unnecessarily makes
bitrot easier.

>
>     2. "active VM" is outdated terminology, the kernel uses "active_mm".

Well VM refers to the virtual address space so it's perfectly fine
terminology.

>
> Update the comment to accurately describe what this code does.

I hate to say it but this sort of enumerated list is very AI-like so if you
are sending a patch that is AI-generated please do indicate that this is so
(and put e.g. Co-Developed-by: <whatever>) if so.

>
> Signed-off-by: Minu Jin <s9430939@naver.com>

Thanks for the patch but I honestly don't think this is all that
misleading. This original comment is from 2005 and I don't think anybody's
been all that confused by it in the meantime.

So I don't think we should take this.

Cheers, Lorenzo

> ---
>  kernel/fork.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b1f3915d5f8e..4bfd606004db 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1566,9 +1566,10 @@ static int copy_mm(u64 clone_flags, struct task_struct *tsk)
>  	tsk->active_mm = NULL;
>
>  	/*
> -	 * Are we cloning a kernel thread?
> +	 * Kernel threads have no 'mm' of their own as they borrow the
> +	 * 'active_mm' from the previous process at context_switch().
>  	 *
> -	 * We need to steal a active VM for that..
> +	 * Return 0 directly to skip the copy_mm process.
>  	 */
>  	oldmm = current->mm;
>  	if (!oldmm)
> --
> 2.43.0
>

Re: [PATCH] fork: clarify copy_mm() comment for kernel thread

[Minu Jin]

On Tue, Jan 27, 2026 at 09:25:26AM +0000, Lorenzo Stoakes wrote:
> On Tue, Jan 27, 2026 at 02:53:21PM +0900, Minu Jin wrote:
> > The existing comment is misleading:
> >
> >     1. "steal a active VM" suggests this code performs the active_mm
> >        borrowing, but the actual borrowing happens in context_switch()
> >        where 'next->active_mm = prev->active_mm' is assigned.
> >        This code simply returns 0 to skip mm copying for kernel threads.
> 
> Yeah but the comment as it stands implies that you are doing that elsewhere
> so I don't think this is a problem.
> 
> Referring to a specific other function in a comment unnecessarily makes
> bitrot easier.
> 
> >
> >     2. "active VM" is outdated terminology, the kernel uses "active_mm".
> 
> Well VM refers to the virtual address space so it's perfectly fine
> terminology.
> 
> >
> > Update the comment to accurately describe what this code does.
> 
> I hate to say it but this sort of enumerated list is very AI-like so if you
> are sending a patch that is AI-generated please do indicate that this is so
> (and put e.g. Co-Developed-by: <whatever>) if so.
> 
> >
> > Signed-off-by: Minu Jin <s9430939@naver.com>
> 
> Thanks for the patch but I honestly don't think this is all that
> misleading. This original comment is from 2005 and I don't think anybody's
> been all that confused by it in the meantime.
> 
> So I don't think we should take this.
> 
> Cheers, Lorenzo
> 

Hi Lorenzo,

Thanks for your feedback and historical context.
I understand your point about preserving the existing comment.

About 'AI-like' formatting:
I am not a native English speaker, 
so I used an AI tool to help structure and refine my English sentences
for better clarity.

However, the technical analysis and the logic in the patch 
were entirely my own findings from studying the code.

I'll keep your advice in mind regarding 'bitrot' and will focus
on more impactful technical issues in the future.

Thanks again for the review.

Minu Jin.