[PATCH 0/2] Align kvrealloc() with krealloc()

[PATCH 0/2] Align kvrealloc() with krealloc()

[Danilo Krummrich]

Hi,

Besides the obvious (and desired) difference between krealloc() and kvrealloc(),
there is some inconsistency in their function signatures and behavior:

 - krealloc() frees the memory when the requested size is zero, whereas
   kvrealloc() simply returns a pointer to the existing allocation.

 - krealloc() behaves like kmalloc() if a NULL pointer is passed, whereas
   kvrealloc() does not accept a NULL pointer at all and, if passed, would fault
   instead.

 - krealloc() is self-contained, whereas kvrealloc() relies on the caller to
   provide the size of the previous allocation.

Inconsistent behavior throughout allocation APIs is error prone, hence make
kvrealloc() behave like krealloc(), which seems superior in all mentioned
aspects.

In order to be able to get rid of kvrealloc()'s oldsize parameter, introduce
vrealloc() and make use of it in kvrealloc().

Making use of vrealloc() in kvrealloc() also provides oppertunities to grow (and
shrink) allocations more efficiently. For instance, vrealloc() could be
optimized to extend the existing allocation if there is enough contiguous space
left in the virtual address space at the end of the existing allocation.

Besides the above, those functions are required by Rust's allocator abstractons
[1] (rework based on this series in [2]). With `Vec` or `KVec` respectively,
potentially growing (and shrinking) data structures are rather common.

The patches of this series can also be found in [3].

[1] https://lore.kernel.org/lkml/20240704170738.3621-1-dakr@redhat.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=rust/mm
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dakr/linux.git/log/?h=mm/krealloc

Danilo Krummrich (2):
  mm: vmalloc: implement vrealloc()
  mm: kvmalloc: align kvrealloc() with krealloc()

 arch/powerpc/platforms/pseries/papr-vpd.c |  5 +-
 drivers/gpu/drm/drm_exec.c                |  3 +-
 fs/xfs/xfs_log_recover.c                  |  2 +-
 include/linux/slab.h                      |  3 +-
 include/linux/vmalloc.h                   |  4 ++
 kernel/resource.c                         |  3 +-
 lib/fortify_kunit.c                       |  3 +-
 mm/util.c                                 | 72 ++++++++++++++++-------
 mm/vmalloc.c                              | 58 ++++++++++++++++++
 9 files changed, 118 insertions(+), 35 deletions(-)


base-commit: 51835949dda3783d4639cfa74ce13a3c9829de00
-- 
2.45.2

[PATCH 1/2] mm: vmalloc: implement vrealloc()

[Danilo Krummrich]

Implement vrealloc() analogous to krealloc().

Currently, krealloc() requires the caller to pass the size of the
previous memory allocation, which, instead, should be self-contained.

We attempt to fix this in a subsequent patch which, in order to do so,
requires vrealloc().

Besides that, we need realloc() functions for kernel allocators in Rust
too. With `Vec` or `KVec` respectively, potentially growing (and
shrinking) data structures are rather common.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 include/linux/vmalloc.h |  4 +++
 mm/vmalloc.c            | 58 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index e4a631ec430b..9ff0a8e5c323 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -189,6 +189,10 @@ extern void *__vcalloc_noprof(size_t n, size_t size, gfp_t flags) __alloc_size(1
 extern void *vcalloc_noprof(size_t n, size_t size) __alloc_size(1, 2);
 #define vcalloc(...)		alloc_hooks(vcalloc_noprof(__VA_ARGS__))
 
+extern void * __must_check vrealloc_noprof(const void *p, size_t size,
+					   gfp_t flags) __realloc_size(2);
+#define vrealloc(...)		alloc_hooks(vrealloc_noprof(__VA_ARGS__))
+
 extern void vfree(const void *addr);
 extern void vfree_atomic(const void *addr);
 
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index e34ea860153f..4ec949ac9d9d 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -4036,6 +4036,64 @@ void *vzalloc_node_noprof(unsigned long size, int node)
 }
 EXPORT_SYMBOL(vzalloc_node_noprof);
 
