2019-09-09 12:49:58

by Ivan Safonov

Subject: [PATCH] cgroup: use kv(malloc|free) instead of pidlist_(allocate|free)

Resolve TODO:
> The following two functions "fix" the issue where there are more pids
> than kmalloc will give memory for; in such cases, we use vmalloc/vfree.
> TODO: replace with a kernel-wide solution to this problem

kv(malloc|free) is appropriate replacement for pidlist_(allocate|free).

Signed-off-by: Ivan Safonov <[email protected]>
---
kernel/cgroup/cgroup-v1.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 88006be40ea3..1f25f35af2c4 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -193,25 +193,6 @@ struct cgroup_pidlist {
struct delayed_work destroy_dwork;
};

-/*
- * The following two functions "fix" the issue where there are more pids
- * than kmalloc will give memory for; in such cases, we use vmalloc/vfree.
- * TODO: replace with a kernel-wide solution to this problem
- */
-#define PIDLIST_TOO_LARGE(c) ((c) * sizeof(pid_t) > (PAGE_SIZE * 2))
-static void *pidlist_allocate(int count)
-{
- if (PIDLIST_TOO_LARGE(count))
- return vmalloc(array_size(count, sizeof(pid_t)));
- else
- return kmalloc_array(count, sizeof(pid_t), GFP_KERNEL);
-}
-
-static void pidlist_free(void *p)
-{
- kvfree(p);
-}
-
/*
* Used to destroy all pidlists lingering waiting for destroy timer. None
* should be left afterwards.
@@ -244,7 +225,7 @@ static void cgroup_pidlist_destroy_work_fn(struct work_struct *work)
*/
if (!delayed_work_pending(dwork)) {
list_del(&l->links);
- pidlist_free(l->list);
+ kvfree(l->list);
put_pid_ns(l->key.ns);
tofree = l;
}
@@ -365,7 +346,7 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,
* show up until sometime later on.
*/
length = cgroup_task_count(cgrp);
- array = pidlist_allocate(length);
+ array = kvmalloc(array_size(length, sizeof(pid_t)), GFP_KERNEL);
if (!array)
return -ENOMEM;
/* now, populate the array */
@@ -390,12 +371,12 @@ static int pidlist_array_load(struct cgroup *cgrp, enum cgroup_filetype type,

l = cgroup_pidlist_find_create(cgrp, type);
if (!l) {
- pidlist_free(array);
+ kvfree(array);
return -ENOMEM;
}

/* store array, freeing old if necessary */
- pidlist_free(l->list);
+ kvfree(l->list);
l->list = array;
l->length = length;
*lp = l;
--
2.21.0

2019-09-10 08:56:02

by Roman Gushchin

Subject: Re: [PATCH] cgroup: use kv(malloc|free) instead of pidlist_(allocate|free)

On Sun, Sep 08, 2019 at 05:40:41PM +0300, Ivan Safonov wrote:
> Resolve TODO:
> > The following two functions "fix" the issue where there are more pids
> > than kmalloc will give memory for; in such cases, we use vmalloc/vfree.
> > TODO: replace with a kernel-wide solution to this problem
>
> kv(malloc|free) is appropriate replacement for pidlist_(allocate|free).
>
> Signed-off-by: Ivan Safonov <[email protected]>

Looks good to me!

Acked-by: Roman Gushchin <[email protected]>

Thanks!

2019-09-10 19:21:39

by Tejun Heo

Subject: Re: [PATCH] cgroup: use kv(malloc|free) instead of pidlist_(allocate|free)

On Sun, Sep 08, 2019 at 05:40:41PM +0300, Ivan Safonov wrote:
> Resolve TODO:
> > The following two functions "fix" the issue where there are more pids
> > than kmalloc will give memory for; in such cases, we use vmalloc/vfree.
> > TODO: replace with a kernel-wide solution to this problem
>
> kv(malloc|free) is appropriate replacement for pidlist_(allocate|free).
>
> Signed-off-by: Ivan Safonov <[email protected]>

An equivalent patch is already queued.

https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git/commit/?h=for-5.4&id=653a23ca7e1e1ae907654e91d028bc6dfc7fce0c

Thanks.

--
tejun