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