Re: [PATCH 1/3] mm: introduce memalloc_flags_{save,restore}

[Vlastimil Babka <vbabka@suse.cz>]

On 2/6/24 22:50, Kent Overstreet wrote:
> Our proliferation of memalloc_*_{save,restore} APIs is getting a bit
> silly, this adds a generic version and converts the existing
> save/restore functions to wrappers.
> 
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Darrick J. Wong <djwong@kernel.org>
> Cc: linux-mm@kvack.org

Acked-by: Vlastimil Babka <vbabka@suse.cz>

This will also make it possible to do combinations of flags in a single
call, in case some ad-hoc combinations are needed only in few places that
don't warrant a designated helper.

> ---
>  include/linux/sched/mm.h | 43 ++++++++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 9a19f1b42f64..f00d7ecc2adf 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -306,6 +306,24 @@ static inline void might_alloc(gfp_t gfp_mask)
>  	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
>  }
>  
> +/**
> + * memalloc_flags_save - Add a PF_* flag to current->flags, save old value
> + *
> + * This allows PF_* flags to be conveniently added, irrespective of current
> + * value, and then the old version restored with memalloc_flags_restore().
> + */
> +static inline unsigned memalloc_flags_save(unsigned flags)
> +{
> +	unsigned oldflags = ~current->flags & flags;
> +	current->flags |= flags;
> +	return oldflags;
> +}
> +
> +static inline void memalloc_flags_restore(unsigned flags)
> +{
> +	current->flags &= ~flags;
> +}
> +
>  /**
>   * memalloc_noio_save - Marks implicit GFP_NOIO allocation scope.
>   *
> @@ -319,9 +337,7 @@ static inline void might_alloc(gfp_t gfp_mask)
>   */
>  static inline unsigned int memalloc_noio_save(void)
>  {
> -	unsigned int flags = current->flags & PF_MEMALLOC_NOIO;
> -	current->flags |= PF_MEMALLOC_NOIO;
> -	return flags;
> +	return memalloc_flags_save(PF_MEMALLOC_NOIO);
>  }
>  
>  /**
> @@ -334,7 +350,7 @@ static inline unsigned int memalloc_noio_save(void)
>   */
>  static inline void memalloc_noio_restore(unsigned int flags)
>  {
> -	current->flags = (current->flags & ~PF_MEMALLOC_NOIO) | flags;
> +	memalloc_flags_restore(flags);
>  }
>  
>  /**
> @@ -350,9 +366,7 @@ static inline void memalloc_noio_restore(unsigned int flags)
>   */
>  static inline unsigned int memalloc_nofs_save(void)
>  {
> -	unsigned int flags = current->flags & PF_MEMALLOC_NOFS;
> -	current->flags |= PF_MEMALLOC_NOFS;
> -	return flags;
> +	return memalloc_flags_save(PF_MEMALLOC_NOFS);
>  }
>  
>  /**
> @@ -365,32 +379,27 @@ static inline unsigned int memalloc_nofs_save(void)
>   */
>  static inline void memalloc_nofs_restore(unsigned int flags)
>  {
> -	current->flags = (current->flags & ~PF_MEMALLOC_NOFS) | flags;
> +	memalloc_flags_restore(flags);
>  }
>  
>  static inline unsigned int memalloc_noreclaim_save(void)
>  {
> -	unsigned int flags = current->flags & PF_MEMALLOC;
> -	current->flags |= PF_MEMALLOC;
> -	return flags;
> +	return memalloc_flags_save(PF_MEMALLOC);
>  }
>  
>  static inline void memalloc_noreclaim_restore(unsigned int flags)
>  {
> -	current->flags = (current->flags & ~PF_MEMALLOC) | flags;
> +	memalloc_flags_restore(flags);
>  }
>  
>  static inline unsigned int memalloc_pin_save(void)
>  {
> -	unsigned int flags = current->flags & PF_MEMALLOC_PIN;
> -
> -	current->flags |= PF_MEMALLOC_PIN;
> -	return flags;
> +	return memalloc_flags_save(PF_MEMALLOC_PIN);
>  }
>  
>  static inline void memalloc_pin_restore(unsigned int flags)
>  {
> -	current->flags = (current->flags & ~PF_MEMALLOC_PIN) | flags;
> +	memalloc_flags_restore(flags);
>  }
>  
>  #ifdef CONFIG_MEMCG