pid: cosmetic changes to alloc_pid()
PATCH RESEND 1/1 pid: cosmetic changes to alloc_pid()
Commit 6d864a1b182532e7 ("pid: only take pidmap_lock once on alloc") landed v2 of the patch instead of v3. This patch remedies the problem.
No functional changes.
Signed-off-by Mateusz Guzik <[email protected]>
---
Hi Andrew,
this is an old & uncontroversial patch which fell through the cracks. Would you mind picking it up?
kernel/pid.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)
kernel/pid.c +10 -11
@@ -177,7 +177,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid,* for a process in all nested PID namespaces but arg_set_tid_size must * never be greater than the current ns->level + 1. */ -if (arg_set_tid_size > ns->level +1 ) +if (unlikely (arg_set_tid_size > ns->level +1 ))return ERR_PTR (-EINVAL );/*
@@ -186,7 +186,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid,* 1. allocate and fill in pid struct */ pid =kmem_cache_alloc (ns->pid_cachep,GFP_KERNEL ); -if (!pid) +if (unlikely (!pid))return ERR_PTR (retval);get_pid_ns (ns);
@@ -205,7 +205,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid,* This stores found pid_max to make sure the used value is the same should * later code need it. */ -for (tmp = ns, i = ns->level; i >=0 ; i--) { +for (tmp = ns, i = ns->level; i >=0 ;) { pid_max[ns->level - i] =READ_ONCE (tmp->pid_max);if (arg_set_tid_size) {
@@ -221,6 +221,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid, } tmp = tmp->parent; + i--; }/*
@@ -241,10 +242,9 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid, tid + 1, GFP_ATOMIC);/* * If ENOSPC is returned it means that the PID is -* alreay in use. Return EEXIST in that case. +* already in use. Return EEXIST in that case. */ if (nr == -ENOSPC ) - nr = -EEXIST ; }else {int pid_min =1 ;
@@ -270,12 +270,11 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid,* Preload more memory if idr_alloc{,cyclic} failed with -ENOMEM. * * The IDR API only allows us to preload memory for one call, while we may end -* up doing several under pidmap_lock with GFP_ATOMIC. The situation may be -* salvageable with GFP_KERNEL. But make sure to not loop indefinitely if preload -* did not help (the routine unfortunately returns void, so we have no idea -* if it got anywhere). +* up doing several with GFP_ATOMIC. It may be the situation is salvageable with +* GFP_KERNEL. But make sure to not loop indefinitely if preload did not help +* (the routine unfortunately returns void, so we have no idea if it got anywhere). * -* The lock can be safely dropped and picked up as historically pid allocation +* The pidmap lock can be safely dropped and picked up as historically pid allocation * for different namespaces was *not* atomic -- we try to hold on to it the * entire time only for performance reasons. */
@@ -299,7 +298,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid,/* * PID 1 (init) must be created first. */ -if (!READ_ONCE (tmp->child_reaper) && nr !=1 ) { +if (unlikely (!READ_ONCE (tmp->child_reaper) && nr !=1 )) { retval = -EINVAL ; goto out_free; }
Signature · 1 line
2.48.1
Andrew Morton
maintainerQuoted 4 lines · On Thu, 16 Jul 2026 16:11:30 +0200 Mateusz Guzik <[email protected]> wrote:
Commit
6d864a1b182532e7("pid: only takepidmap_lockonce on alloc") landed v2 of the patch instead of v3. This patch remedies the problem.No functional changes.
Oh.
Quoted 3 lines
this is an old & uncontroversial patch which fell through the cracks. Would you mind picking it up?
Please prepare a changelog for this patch then resend with a cc to Oleg and Christian?