From nobody Tue Dec 2 01:06:34 2025 Received: from mail-ej1-f41.google.com (mail-ej1-f41.google.com [209.85.218.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D11AE2749DC for ; Sun, 23 Nov 2025 06:31:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763879475; cv=none; b=QceDJql/d+hAPT7zwEj3upNkk+e4faah1AoVG/nSJS+Z5sTOfWT4ifz69yH3/tVrXot56WglK9iqaJXA3jPld7ZJN29DOAYhPawAzx/hFHC/PdNfRdK7CSc0FOswK30aY+9mJT3riiyQjplnSLP+FFq7RS5P1Onu/QXHUMzRl2Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763879475; c=relaxed/simple; bh=D0Aild/73jmKg7ZMSpkPXvJNgfSF2aMMJG78JV64k5U=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=U0g0j/vxoT7AUBPrM38U4fgant+Ab+yDh9M7FmL4TU3/dNDhWXbZpe0AhNp3A9ZpLDFLe1YvA/rqdvluCVWLy7WBhhgAVU+upQGN1jD9fcOyxkJGpUJ/5XXBbp8eDFu43hNGI6KiWRHHbdrIU2auiCII3OtuKfwz4FETCRBkPdA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=WNDjJZQR; arc=none smtp.client-ip=209.85.218.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="WNDjJZQR" Received: by mail-ej1-f41.google.com with SMTP id a640c23a62f3a-b762de65c07so41324766b.2 for ; Sat, 22 Nov 2025 22:31:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763879472; x=1764484272; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=fzeVFSUzIu035ZBi7H1TZb6fE+64VF6oJ8fhy8LFjfw=; b=WNDjJZQRmXNcd9TlV5CCSx9VH5uxzU4ux+SxdYrFb9vzE8XdwnIGgYmz5gfExerRBE rYTj6JJ/l4YymlKjIVBJguPokgOK3U+dzmOjObtgR/4Q+xRrHH9pR+4j0374o7bN/zAU TPswPySL2zP5g4XQiVkP9jO04llWnd35UV7H02+Sadt+7qs+a2//QXkRB9lgP0HlcLwh FolX2ahMWpGGGtPG/YDc05EJoZO5dJS4sEViA4A3/klnI+Z53/kDwYoAuxXTOmtW8k3+ HU2H5V1u1PvF1tIZD3s1W/qR3UKptHoetLV5QQgn0cZRQh0P+MeVqgqbY05y34tBdoZU PyUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763879472; x=1764484272; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=fzeVFSUzIu035ZBi7H1TZb6fE+64VF6oJ8fhy8LFjfw=; b=UWMECkMjCVJCJPpIXhXiPbafkzqSrGcWUyIRgz59bQ0g6mTYgnYXE+wEKg82iD+nLg Nandg9aphpFjMcZI8ozk71fXck2gYePM8PFAAVQe4uIsNd93FuSlqhuKxtSx8SaF+v+0 BbsABQjS4Xm5pzRHpuEDmAtK2LNvZQ6AS3RGvDFfPEhTs+bxhHN2uVTN5ytlphA8e7jK Roai+uwdkeHjalpGxnoUkoqYT0eDkl0N/AFpqv/ZNsb4z7V2epU2RgscyBJk/TzdihWt 5X0skYKGH8pvAMGTt3rSRyfXBI6cab8MCvY75MGoa7HLQcFz5vhTpE1i4dmGzCYG/Otx tOww== X-Forwarded-Encrypted: i=1; AJvYcCVTsCrAh+dM6cizjhuHuArKyPopZ+84RlnCswixfiNVf4aUY0FXrKmps5vl69x8wPuJzjIgT7Rwz+D0LbQ=@vger.kernel.org X-Gm-Message-State: AOJu0YxnOlodkNMDrU/J3rgPxxNr9tVaTfBS6xcke1MMGsSUcO8bCmCh VHDx8BXOGC1OuPRGF1nO2FpzCeO+/lSL+1oKVBnXxhFvpgafmdtpJHp4 X-Gm-Gg: ASbGnctAU5FNZ6Fg06DLadMjrIkXiEANuzLovm4nWqZP2grkq1xTP1npJkJKpA0dsUX TZk9gYBY97UX3hKn3l+DT+XNniszkj7A4ST/M7M8oCDH19bkzMtZGVo6JhePabqfMhIHixwpxi9 1uWYN1MxI5/fMhNgxgFiGaYfituShfOgZ3TNHBQzJqkRiWoZOi8sr0n1idNeL+FKijPWI34ca8n jpjSP3Fout9XgTaMXFrlQbHOUgpK6ghjGdV0TLrdUilNBSdIe46neyHOfYAiPp+L6dGwQ64KfZh jHiGxo4zhFzvy7oweLpuRK9FnkrgbESVznY+gttHlcPwjf3/y+XCOcx2eT1Z6fdqud5vUQJliJe yuvLDEGERdi1iYhbKdkHS3jfkIlQax72JDWODybWyxHcCdNhFKf1nM3G9qRcz9JcY+U7IXdTXgM EuyzGbhKhOOKy5MKrQuu6zbECJGm2WiOFo/qx1bTv7/hUA2MfmqeNSQ7flj5c= X-Google-Smtp-Source: AGHT+IG3sZ0KYmhLtFy9FEoE/ix2RTytRA6o14m8ny/qtVDn43naSDP8Y8a8Os47AAL1eliyOZzjSw== X-Received: by 2002:a17:907:940b:b0:b73:806c:bab7 with SMTP id a640c23a62f3a-b76715723ffmr788343166b.19.1763879472026; Sat, 22 Nov 2025 22:31:12 -0800 (PST) Received: from f.. (cst-prg-14-82.cust.vodafone.cz. [46.135.14.82]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-b7654cf0435sm929432866b.4.2025.11.22.22.31.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 22 Nov 2025 22:31:11 -0800 (PST) From: Mateusz Guzik To: oleg@redhat.com Cc: brauner@kernel.org, linux-kernel@vger.kernel.org, akpm@linux-foundation.org, linux-mm@kvack.org, Mateusz Guzik Subject: [PATCH 3/3] pid: only take pidmap_lock once on alloc Date: Sun, 23 Nov 2025 07:30:54 +0100 Message-ID: <20251123063054.3502938-4-mjguzik@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20251123063054.3502938-1-mjguzik@gmail.com> References: <20251123063054.3502938-1-mjguzik@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" This reduces contention on the lock during parallel clone/exit. It remains the primary bottleneck in such a case. While here tidy up the code. Signed-off-by: Mateusz Guzik --- kernel/pid.c | 99 +++++++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 44 deletions(-) diff --git a/kernel/pid.c b/kernel/pid.c index a31771bc89c1..6a87ce5a6040 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -159,9 +159,11 @@ void free_pids(struct pid **pids) free_pid(pids[tmp]); } =20 -struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid, - size_t set_tid_size) +struct pid *alloc_pid(struct pid_namespace *ns, pid_t *arg_set_tid, + size_t arg_set_tid_size) { + int set_tid[MAX_PID_NS_LEVEL + 1] =3D {}; + int pid_max[MAX_PID_NS_LEVEL + 1] =3D {}; struct pid *pid; enum pid_type type; int i, nr; @@ -170,47 +172,71 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t= *set_tid, int retval =3D -ENOMEM; =20 /* - * set_tid_size contains the size of the set_tid array. Starting at + * arg_set_tid_size contains the size of the arg_set_tid array. Starting = at * the most nested currently active PID namespace it tells alloc_pid() * which PID to set for a process in that most nested PID namespace - * up to set_tid_size PID namespaces. It does not have to set the PID - * for a process in all nested PID namespaces but set_tid_size must + * up to arg_set_tid_size PID namespaces. It does not have to set the PID + * for a process in all nested PID namespaces but arg_set_tid_size must * never be greater than the current ns->level + 1. */ - if (set_tid_size > ns->level + 1) + if (arg_set_tid_size > ns->level + 1) return ERR_PTR(-EINVAL); =20 + /* + * Prep before we take locks: + * + * 1. allocate and fill in pid struct + */ pid =3D kmem_cache_alloc(ns->pid_cachep, GFP_KERNEL); if (!pid) return ERR_PTR(retval); =20 - tmp =3D ns; + get_pid_ns(ns); pid->level =3D ns->level; + refcount_set(&pid->count, 1); + spin_lock_init(&pid->lock); + for (type =3D 0; type < PIDTYPE_MAX; ++type) + INIT_HLIST_HEAD(&pid->tasks[type]); + init_waitqueue_head(&pid->wait_pidfd); + INIT_HLIST_HEAD(&pid->inodes); =20 - for (i =3D ns->level; i >=3D 0; i--) { - int tid =3D 0; - int pid_max =3D READ_ONCE(tmp->pid_max); + /* + * 2. perm check checkpoint_restore_ns_capable() + * + * This stores found pid_max to make sure the used value is the same shou= ld + * later code need it. + */ + for (tmp =3D ns, i =3D ns->level; i >=3D 0; i--) { + pid_max[ns->level - i] =3D READ_ONCE(tmp->pid_max); =20 - if (set_tid_size) { - tid =3D set_tid[ns->level - i]; + if (arg_set_tid_size) { + int tid =3D set_tid[ns->level - i] =3D arg_set_tid[ns->level - i]; =20 retval =3D -EINVAL; - if (tid < 1 || tid >=3D pid_max) - goto out_free; + if (tid < 1 || tid >=3D pid_max[ns->level - i]) + goto out_abort; /* * Also fail if a PID !=3D 1 is requested and * no PID 1 exists. */ if (tid !=3D 1 && !tmp->child_reaper) - goto out_free; + goto out_abort; retval =3D -EPERM; if (!checkpoint_restore_ns_capable(tmp->user_ns)) - goto out_free; - set_tid_size--; + goto out_abort; + arg_set_tid_size--; } =20 - idr_preload(GFP_KERNEL); - spin_lock(&pidmap_lock); + tmp =3D tmp->parent; + } + + /* + * Prep is done, id allocation goes here: + */ + idr_preload_many(ns->level + 1, GFP_KERNEL); + spin_lock(&pidmap_lock); + for (tmp =3D ns, i =3D ns->level; i >=3D 0; i--) { + int tid =3D set_tid[ns->level - i]; =20 if (tid) { nr =3D idr_alloc(&tmp->idr, NULL, tid, @@ -235,10 +261,8 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t = *set_tid, * a partially initialized PID (see below). */ nr =3D idr_alloc_cyclic(&tmp->idr, NULL, pid_min, - pid_max, GFP_ATOMIC); + pid_max[ns->level - i], GFP_ATOMIC); } - spin_unlock(&pidmap_lock); - idr_preload_end(); =20 if (nr < 0) { retval =3D (nr =3D=3D -ENOSPC) ? -EAGAIN : nr; @@ -257,25 +281,15 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t= *set_tid, * is what we have exposed to userspace for a long time and it is * documented behavior for pid namespaces. So we can't easily * change it even if there were an error code better suited. + * + * This can't be done earlier because we need to preserve other + * error conditions. */ retval =3D -ENOMEM; - - get_pid_ns(ns); - refcount_set(&pid->count, 1); - spin_lock_init(&pid->lock); - for (type =3D 0; type < PIDTYPE_MAX; ++type) - INIT_HLIST_HEAD(&pid->tasks[type]); - - init_waitqueue_head(&pid->wait_pidfd); - INIT_HLIST_HEAD(&pid->inodes); - - upid =3D pid->numbers + ns->level; - idr_preload(GFP_KERNEL); - spin_lock(&pidmap_lock); - if (!(ns->pid_allocated & PIDNS_ADDING)) - goto out_unlock; + if (unlikely(!(ns->pid_allocated & PIDNS_ADDING))) + goto out_free; pidfs_add_pid(pid); - for ( ; upid >=3D pid->numbers; --upid) { + for (upid =3D pid->numbers + ns->level; upid >=3D pid->numbers; --upid) { /* Make the PID visible to find_pid_ns. */ idr_replace(&upid->ns->idr, pid, upid->nr); upid->ns->pid_allocated++; @@ -286,13 +300,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t = *set_tid, =20 return pid; =20 -out_unlock: - spin_unlock(&pidmap_lock); - idr_preload_end(); - put_pid_ns(ns); - out_free: - spin_lock(&pidmap_lock); while (++i <=3D ns->level) { upid =3D pid->numbers + i; idr_remove(&upid->ns->idr, upid->nr); @@ -303,7 +311,10 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t = *set_tid, idr_set_cursor(&ns->idr, 0); =20 spin_unlock(&pidmap_lock); + idr_preload_end(); =20 +out_abort: + put_pid_ns(ns); kmem_cache_free(ns->pid_cachep, pid); return ERR_PTR(retval); } --=20 2.48.1