From: Michal Hocko <mhocko@suse.com>
To: Vasily Averin <vvs@virtuozzo.com>
Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
Alexey Dobriyan <adobriyan@gmail.com>,
Shakeel Butt <shakeelb@google.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Vladimir Davydov <vdavydov.dev@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Dmitry Safonov <0x7f454c46@gmail.com>
Subject: Re: [PATCH 2/2] ipc: use kmalloc for msg_queue and shmid_kernel
Date: Mon, 26 Apr 2021 12:25:46 +0200 [thread overview]
Message-ID: <YIaVKi+0VMrz5LGD@dhcp22.suse.cz> (raw)
In-Reply-To: <b0845b85-f4fe-601d-3328-d707d7db27f5@virtuozzo.com>
On Mon 26-04-21 13:18:14, Vasily Averin wrote:
> msg_queue and shmid_kernel are quite small objects, no need to use
> kvmalloc for them.
Both of them are 256B on most 64b systems.
> Previously these objects was allocated via ipc_alloc/ipc_rcu_alloc(),
> common function for several ipc objects. It had kvmalloc call inside().
> Later, this function went away and was finally replaced by direct
> kvmalloc call, and now we can use more suitable kmalloc/kfree for them.
This describes the history of the code but it doesn't really explain why
kvmalloc is a bad decision here. I would go with the following:
"
Using kvmalloc for sub page size objects is suboptimal because kmalloc
can easily fallback into vmalloc under memory pressure and smaller
objects would fragment memory. Therefore replace kvmalloc by a simple
kmalloc.
"
>
> Reported-by: Alexey Dobriyan <adobriyan@gmail.com>
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
With a clarified changelog, feel free to add
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> ipc/msg.c | 6 +++---
> ipc/shm.c | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ipc/msg.c b/ipc/msg.c
> index 87898cb..79c6625 100644
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -130,7 +130,7 @@ static void msg_rcu_free(struct rcu_head *head)
> struct msg_queue *msq = container_of(p, struct msg_queue, q_perm);
>
> security_msg_queue_free(&msq->q_perm);
> - kvfree(msq);
> + kfree(msq);
> }
>
> /**
> @@ -147,7 +147,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
> key_t key = params->key;
> int msgflg = params->flg;
>
> - msq = kvmalloc(sizeof(*msq), GFP_KERNEL_ACCOUNT);
> + msq = kmalloc(sizeof(*msq), GFP_KERNEL_ACCOUNT);
> if (unlikely(!msq))
> return -ENOMEM;
>
> @@ -157,7 +157,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
> msq->q_perm.security = NULL;
> retval = security_msg_queue_alloc(&msq->q_perm);
> if (retval) {
> - kvfree(msq);
> + kfree(msq);
> return retval;
> }
>
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 7632d72..85da060 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -222,7 +222,7 @@ static void shm_rcu_free(struct rcu_head *head)
> struct shmid_kernel *shp = container_of(ptr, struct shmid_kernel,
> shm_perm);
> security_shm_free(&shp->shm_perm);
> - kvfree(shp);
> + kfree(shp);
> }
>
> static inline void shm_rmid(struct ipc_namespace *ns, struct shmid_kernel *s)
> @@ -619,7 +619,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> ns->shm_tot + numpages > ns->shm_ctlall)
> return -ENOSPC;
>
> - shp = kvmalloc(sizeof(*shp), GFP_KERNEL_ACCOUNT);
> + shp = kmalloc(sizeof(*shp), GFP_KERNEL_ACCOUNT);
> if (unlikely(!shp))
> return -ENOMEM;
>
> @@ -630,7 +630,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
> shp->shm_perm.security = NULL;
> error = security_shm_alloc(&shp->shm_perm);
> if (error) {
> - kvfree(shp);
> + kfree(shp);
> return error;
> }
>
> --
> 1.8.3.1
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2021-04-26 10:25 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <e67f2a95-4b01-9db2-fe47-0b2210f0b138@virtuozzo.com>
2021-04-26 10:18 ` [PATCH 0/2] ipc: allocations cleanup Vasily Averin
2021-04-28 7:35 ` [PATCH v2 " Vasily Averin
2021-04-28 7:35 ` [PATCH v2 1/2] ipc sem: use kvmalloc for sem_undo allocation Vasily Averin
2021-04-28 7:35 ` [PATCH v2 2/2] ipc: use kmalloc for msg_queue and shmid_kernel Vasily Averin
2021-04-26 10:18 ` [PATCH 1/2] ipc sem: use kvmalloc for sem_undo allocation Vasily Averin
2021-04-26 10:28 ` Michal Hocko
2021-04-26 16:22 ` Shakeel Butt
2021-04-26 20:29 ` Roman Gushchin
2021-04-26 10:18 ` [PATCH 2/2] ipc: use kmalloc for msg_queue and shmid_kernel Vasily Averin
2021-04-26 10:25 ` Michal Hocko [this message]
2021-04-28 5:15 ` Vasily Averin
2021-04-28 6:33 ` Michal Hocko
2021-04-26 16:23 ` Shakeel Butt
2021-04-26 20:29 ` Roman Gushchin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YIaVKi+0VMrz5LGD@dhcp22.suse.cz \
--to=mhocko@suse.com \
--cc=0x7f454c46@gmail.com \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=shakeelb@google.com \
--cc=vdavydov.dev@gmail.com \
--cc=vvs@virtuozzo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).