Skip to content

Commit

Permalink
mm: kvmalloc: align kvrealloc() with krealloc()
Browse files Browse the repository at this point in the history
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() can be optimized to allocate and
map additional pages to grow the allocation or unmap and free unused pages
to shrink the allocation.

[[email protected]: document concurrency restrictions]
  Link: https://lkml.kernel.org/r/[email protected]
[[email protected]: disable KASAN when switching to vmalloc]
  Link: https://lkml.kernel.org/r/[email protected]
[[email protected]: properly document __GFP_ZERO behavior]
  Link: https://lkml.kernel.org/r/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Danilo Krummrich <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
Cc: Chandan Babu R <[email protected]>
Cc: Christian König <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Hyeonggon Yoo <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Cc: Oliver Upton <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: Roman Gushchin <[email protected]>
Cc: Uladzislau Rezki <[email protected]>
Cc: Wedson Almeida Filho <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
  • Loading branch information
Danilo Krummrich authored and akpm00 committed Sep 2, 2024
1 parent 3ddc2fe commit 590b9d5
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 44 deletions.
1 change: 0 additions & 1 deletion arch/arm64/kvm/nested.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ int kvm_vcpu_init_nested(struct kvm_vcpu *vcpu)
*/
num_mmus = atomic_read(&kvm->online_vcpus) * S2_MMU_PER_VCPU;
tmp = kvrealloc(kvm->arch.nested_mmus,
size_mul(sizeof(*kvm->arch.nested_mmus), kvm->arch.nested_mmus_size),
size_mul(sizeof(*kvm->arch.nested_mmus), num_mmus),
GFP_KERNEL_ACCOUNT | __GFP_ZERO);
if (!tmp)
Expand Down
5 changes: 1 addition & 4 deletions arch/powerpc/platforms/pseries/papr-vpd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 1 addition & 2 deletions drivers/gpu/drm/drm_exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion fs/xfs/xfs_log_recover.c
Original file line number Diff line number Diff line change
Expand Up @@ -2128,7 +2128,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);
Expand Down
4 changes: 2 additions & 2 deletions include/linux/slab.h
Original file line number Diff line number Diff line change
Expand Up @@ -841,8 +841,8 @@ 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);
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);
Expand Down
3 changes: 1 addition & 2 deletions kernel/resource.c
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,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;
Expand Down
3 changes: 1 addition & 2 deletions lib/fortify_kunit.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,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)
Expand Down
100 changes: 70 additions & 30 deletions mm/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,28 @@ unsigned long vm_mmap(struct file *file, unsigned long addr,
}
EXPORT_SYMBOL(vm_mmap);

static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size)
{
/*
* We want to attempt a large physically contiguous block first because
* it is less likely to fragment multiple larger blocks and therefore
* contribute to a long term fragmentation less than vmalloc fallback.
* 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) {
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.
Expand All @@ -627,32 +649,15 @@ EXPORT_SYMBOL(vm_mmap);
*/
void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node)
{
gfp_t kmalloc_flags = flags;
void *ret;

/*
* We want to attempt a large physically contiguous block first because
* it is less likely to fragment multiple larger blocks and therefore
* contribute to a long term fragmentation less than vmalloc fallback.
* 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(PASS_BUCKET_PARAMS(size, b), kmalloc_flags, node);

/*
* It doesn't really make sense to fallback to vmalloc for sub page
* requests
*/
ret = __kmalloc_node_noprof(PASS_BUCKET_PARAMS(size, b),
kmalloc_gfp_adjust(flags, size),
node);
if (ret || size <= PAGE_SIZE)
return ret;

Expand Down Expand Up @@ -715,18 +720,53 @@ 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
*
* 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.
*
* If __GFP_ZERO logic is requested, callers must ensure that, starting with the
* initial memory allocation, every subsequent call to this API for the same
* memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
* __GFP_ZERO is not fully honored by this API.
*
* In any case, the contents of the object pointed to are preserved up to the
* lesser of the new and old sizes.
*
* This function must not be called concurrently with itself or kvfree() for the
* same memory allocation.
*
* 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 (oldsize >= newsize)
return (void *)p;
newp = kvmalloc_noprof(newsize, flags);
if (!newp)
return NULL;
memcpy(newp, p, oldsize);
kvfree(p);
return newp;
if (is_vmalloc_addr(p))
return vrealloc_noprof(p, size, flags);

n = krealloc_noprof(p, size, kmalloc_gfp_adjust(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. */
kasan_disable_current();
memcpy(n, kasan_reset_tag(p), ksize(p));
kasan_enable_current();

kfree(p);
}
}

return n;
}
EXPORT_SYMBOL(kvrealloc_noprof);

Expand Down

0 comments on commit 590b9d5

Please sign in to comment.