+/**
+ * vrealloc - reallocate virtually contiguous memory; contents remain unchanged
+ * @p: object to reallocate memory for
+ * @size: the size to reallocate
+ * @flags: the flags for the page level allocator
+ *
+ * The contents of the object pointed to are preserved up to the lesser of the
+ * new and old size (__GFP_ZERO flag is effectively ignored).
+ *
+ * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
+ * @p is not a %NULL pointer, the object pointed to is freed.
+ *
+ * Return: pointer to the allocated memory; %NULL if @size is zero or in case of
+ *         failure
+ */
+void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
+{
+	size_t old_size = 0;
+	void *n;
+
+	if (!size) {
+		vfree(p);
+		return NULL;
+	}
+
+	if (p) {
+		struct vm_struct *vm;
+
+		vm = find_vm_area(p);
+		if (unlikely(!vm)) {
+			WARN(1, "Trying to vrealloc() nonexistent vm area (%p)\n", p);
+			return NULL;
+		}
+
+		old_size = get_vm_area_size(vm);
+	}
+
+	if (size <= old_size) {
+		/* TODO: Can we optimize and shrink the allocation? What would
+		 * be a good metric for when to shrink the vm_area?
+		 */
+		return (void *)p;
+	}
+
+	/* TODO: Can we optimize and extend the existing allocation if we have
+	 * enough contiguous space left in the virtual address space?
+	 */
+	n = __vmalloc_noprof(size, flags);
+
+	if (p) {
+		memcpy(n, p, old_size);
+		vfree(p);
+	}
+
+	return n;
+}
+EXPORT_SYMBOL(vrealloc_noprof);
+
 #if defined(CONFIG_64BIT) && defined(CONFIG_ZONE_DMA32)
 #define GFP_VMALLOC32 (GFP_DMA32 | GFP_KERNEL)
 #elif defined(CONFIG_64BIT) && defined(CONFIG_ZONE_DMA)
-- 
2.45.2

Re: [PATCH 1/2] mm: vmalloc: implement vrealloc()

[Christoph Hellwig]

On Thu, Jul 18, 2024 at 12:24:01AM +0200, Danilo Krummrich wrote:
> +extern void * __must_check vrealloc_noprof(const void *p, size_t size,
> +					   gfp_t flags) __realloc_size(2);

No need for the extern here.

It would also help readability a bit to keep the __realloc_size on a
separate like, aka:

void *__must_check vrealloc_noprof(const void *p, size_t size, gfp_t flags)
		__realloc_size(2);

