Re: [PATCH v2 3/3] um: Stop tracking stub's PID via userspace_pid[]

[Tiwei Bie <tiwei.bie@linux.dev>]

On Fri, 11 Jul 2025 09:03:26 +0200, Johannes Berg wrote:
> On Fri, 2025-07-11 at 14:50 +0800, Tiwei Bie wrote:
> > From: Tiwei Bie <tiwei.btw@antgroup.com>
> > 
> > The PID of the stub process can be obtained from current_mm_id().
> > There is no need to track it via userspace_pid[]. Stop doing that
> > to simplify the code.
> 
> So that is really obvious cleanups, and I can go apply them on that
> grounds, but I started wondering if we're not separately being
> inconsistent here, which perhaps didn't matter due to non-SMP:
> 
> >  #define activate_mm activate_mm
> >  static inline void activate_mm(struct mm_struct *old, struct mm_struct *new)
> >  {
> > -	/*
> > -	 * This is called by fs/exec.c and sys_unshare()
> > -	 * when the new ->mm is used for the first time.
> > -	 */
> > -	__switch_mm(&new->context.id);
> >  }
> 
> This is now empty, so I wondered if we can just remove it _entirely_.
> 
> But the generic version calls switch_mm():

Yeah, removing activate_mm() will cause it to call switch_mm(). This is
somewhat a behavior change beyond the scope of this patchset, so I kept
it as is.

>  
> >  static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, 
> > @@ -28,11 +23,9 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
> >  {
> >  	unsigned cpu = smp_processor_id();
> >  
> > -	if(prev != next){
> > +	if (prev != next) {
> >  		cpumask_clear_cpu(cpu, mm_cpumask(prev));
> >  		cpumask_set_cpu(cpu, mm_cpumask(next));
> > -		if(next != &init_mm)
> > -			__switch_mm(&next->context.id);
> >  	}
> >  }
> 
> which plays with the CPU mask, but realistically being non-SMP the CPU
> mask is never really used?
> 
> Certainly removing activate_mm() entirely seems to _work_, but of course
> it never does anything since smp_processor_id() eventually is just
> macros that expand to "0" (unless preempt debug is enabled). Any
> thoughts?

Yeah, things are a bit messy here and need to be sorted out.

IIUC, activate_mm() and switch_mm() are primarily used to update the
page table and manage the TLBs on the CPU for each user address space.
However, in UML, each user address space is represented by a separate
stub process, and the host kernel already takes care of that. So, I'm
not entirely sure why we need to maintain the mm_cpumask. Perhaps we
could just do this:

--- a/arch/um/include/asm/mmu_context.h
+++ b/arch/um/include/asm/mmu_context.h
@@ -13,20 +13,9 @@
 #include <asm/mm_hooks.h>
 #include <asm/mmu.h>
 
-#define activate_mm activate_mm
-static inline void activate_mm(struct mm_struct *old, struct mm_struct *new)
-{
-}
-
 static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next, 
 			     struct task_struct *tsk)
 {
-	unsigned cpu = smp_processor_id();
-
-	if (prev != next) {
-		cpumask_clear_cpu(cpu, mm_cpumask(prev));
-		cpumask_set_cpu(cpu, mm_cpumask(next));
-	}
 }
 
 #define init_new_context init_new_context

Regards,
Tiwei