Re: [PATCH v2 02/11] asm-generic/pgtable: Adds dummy functions to monitor lockless pgtable walks

[John Hubbard <jhubbard@nvidia.com>]

On 9/20/19 12:50 PM, Leonardo Bras wrote:
> There is a need to monitor lockless pagetable walks, in order to avoid
> doing THP splitting/collapsing during them.
> 
> Some methods rely on local_irq_{save,restore}, but that can be slow on
> cases with a lot of cpus are used for the process.
> 
> In order to speedup these cases, I propose a refcount-based approach, that
> counts the number of lockless pagetable	walks happening on the process.
> 
> Given that there are lockless pagetable walks on generic code, it's
> necessary to create dummy functions for archs that won't use the approach.
> 
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
>  include/asm-generic/pgtable.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index 75d9d68a6de7..6eb4fabb5595 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -1172,6 +1172,15 @@ static inline bool arch_has_pfn_modify_check(void)
>  #endif
>  #endif
>  
> +#ifndef __HAVE_ARCH_LOCKLESS_PGTBL_WALK_COUNTER
> +static inline void start_lockless_pgtbl_walk(struct mm_struct *mm) { }
> +static inline void end_lockless_pgtbl_walk(struct mm_struct *mm) { }
> +static inline int running_lockless_pgtbl_walk(struct mm_struct *mm)
> +{
> +	return 0;
> +}
> +#endif
> +

Please remember to include linux-mm if there is a v2.

Nit: seems like it would be nicer to just put it all in one place, and use
positive logic, and also I think people normally don't compress the empty
functions quite that much. So like this:

#ifdef __HAVE_ARCH_LOCKLESS_PGTBL_WALK_COUNTER
void start_lockless_pgtbl_walk(struct mm_struct *mm); 
void end_lockless_pgtbl_walk(struct mm_struct *mm); 
int running_lockless_pgtbl_walk(struct mm_struct *mm); 

#else
static inline void start_lockless_pgtbl_walk(struct mm_struct *mm)
{
}
static inline void end_lockless_pgtbl_walk(struct mm_struct *mm)
{
}
static inline int running_lockless_pgtbl_walk(struct mm_struct *mm)
{
	return 0;
}
#endif

thanks,
-- 
John Hubbard
NVIDIA