Re: [patch 10/21] Xen-paravirt: add hooks to intercept mm creation and destruction

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 15 Feb 2007 18:24:59 -0800 Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Add hooks to allow a paravirt implementation to track the lifetime of
> an mm.
> 
> --- a/arch/i386/kernel/paravirt.c
> +++ b/arch/i386/kernel/paravirt.c
> @@ -706,6 +706,10 @@ struct paravirt_ops paravirt_ops = {
>  	.irq_enable_sysexit = native_irq_enable_sysexit,
>  	.iret = native_iret,
>  
> +	.dup_mmap = (void *)native_nop,
> +	.exit_mmap = (void *)native_nop,
> +	.activate_mm = (void *)native_nop,
> +
>  	.startup_ipi_hook = (void *)native_nop,
>  };

eww.  I suppose there's a good reason for the casting.

> ===================================================================
> --- a/include/asm-i386/mmu_context.h
> +++ b/include/asm-i386/mmu_context.h
> @@ -5,6 +5,7 @@
>  #include <asm/atomic.h>
>  #include <asm/pgalloc.h>
>  #include <asm/tlbflush.h>
> +#include <asm/paravirt.h>
>  
>  /*
>   * Used for LDT copy/destruction.
> @@ -65,7 +66,10 @@ static inline void switch_mm(struct mm_s
>  #define deactivate_mm(tsk, mm)			\
>  	asm("movl %0,%%gs": :"r" (0));
>  
> -#define activate_mm(prev, next) \
> -	switch_mm((prev),(next),NULL)
> +#define activate_mm(prev, next)				\
> +	do {						\
> +		arch_activate_mm(prev, next);		\
> +		switch_mm((prev),(next),NULL);		\
> +	} while(0);

It seems strange to call out to arch_foo() from within an arch header file.
I mean, we implicity know we're i386.

Maybe it's just poorly named.

> +static inline void paravirt_activate_mm(struct mm_struct *prev,
> +					struct mm_struct *next)
> +{
> +}
> +
> +static inline void paravirt_dup_mmap(struct mm_struct *oldmm,
> +				     struct mm_struct *mm)
> +{
> +}
> +
> +static inline void paravirt_exit_mmap(struct mm_struct *mm)
> +{
> +}

These functions are unreferenced in this patchset.

>  #endif /* CONFIG_PARAVIRT */
>  #endif	/* __ASM_PARAVIRT_H */
> ===================================================================
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -374,6 +374,12 @@ struct mm_struct {
>  	rwlock_t		ioctx_list_lock;
>  	struct kioctx		*ioctx_list;
>  };
> +
> +#ifndef __HAVE_ARCH_MM_LIFETIME
> +#define arch_activate_mm(prev, next)	do {} while(0)
> +#define arch_dup_mmap(oldmm, mm)	do {} while(0)
> +#define arch_exit_mmap(mm)		do {} while(0)
> +#endif

Can we lose __HAVE_ARCH_MM_LIFETIME?  Just define these (preferably in C,
not in cpp) in the appropriate include/asm-foo/ files?

>  struct sighand_struct {
>  	atomic_t		count;
> ===================================================================
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -286,6 +286,7 @@ static inline int dup_mmap(struct mm_str
>  		if (retval)
>  			goto out;
>  	}
> +	arch_dup_mmap(oldmm, mm);
>  	retval = 0;
>  out:
>  	up_write(&mm->mmap_sem);
> ===================================================================
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1976,6 +1976,8 @@ void exit_mmap(struct mm_struct *mm)
>  	struct vm_area_struct *vma = mm->mmap;
>  	unsigned long nr_accounted = 0;
>  	unsigned long end;
> +
> +	arch_exit_mmap(mm);
>  
>  	lru_add_drain();
>  	flush_cache_mm(mm);

Perhaps some commentary telling the arch maintainer what these hooks he's
being offered are for?