> +	if (size <= old_size) {
> +		/* TODO: Can we optimize and shrink the allocation? What would
> +		 * be a good metric for when to shrink the vm_area?
> +		 */

Kernel comment style is to keep the

		/*

on a line of it's own.  A realloc that doesn't shrink is a bit
pointless.  The same heuristics as for krealloc should probably apply
here, adjust to the fact that we always deal with whole pages.


> +	/* TODO: Can we optimize and extend the existing allocation if we have
> +	 * enough contiguous space left in the virtual address space?
> +	 */

I don't understand this comment.

> +EXPORT_SYMBOL(vrealloc_noprof);

New interfaces should be EXPORT_SYMBOL_GPL.  That being said for this
to be added an exported we'll need an actual user to start with anyway.

Re: [PATCH 1/2] mm: vmalloc: implement vrealloc()

[Danilo Krummrich]

On Wed, Jul 17, 2024 at 08:16:29PM -0700, Christoph Hellwig wrote:
> On Thu, Jul 18, 2024 at 12:24:01AM +0200, Danilo Krummrich wrote:
> > +extern void * __must_check vrealloc_noprof(const void *p, size_t size,
> > +					   gfp_t flags) __realloc_size(2);
> 
> No need for the extern here.
> 
> It would also help readability a bit to keep the __realloc_size on a
> separate like, aka:
> 
> void *__must_check vrealloc_noprof(const void *p, size_t size, gfp_t flags)
> 		__realloc_size(2);

Sure, will change that.

> 
> > +	if (size <= old_size) {
> > +		/* TODO: Can we optimize and shrink the allocation? What would
> > +		 * be a good metric for when to shrink the vm_area?
> > +		 */
> 
> Kernel comment style is to keep the
> 
> 		/*
> 
> on a line of it's own.

I think it differs a bit throughout subsystems - will change.


> A realloc that doesn't shrink is a bit pointless.

Indeed - the idea was to mostly clean up kvrealloc() in this series first and
implement proper shrinking and growing in a subsequent one. Does that make
sense?

> The same heuristics as for krealloc should probably apply
> here, adjust to the fact that we always deal with whole pages.

Sounds reasonable, but are there any? In __do_krealloc() if ks > new_size we
only call into kasan_krealloc() to poison things and return the original
pointer. Do I miss anything?

> 
> 
> > +	/* TODO: Can we optimize and extend the existing allocation if we have
> > +	 * enough contiguous space left in the virtual address space?
> > +	 */
> 
> I don't understand this comment.

I meant to say that we could optimze by allocating additional pages and map them
at the end of the existing allocation, if there is enough space in the VA space.

If that doesn't work, we can still allocate additional pages and remap
everything.

> 
> > +EXPORT_SYMBOL(vrealloc_noprof);
> 
> New interfaces should be EXPORT_SYMBOL_GPL.  That being said for this
> to be added an exported we'll need an actual user to start with anyway.

Besides kvrealloc(), the Rust abstractions will be a user soon, but for that we
probably don't need the export either. I will remove it for now.

Re: [PATCH 1/2] mm: vmalloc: implement vrealloc()

[Uladzislau Rezki]

On Thu, Jul 18, 2024 at 12:24:01AM +0200, Danilo Krummrich wrote:
> Implement vrealloc() analogous to krealloc().
> 
> Currently, krealloc() requires the caller to pass the size of the
> previous memory allocation, which, instead, should be self-contained.
> 
> We attempt to fix this in a subsequent patch which, in order to do so,
> requires vrealloc().
> 
> Besides that, we need realloc() functions for kernel allocators in Rust
> too. With `Vec` or `KVec` respectively, potentially growing (and
> shrinking) data structures are rather common.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  include/linux/vmalloc.h |  4 +++
>  mm/vmalloc.c            | 58 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index e4a631ec430b..9ff0a8e5c323 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -189,6 +189,10 @@ extern void *__vcalloc_noprof(size_t n, size_t size, gfp_t flags) __alloc_size(1
>  extern void *vcalloc_noprof(size_t n, size_t size) __alloc_size(1, 2);
>  #define vcalloc(...)		alloc_hooks(vcalloc_noprof(__VA_ARGS__))
>  
> +extern void * __must_check vrealloc_noprof(const void *p, size_t size,
> +					   gfp_t flags) __realloc_size(2);
> +#define vrealloc(...)		alloc_hooks(vrealloc_noprof(__VA_ARGS__))
> +
>  extern void vfree(const void *addr);
>  extern void vfree_atomic(const void *addr);
>  
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index e34ea860153f..4ec949ac9d9d 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -4036,6 +4036,64 @@ void *vzalloc_node_noprof(unsigned long size, int node)
>  }
>  EXPORT_SYMBOL(vzalloc_node_noprof);
>  
> +/**
> + * vrealloc - reallocate virtually contiguous memory; contents remain unchanged
> + * @p: object to reallocate memory for
> + * @size: the size to reallocate
> + * @flags: the flags for the page level allocator
> + *
> + * The contents of the object pointed to are preserved up to the lesser of the
> + * new and old size (__GFP_ZERO flag is effectively ignored).
> + *
> + * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
> + * @p is not a %NULL pointer, the object pointed to is freed.
> + *
> + * Return: pointer to the allocated memory; %NULL if @size is zero or in case of
> + *         failure
> + */
> +void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
> +{
> +	size_t old_size = 0;
> +	void *n;
> +
> +	if (!size) {
> +		vfree(p);
> +		return NULL;
> +	}
> +
> +	if (p) {
> +		struct vm_struct *vm;
> +
> +		vm = find_vm_area(p);
>
Concurrent vfree() will lead to use-after-free. Either add a comment
that a user is responsible for not using vrealloc()/vfree() on the same
pointer concurrently or use find_unlink_vmap_area() which might be more
complex when it comes to design of the vrealloc().

--
Uladzislau Rezki

Re: [PATCH 1/2] mm: vmalloc: implement vrealloc()

[Christoph Hellwig]

On Tue, Jul 23, 2024 at 01:28:32PM +0200, Uladzislau Rezki wrote:
> Concurrent vfree() will lead to use-after-free. Either add a comment
> that a user is responsible for not using vrealloc()/vfree() on the same
> pointer concurrently or use find_unlink_vmap_area() which might be more
> complex when it comes to design of the vrealloc().

You can never use *free concurrently with *realloc.  I guess it doesn't
hurt to clearly document that, but other than that we should not try
to cater to that use pattern at all.

Re: [PATCH 1/2] mm: vmalloc: implement vrealloc()

[Uladzislau Rezki]

On Tue, Jul 23, 2024 at 06:44:56AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 23, 2024 at 01:28:32PM +0200, Uladzislau Rezki wrote:
> > Concurrent vfree() will lead to use-after-free. Either add a comment
> > that a user is responsible for not using vrealloc()/vfree() on the same
> > pointer concurrently or use find_unlink_vmap_area() which might be more
> > complex when it comes to design of the vrealloc().
> 
> You can never use *free concurrently with *realloc.  I guess it doesn't
> hurt to clearly document that, but other than that we should not try
> to cater to that use pattern at all.
>
Agree, i mentioned that as a first option. I think, it is enough to document that.

Thanks!

--
Uladzislau Rezki

[PATCH 2/2] mm: kvmalloc: align kvrealloc() with krealloc()

[Danilo Krummrich]

Besides the obvious (and desired) difference between krealloc() and
kvrealloc(), there is some inconsistency in their function signatures
and behavior:

 - krealloc() frees the memory when the requested size is zero, whereas
   kvrealloc() simply returns a pointer to the existing allocation.

 - krealloc() behaves like kmalloc() if a NULL pointer is passed, whereas
   kvrealloc() does not accept a NULL pointer at all and, if passed,
   would fault instead.

 - krealloc() is self-contained, whereas kvrealloc() relies on the caller
   to provide the size of the previous allocation.

Inconsistent behavior throughout allocation APIs is error prone, hence make
kvrealloc() behave like krealloc(), which seems superior in all mentioned
aspects.

Besides that, implementing kvrealloc() by making use of krealloc() and
vrealloc() provides oppertunities to grow (and shrink) allocations more
efficiently. For instance, vrealloc() could be optimized to extend the
existing allocation if there is enough contiguous space left in the
virtual address space at the end of the existing allocation.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 arch/powerpc/platforms/pseries/papr-vpd.c |  5 +-
 drivers/gpu/drm/drm_exec.c                |  3 +-
 fs/xfs/xfs_log_recover.c                  |  2 +-
 include/linux/slab.h                      |  3 +-
 kernel/resource.c                         |  3 +-
 lib/fortify_kunit.c                       |  3 +-
 mm/util.c                                 | 72 ++++++++++++++++-------
 7 files changed, 56 insertions(+), 35 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/papr-vpd.c b/arch/powerpc/platforms/pseries/papr-vpd.c
index c29e85db5f35..1574176e3ffc 100644
--- a/arch/powerpc/platforms/pseries/papr-vpd.c
+++ b/arch/powerpc/platforms/pseries/papr-vpd.c
@@ -156,10 +156,7 @@ static int vpd_blob_extend(struct vpd_blob *blob, const char *data, size_t len)
 	const char *old_ptr = blob->data;
 	char *new_ptr;
 
-	new_ptr = old_ptr ?
-		kvrealloc(old_ptr, old_len, new_len, GFP_KERNEL_ACCOUNT) :
-		kvmalloc(len, GFP_KERNEL_ACCOUNT);
-
+	new_ptr = kvrealloc(old_ptr, new_len, GFP_KERNEL_ACCOUNT);
 	if (!new_ptr)
 		return -ENOMEM;
 
diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
index 2da094bdf8a4..18e366cc4993 100644
--- a/drivers/gpu/drm/drm_exec.c
+++ b/drivers/gpu/drm/drm_exec.c
@@ -145,8 +145,7 @@ static int drm_exec_obj_locked(struct drm_exec *exec,
 		size_t size = exec->max_objects * sizeof(void *);
 		void *tmp;
 
-		tmp = kvrealloc(exec->objects, size, size + PAGE_SIZE,
-				GFP_KERNEL);
+		tmp = kvrealloc(exec->objects, size + PAGE_SIZE, GFP_KERNEL);
 		if (!tmp)
 			return -ENOMEM;
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 4fe627991e86..bbe5ecb2345b 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2132,7 +2132,7 @@ xlog_recover_add_to_cont_trans(
 	old_ptr = item->ri_buf[item->ri_cnt-1].i_addr;
 	old_len = item->ri_buf[item->ri_cnt-1].i_len;
 
-	ptr = kvrealloc(old_ptr, old_len, len + old_len, GFP_KERNEL);
+	ptr = kvrealloc(old_ptr, len + old_len, GFP_KERNEL);
 	if (!ptr)
 		return -ENOMEM;
 	memcpy(&ptr[old_len], dp, len);
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 7247e217e21b..41ede46d3bd2 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -808,8 +808,7 @@ kvmalloc_array_node_noprof(size_t n, size_t size, gfp_t flags, int node)
 #define kvcalloc_node(...)			alloc_hooks(kvcalloc_node_noprof(__VA_ARGS__))
 #define kvcalloc(...)				alloc_hooks(kvcalloc_noprof(__VA_ARGS__))
 
-extern void *kvrealloc_noprof(const void *p, size_t oldsize, size_t newsize, gfp_t flags)
-		      __realloc_size(3);
+extern void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags) __realloc_size(2);
 #define kvrealloc(...)				alloc_hooks(kvrealloc_noprof(__VA_ARGS__))
 
 extern void kvfree(const void *addr);
diff --git a/kernel/resource.c b/kernel/resource.c
index fcbca39dbc45..46e064c8dce2 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -458,8 +458,7 @@ int walk_system_ram_res_rev(u64 start, u64 end, void *arg,
 			/* re-alloc */
 			struct resource *rams_new;
 
-			rams_new = kvrealloc(rams, rams_size * sizeof(struct resource),
-					     (rams_size + 16) * sizeof(struct resource),
+			rams_new = kvrealloc(rams, (rams_size + 16) * sizeof(struct resource),
 					     GFP_KERNEL);
 			if (!rams_new)
 				goto out;
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
index 27ea8bf0252c..acbdc7855100 100644
--- a/lib/fortify_kunit.c
+++ b/lib/fortify_kunit.c
@@ -308,8 +308,7 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(vmalloc)
 	orig = kvmalloc(prev_size, gfp);				\
 	KUNIT_EXPECT_TRUE(test, orig != NULL);				\
 	checker(((expected_pages) * PAGE_SIZE) * 2,			\
-		kvrealloc(orig, prev_size,				\
-			  ((alloc_pages) * PAGE_SIZE) * 2, gfp),	\
+		kvrealloc(orig, ((alloc_pages) * PAGE_SIZE) * 2, gfp),	\
 		kvfree(p));						\
 } while (0)
 DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
diff --git a/mm/util.c b/mm/util.c
index 983baf2bd675..6a4eb9c1d9b7 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -598,6 +598,21 @@ unsigned long vm_mmap(struct file *file, unsigned long addr,
 }
 EXPORT_SYMBOL(vm_mmap);
 
+static gfp_t to_kmalloc_flags(gfp_t flags, size_t size)
+{
+	if (size > PAGE_SIZE) {
+		flags |= __GFP_NOWARN;
+
+		if (!(flags & __GFP_RETRY_MAYFAIL))
+			flags |= __GFP_NORETRY;
+
+		/* nofail semantic is implemented by the vmalloc fallback */
+		flags &= ~__GFP_NOFAIL;
+	}
+
+	return flags;
+}
+
 /**
  * kvmalloc_node - attempt to allocate physically contiguous memory, but upon
  * failure, fall back to non-contiguous (vmalloc) allocation.
@@ -616,7 +631,6 @@ EXPORT_SYMBOL(vm_mmap);
  */
 void *kvmalloc_node_noprof(size_t size, gfp_t flags, int node)
 {
-	gfp_t kmalloc_flags = flags;
 	void *ret;
 
 	/*
@@ -626,17 +640,7 @@ void *kvmalloc_node_noprof(size_t size, gfp_t flags, int node)
 	 * However make sure that larger requests are not too disruptive - no
 	 * OOM killer and no allocation failure warnings as we have a fallback.
 	 */
-	if (size > PAGE_SIZE) {
-		kmalloc_flags |= __GFP_NOWARN;
-
-		if (!(kmalloc_flags & __GFP_RETRY_MAYFAIL))
-			kmalloc_flags |= __GFP_NORETRY;
-
-		/* nofail semantic is implemented by the vmalloc fallback */
-		kmalloc_flags &= ~__GFP_NOFAIL;
-	}
-
-	ret = kmalloc_node_noprof(size, kmalloc_flags, node);
+	ret = kmalloc_node_noprof(size, to_kmalloc_flags(flags, size), node);
 
 	/*
 	 * It doesn't really make sense to fallback to vmalloc for sub page
@@ -704,18 +708,42 @@ void kvfree_sensitive(const void *addr, size_t len)
 }
 EXPORT_SYMBOL(kvfree_sensitive);
 
-void *kvrealloc_noprof(const void *p, size_t oldsize, size_t newsize, gfp_t flags)
+/**
+ * kvrealloc - reallocate memory; contents remain unchanged
+ * @p: object to reallocate memory for
+ * @size: the size to reallocate
+ * @flags: the flags for the page level allocator
+ *
+ * The contents of the object pointed to are preserved up to the lesser of the
+ * new and old size (__GFP_ZERO flag is effectively ignored).
+ *
+ * If @p is %NULL, kvrealloc() behaves exactly like kvmalloc(). If @size is 0
+ * and @p is not a %NULL pointer, the object pointed to is freed.
+ *
+ * Return: pointer to the allocated memory or %NULL in case of error
+ */
+void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
 {
-	void *newp;
+	void *n;
+
+	if (is_vmalloc_addr(p))
+		return vrealloc_noprof(p, size, flags);
+
+	n = krealloc_noprof(p, size, to_kmalloc_flags(flags, size));
+	if (!n) {
+		/* We failed to krealloc(), fall back to kvmalloc(). */
+		n = kvmalloc_noprof(size, flags);
+		if (!n)
+			return NULL;
+
+		if (p) {
+			/* We already know that `p` is not a vmalloc address. */
+			memcpy(n, p, ksize(p));
+			kfree(p);
+		}
+	}
 
-	if (oldsize >= newsize)
-		return (void *)p;
-	newp = kvmalloc_noprof(newsize, flags);
-	if (!newp)
-		return NULL;
-	memcpy(newp, p, oldsize);
-	kvfree(p);
-	return newp;
+	return n;
 }
 EXPORT_SYMBOL(kvrealloc_noprof);
 
-- 
2.45.2

Re: [PATCH 2/2] mm: kvmalloc: align kvrealloc() with krealloc()

[Christoph Hellwig]

> +extern void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags) __realloc_size(2);

Please drop the extern while you're at it and move the __realloc_size
attribute to a separate line.

> +static gfp_t to_kmalloc_flags(gfp_t flags, size_t size)
> +{
> +	if (size > PAGE_SIZE) {
> +		flags |= __GFP_NOWARN;
> +
> +		if (!(flags & __GFP_RETRY_MAYFAIL))
> +			flags |= __GFP_NORETRY;
> +
> +		/* nofail semantic is implemented by the vmalloc fallback */
> +		flags &= ~__GFP_NOFAIL;
> +	}
> +
> +	return flags;

The name for this function sounds a bit odd.  Maybe kmalloc_gfp_adjust
instead?  Also the comment explaining these flags tweaks should move
from the caller to this function.

Re: [PATCH 2/2] mm: kvmalloc: align kvrealloc() with krealloc()

[Danilo Krummrich]

On Wed, Jul 17, 2024 at 08:19:26PM -0700, Christoph Hellwig wrote:
> > +extern void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags) __realloc_size(2);
> 
> Please drop the extern while you're at it and move the __realloc_size
> attribute to a separate line.

Will do.

> 
> > +static gfp_t to_kmalloc_flags(gfp_t flags, size_t size)
> > +{
> > +	if (size > PAGE_SIZE) {
> > +		flags |= __GFP_NOWARN;
> > +
> > +		if (!(flags & __GFP_RETRY_MAYFAIL))
> > +			flags |= __GFP_NORETRY;
> > +
> > +		/* nofail semantic is implemented by the vmalloc fallback */
> > +		flags &= ~__GFP_NOFAIL;
> > +	}
> > +
> > +	return flags;
> 
> The name for this function sounds a bit odd.  Maybe kmalloc_gfp_adjust
> instead?  Also the comment explaining these flags tweaks should move
> from the caller to this function.

kmalloc_gfp_adjust() sounds good to me. I will rename it and move the comment
up.