Re: [PATCH] pid: Do not set pid_max in new pid namespaces

[Alexander Mikhalitsyn]

Am Mi., 5. März 2025 um 15:59 Uhr schrieb Michal Koutný <mkoutny@xxxxxxxx>:
>
> It is already difficult for users to troubleshoot which of multiple pid
> limits restricts their workload. The per-(hierarchical-)NS pid_max would
> contribute to the confusion.
> Also, the implementation copies the limit upon creation from
> parent, this pattern showed cumbersome with some attributes in legacy
> cgroup controllers -- it's subject to race condition between parent's
> limit modification and children creation and once copied it must be
> changed in the descendant.
>
> Let's do what other places do (ucounts or cgroup limits) -- create new
> pid namespaces without any limit at all. The global limit (actually any
> ancestor's limit) is still effectively in place, we avoid the
> set/unshare race and bumps of global (ancestral) limit have the desired
> effect on pid namespace that do not care.
>
> Link: https://lore.kernel.org/r/20240408145819.8787-1-mkoutny@xxxxxxxx/
> Link: https://lore.kernel.org/r/20250221170249.890014-1-mkoutny@xxxxxxxx/
> Fixes: 7863dcc72d0f4 ("pid: allow pid_max to be set per pid namespace")
> Signed-off-by: Michal Koutný <mkoutny@xxxxxxxx>

Dear Michal,

This completely makes sense and I tend to agree. But we also need to
ensure that the kselftest for pid_max is not broken with this change.
Let me play with this stuff a bit and I get back with "Tested-by" ;-)

Kind regards,
Alex

> ---
>  kernel/pid_namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 8f6cfec87555a..7098ed44e717d 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -107,7 +107,7 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns
>                 goto out_free_idr;
>         ns->ns.ops = &pidns_operations;
>
> -       ns->pid_max = parent_pid_ns->pid_max;
> +       ns->pid_max = PID_MAX_LIMIT;
>         err = register_pidns_sysctls(ns);
>         if (err)
>                 goto out_free_inum;
>
> base-commit: 48a5eed9ad584315c30ed35204510536235ce402
> --
> 2.48.1
>