On Mon, Feb 12, 2024 at 07:29:51PM +0100, Vlastimil Babka wrote:
> The memalloc_noreclaim_save() function currently has no documentation
> comment, so the implications of its usage are not obvious. Namely that
> it not only prevents entering reclaim (as the name suggests), but also
> allows using all memory reserves and thus should be only used in
> contexts that are allocating memory to free memory. This may lead to new
> improper usages being added.
>
> Thus add a documenting comment, based on the description of
> __GFP_MEMALLOC. While at it, also document memalloc_pin_save() so that
> all the memalloc_ scopes are documented. In the comments describing the
> relevant PF_MEMALLOC flags, refer to their scope setting functions.
>
> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx>
Except few nits below
Reviewed-by: Mike Rapoport (IBM) <rppt@xxxxxxxxxx>
> ---
> include/linux/sched.h | 9 ++++----
> include/linux/sched/mm.h | 45 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index ffe8f618ab86..f2cb479f56a7 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1623,15 +1623,15 @@ extern struct pid *cad_pid;
> #define PF_SUPERPRIV 0x00000100 /* Used super-user privileges */
> #define PF_DUMPCORE 0x00000200 /* Dumped core */
> #define PF_SIGNALED 0x00000400 /* Killed by a signal */
> -#define PF_MEMALLOC 0x00000800 /* Allocating memory */
> +#define PF_MEMALLOC 0x00000800 /* Allocating memory to free memory. See memalloc_noreclaim_save() */
> #define PF_NPROC_EXCEEDED 0x00001000 /* set_user() noticed that RLIMIT_NPROC was exceeded */
> #define PF_USED_MATH 0x00002000 /* If unset the fpu must be initialized before use */
> #define PF_USER_WORKER 0x00004000 /* Kernel thread cloned from userspace thread */
> #define PF_NOFREEZE 0x00008000 /* This thread should not be frozen */
> #define PF__HOLE__00010000 0x00010000
> #define PF_KSWAPD 0x00020000 /* I am kswapd */
> -#define PF_MEMALLOC_NOFS 0x00040000 /* All allocation requests will inherit GFP_NOFS */
> -#define PF_MEMALLOC_NOIO 0x00080000 /* All allocation requests will inherit GFP_NOIO */
> +#define PF_MEMALLOC_NOFS 0x00040000 /* All allocations inherit GFP_NOFS. See memalloc_nfs_save() */
> +#define PF_MEMALLOC_NOIO 0x00080000 /* All allocations inherit GFP_NOIO. See memalloc_noio_save() */
> #define PF_LOCAL_THROTTLE 0x00100000 /* Throttle writes only against the bdi I write to,
> * I am cleaning dirty pages from some other bdi. */
> #define PF_KTHREAD 0x00200000 /* I am a kernel thread */
> @@ -1641,7 +1641,8 @@ extern struct pid *cad_pid;
> #define PF__HOLE__02000000 0x02000000
> #define PF_NO_SETAFFINITY 0x04000000 /* Userland is not allowed to meddle with cpus_mask */
> #define PF_MCE_EARLY 0x08000000 /* Early kill for mce process policy */
> -#define PF_MEMALLOC_PIN 0x10000000 /* Allocation context constrained to zones which allow long term pinning. */
> +#define PF_MEMALLOC_PIN 0x10000000 /* Allocations constrained to zones which allow long term pinning.
> + * See memalloc_pin_save() */
> #define PF__HOLE__20000000 0x20000000
> #define PF__HOLE__40000000 0x40000000
> #define PF_SUSPEND_TASK 0x80000000 /* This thread called freeze_processes() and should not be frozen */
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index 9a19f1b42f64..eef8fa5ba5de 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -368,6 +368,27 @@ static inline void memalloc_nofs_restore(unsigned int flags)
> current->flags = (current->flags & ~PF_MEMALLOC_NOFS) | flags;
> }
>
> +/**
> + * memalloc_noreclaim_save - Marks implicit __GFP_MEMALLOC scope.
> + *
> + * This functions marks the beginning of the __GFP_MEMALLOC allocation scope.
^ function
> + * All further allocations will implicitly add the __GFP_MEMALLOC flag, which
> + * prevents entering reclaim and allows access to all memory reserves. This
> + * should only be used when the caller guarantees the allocation will allow more
> + * memory to be freed very shortly, i.e. it needs to allocate some memory in
> + * the process of freeing memory, and cannot reclaim due to potential recursion.
> + *
> + * Users of this scope have to be extremely careful to not deplete the reserves
> + * completely and implement a throttling mechanism which controls the
> + * consumption of the reserve based on the amount of freed memory. Usage of a
> + * pre-allocated pool (e.g. mempool) should be always considered before using
> + * this scope.
> + *
> + * Individual allocations under the scope can opt out using __GFP_NOMEMALLOC
> + *
> + * This function should not be used in an interrupt context as that one does not
> + * give PF_MEMALLOC access to reserves, see __gfp_pfmemalloc_flags().
Missing Return: description
> + */
> static inline unsigned int memalloc_noreclaim_save(void)
> {
> unsigned int flags = current->flags & PF_MEMALLOC;
> @@ -375,11 +396,27 @@ static inline unsigned int memalloc_noreclaim_save(void)
> return flags;
> }
>
> +/**
> + * memalloc_noreclaim_restore - Ends the implicit __GFP_MEMALLOC scope.
> + * @flags: Flags to restore.
> + *
> + * Ends the implicit __GFP_MEMALLOC scope started by memalloc_noreclaim_save
> + * function. Always make sure that the given flags is the return value from the
> + * pairing memalloc_noreclaim_save call.
> + */
> static inline void memalloc_noreclaim_restore(unsigned int flags)
> {
> current->flags = (current->flags & ~PF_MEMALLOC) | flags;
> }
>
> +/**
> + * memalloc_pin_save - Marks implicit ~__GFP_MOVABLE scope.
> + *
> + * This functions marks the beginning of the ~__GFP_MOVABLE allocation scope.
^ function
> + * All further allocations will implicitly remove the __GFP_MOVABLE flag, which
> + * will constraint the allocations to zones that allow long term pinning, i.e.
> + * not ZONE_MOVABLE zones.
Missing Return: description
> + */
> static inline unsigned int memalloc_pin_save(void)
> {
> unsigned int flags = current->flags & PF_MEMALLOC_PIN;
> @@ -388,6 +425,14 @@ static inline unsigned int memalloc_pin_save(void)
> return flags;
> }
>
> +/**
> + * memalloc_pin_restore - Ends the implicit ~__GFP_MOVABLE scope.
> + * @flags: Flags to restore.
> + *
> + * Ends the implicit ~__GFP_MOVABLE scope started by memalloc_pin_save function.
> + * Always make sure that the given flags is the return value from the pairing
> + * memalloc_pin_save call.
> + */
> static inline void memalloc_pin_restore(unsigned int flags)
> {
> current->flags = (current->flags & ~PF_MEMALLOC_PIN) | flags;
> --
> 2.43.0
>
>
--
Sincerely yours,
Mike.
