[PATCH 0/3] few mm helpers for bcachefs

[PATCH 0/3] few mm helpers for bcachefs

[Kent Overstreet]

memalloc_flags_{save,restore} - cleanup of the existing duplication in
our memalloc_*_{save,restore} functions

PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN: we've been talking about
getting away from gfp flags for years, but we've been lacking some
PF_MEMALLOC_* equivalents that we need to make that happen - this adds
some of the missing ones.

they're needed for calling alloc_inode() in bcachefs with btree locks
held (so that we can make lookup() properly transactional), since it
doesn't take a gfp flags param and adding it would require changing
methods in every fs.

Kent Overstreet (3):
  mm: introduce memalloc_flags_{save,restore}
  mm: introduce PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN
  mempool: kvmalloc pool

 include/linux/mempool.h  | 13 +++++++++
 include/linux/sched.h    |  4 +--
 include/linux/sched/mm.h | 60 ++++++++++++++++++++++++++--------------
 mm/mempool.c             | 13 +++++++++
 4 files changed, 67 insertions(+), 23 deletions(-)

-- 
2.43.0

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

[Kent Overstreet]

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
---
 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
-- 
2.43.0

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

[Vlastimil Babka]

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

[PATCH 2/3] mm: introduce PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN

[Kent Overstreet]

Introduce PF_MEMALLOC_* equivalents of some GFP_ flags:

PF_MEMALLOC_NORECLAIM	-> GFP_NOWAIT
PF_MEMALLOC_NOWARN	-> __GFP_NOWARN

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
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 include/linux/sched.h    |  4 ++--
 include/linux/sched/mm.h | 17 +++++++++++++----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 292c31697248..ca08d92b20ac 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1755,8 +1755,8 @@ extern struct pid *cad_pid;
 						 * I am cleaning dirty pages from some other bdi. */
 #define PF_KTHREAD		0x00200000	/* I am a kernel thread */
 #define PF_RANDOMIZE		0x00400000	/* Randomize virtual address space */
-#define PF__HOLE__00800000	0x00800000
-#define PF__HOLE__01000000	0x01000000
+#define PF_MEMALLOC_NORECLAIM	0x00800000	/* All allocation requests will inherit __GFP_NOWARN */
+#define PF_MEMALLOC_NOWARN	0x01000000	/* All allocation requests will inherit __GFP_NOWARN */
 #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 */
diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index f00d7ecc2adf..c29059a76052 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -236,16 +236,25 @@ static inline gfp_t current_gfp_context(gfp_t flags)
 {
 	unsigned int pflags = READ_ONCE(current->flags);
 
-	if (unlikely(pflags & (PF_MEMALLOC_NOIO | PF_MEMALLOC_NOFS | PF_MEMALLOC_PIN))) {
+	if (unlikely(pflags & (PF_MEMALLOC_NOIO |
+			       PF_MEMALLOC_NOFS |
+			       PF_MEMALLOC_NORECLAIM |
+			       PF_MEMALLOC_NOWARN |
+			       PF_MEMALLOC_PIN))) {
 		/*
-		 * NOIO implies both NOIO and NOFS and it is a weaker context
-		 * so always make sure it makes precedence
+		 * Stronger flags before weaker flags:
+		 * NORECLAIM implies NOIO, which in turn implies NOFS
 		 */
-		if (pflags & PF_MEMALLOC_NOIO)
+		if (pflags & PF_MEMALLOC_NORECLAIM)
+			flags &= ~__GFP_DIRECT_RECLAIM;
+		else if (pflags & PF_MEMALLOC_NOIO)
 			flags &= ~(__GFP_IO | __GFP_FS);
 		else if (pflags & PF_MEMALLOC_NOFS)
 			flags &= ~__GFP_FS;
 
+		if (pflags & PF_MEMALLOC_NOWARN)
+			flags |= __GFP_NOWARN;
+
 		if (pflags & PF_MEMALLOC_PIN)
 			flags &= ~__GFP_MOVABLE;
 	}
-- 
2.43.0

Re: [PATCH 2/3] mm: introduce PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN

[Vlastimil Babka]

On 2/6/24 22:50, Kent Overstreet wrote:
> Introduce PF_MEMALLOC_* equivalents of some GFP_ flags:
> 
> PF_MEMALLOC_NORECLAIM	-> GFP_NOWAIT

In an ideal world, this would be nice, but we are in a world with implicit
"too small to fail" guarantees that has so far been impossible to get away
from [1] for small order GFP_KERNEL allocations, and this scoping would be
only safe if no allocations underneath relied on this behavior. But how to
ensure that's the case?

[1] https://lwn.net/Articles/723317/

> PF_MEMALLOC_NOWARN	-> __GFP_NOWARN
> 
> 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
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> ---
>  include/linux/sched.h    |  4 ++--
>  include/linux/sched/mm.h | 17 +++++++++++++----
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 292c31697248..ca08d92b20ac 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1755,8 +1755,8 @@ extern struct pid *cad_pid;
>  						 * I am cleaning dirty pages from some other bdi. */
>  #define PF_KTHREAD		0x00200000	/* I am a kernel thread */
>  #define PF_RANDOMIZE		0x00400000	/* Randomize virtual address space */
> -#define PF__HOLE__00800000	0x00800000
> -#define PF__HOLE__01000000	0x01000000
> +#define PF_MEMALLOC_NORECLAIM	0x00800000	/* All allocation requests will inherit __GFP_NOWARN */
> +#define PF_MEMALLOC_NOWARN	0x01000000	/* All allocation requests will inherit __GFP_NOWARN */
>  #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 */
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index f00d7ecc2adf..c29059a76052 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -236,16 +236,25 @@ static inline gfp_t current_gfp_context(gfp_t flags)
>  {
>  	unsigned int pflags = READ_ONCE(current->flags);
>  
> -	if (unlikely(pflags & (PF_MEMALLOC_NOIO | PF_MEMALLOC_NOFS | PF_MEMALLOC_PIN))) {
> +	if (unlikely(pflags & (PF_MEMALLOC_NOIO |
> +			       PF_MEMALLOC_NOFS |
> +			       PF_MEMALLOC_NORECLAIM |
> +			       PF_MEMALLOC_NOWARN |
> +			       PF_MEMALLOC_PIN))) {
>  		/*
> -		 * NOIO implies both NOIO and NOFS and it is a weaker context
> -		 * so always make sure it makes precedence
> +		 * Stronger flags before weaker flags:
> +		 * NORECLAIM implies NOIO, which in turn implies NOFS
>  		 */
> -		if (pflags & PF_MEMALLOC_NOIO)
> +		if (pflags & PF_MEMALLOC_NORECLAIM)
> +			flags &= ~__GFP_DIRECT_RECLAIM;
> +		else if (pflags & PF_MEMALLOC_NOIO)
>  			flags &= ~(__GFP_IO | __GFP_FS);
>  		else if (pflags & PF_MEMALLOC_NOFS)
>  			flags &= ~__GFP_FS;
>  
> +		if (pflags & PF_MEMALLOC_NOWARN)
> +			flags |= __GFP_NOWARN;
> +
>  		if (pflags & PF_MEMALLOC_PIN)
>  			flags &= ~__GFP_MOVABLE;
>  	}

Re: [PATCH 2/3] mm: introduce PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN

[Michal Hocko]

On Wed 07-02-24 08:24:33, Vlastimil Babka wrote:
> On 2/6/24 22:50, Kent Overstreet wrote:
> > Introduce PF_MEMALLOC_* equivalents of some GFP_ flags:
> > 
> > PF_MEMALLOC_NORECLAIM	-> GFP_NOWAIT
> 
> In an ideal world, this would be nice, but we are in a world with implicit
> "too small to fail" guarantees that has so far been impossible to get away
> from [1] for small order GFP_KERNEL allocations, and this scoping would be
> only safe if no allocations underneath relied on this behavior. But how to
> ensure that's the case?

Right http://lkml.kernel.org/r/Zbu_yyChbCO6b2Lj@tiehlicka

> [1] https://lwn.net/Articles/723317/
-- 
Michal Hocko
SUSE Labs

Re: [PATCH 2/3] mm: introduce PF_MEMALLOC_NORECLAIM, PF_MEMALLOC_NOWARN

[Kent Overstreet]

On Wed, Feb 07, 2024 at 08:24:33AM +0100, Vlastimil Babka wrote:
> On 2/6/24 22:50, Kent Overstreet wrote:
> > Introduce PF_MEMALLOC_* equivalents of some GFP_ flags:
> > 
> > PF_MEMALLOC_NORECLAIM	-> GFP_NOWAIT
> 
> In an ideal world, this would be nice, but we are in a world with implicit
> "too small to fail" guarantees that has so far been impossible to get away
> from [1] for small order GFP_KERNEL allocations, and this scoping would be
> only safe if no allocations underneath relied on this behavior. But how to
> ensure that's the case?

Fault injection. You can't know if code works if it never gets tested,
and if small allocations don't fail in practice, then you need fault
injection.

But there's a code pattern that absolutely requires GFP_NOWAIT. Say
you've got locks held and you want to allocate memory:

p = kmalloc(GFP_NOWAIT);
if (p)
	goto success;
unlock();
p = kmalloc(GFP_KERNEL);

/* unwind and retry, or tryrelock, depending on what you're doing */

that is - try the allocation nonblocking, then unlock or unwind, then
try it GFP_KERNEL.

bcachefs uses this heavily because we've got bch2_trans_unlock() and
bch2_trans_relock(); relock succeeds iff nothing else took write locks
on the nodes he had locked before - so we can safely use GFP_KERNEL
without causing deadlocks, only the occasional transaction restart.

but: the first GFP_NOWAIT allocation, before using GFP_KERNEL, is
absolutely required - calling unlock() has to be a slowpath operation,
otherwise it will livelock when multiple threads are contending for the
same locks.

More broadly, there's a bunch of other GFP_NOWAIT uses in the kernel,
and we're _not_ going to kill them off, and we are trying to kill off
gfp_t for this kind of purpose - so we need this.

[PATCH 3/3] mempool: kvmalloc pool

[Kent Overstreet]

Add mempool_init_kvmalloc_pool() and mempool_create_kvmalloc_pool(),
which wrap kvmalloc() instead of kmalloc() - kmalloc() with a vmalloc()
fallback.

This is part of a bcachefs cleanup - dropping an internal kvpmalloc()
helper (which predates kvmalloc()) along with mempool helpers; this
replaces the bcachefs-private kvpmalloc_pool.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Cc: linux-mm@kvack.org
---
 include/linux/mempool.h | 13 +++++++++++++
 mm/mempool.c            | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/include/linux/mempool.h b/include/linux/mempool.h
index 4aae6c06c5f2..92181401ea96 100644
--- a/include/linux/mempool.h
+++ b/include/linux/mempool.h
@@ -94,6 +94,19 @@ static inline mempool_t *mempool_create_kmalloc_pool(int min_nr, size_t size)
 			      (void *) size);
 }
 
+void *mempool_kvmalloc(gfp_t gfp_mask, void *pool_data);
+void mempool_kvfree(void *element, void *pool_data);
+
+static inline int mempool_init_kvmalloc_pool(mempool_t *pool, int min_nr, size_t size)
+{
+	return mempool_init(pool, min_nr, mempool_kvmalloc, mempool_kvfree, (void *) size);
+}
+
+static inline mempool_t *mempool_create_kvmalloc_pool(int min_nr, size_t size)
+{
+	return mempool_create(min_nr, mempool_kvmalloc, mempool_kvfree, (void *) size);
+}
+
 /*
  * A mempool_alloc_t and mempool_free_t for a simple page allocator that
  * allocates pages of the order specified by pool_data
diff --git a/mm/mempool.c b/mm/mempool.c
index 734bcf5afbb7..343f0ef6b69c 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -543,6 +543,19 @@ void mempool_kfree(void *element, void *pool_data)
 }
 EXPORT_SYMBOL(mempool_kfree);
 
+void *mempool_kvmalloc(gfp_t gfp_mask, void *pool_data)
+{
+	size_t size = (size_t)pool_data;
+	return kvmalloc(size, gfp_mask);
+}
+EXPORT_SYMBOL(mempool_kvmalloc);
+
+void mempool_kvfree(void *element, void *pool_data)
+{
+	kvfree(element);
+}
+EXPORT_SYMBOL(mempool_kvfree);
+
 /*
  * A simple mempool-backed page allocator that allocates pages
  * of the order specified by pool_data.
-- 
2.43.0