[Devel] [PATCH rh9] pid_ns: More fixes for pid_max virtualization

[Pavel Tikhomirov]

On 31.05.2022 16:37, Konstantin Khorenko wrote:
>   * dropped reappeared global "pid_max" variable
>   * fixed usage of global "pid_max" variable
>   * introduced "pid_max_default" to be used on pid_ns creation instead of
>     the hardcoded value. Should be useful on Nodes with many CPUs.
>   * changed tracing code to use pid_max from init_pid_ns
> 
> https://jira.sw.ru/browse/PSBM-140308
> Fixes: f6a7abc88764 ("pid_ns: Virtualize pid_max")
> 
> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
> ---
>   include/linux/pid.h    |  2 +-
>   kernel/pid.c           | 11 +++++++----
>   kernel/pid_namespace.c |  2 +-
>   kernel/trace/trace.c   |  4 ++--
>   kernel/trace/trace.h   |  2 +-
>   5 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/pid.h b/include/linux/pid.h
> index 518fcfccb1ed..894d6ac4548b 100644
> --- a/include/linux/pid.h
> +++ b/include/linux/pid.h
> @@ -110,7 +110,7 @@ extern void transfer_pid(struct task_struct *old, struct task_struct *new,
>   struct pid_namespace;
>   extern struct pid_namespace init_pid_ns;
>   
> -extern int pid_max;
> +extern int pid_max_default;
>   extern int pid_max_min, pid_max_max;
>   
>   /*
> diff --git a/kernel/pid.c b/kernel/pid.c
> index ebe115ce2a34..a8aaeeb76584 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
> @@ -59,7 +59,7 @@ struct pid init_struct_pid = {
>   	}, }
>   };
>   
> -int pid_max = PID_MAX_DEFAULT;
> +int pid_max_default = PID_MAX_NS_DEFAULT;

Can we just drop PID_MAX_NS_DEFAULT? Is it really that required to 
redefine PID_MAX_DEFAULT and use here instead of original one?

The pid_max_default looks like a good thing to add, maybe even call it 
pid_max_ns_default? =)

>   
>   #define RESERVED_PIDS		300
>   
> @@ -192,7 +192,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
>   			tid = set_tid[ns->level - i];
>   
>   			retval = -EINVAL;
> -			if (tid < 1 || tid >= pid_max)
> +			if (tid < 1 || tid >= tmp->pid_max)
>   				goto out_free;
>   			/*
>   			 * Also fail if a PID != 1 is requested and
> @@ -624,11 +624,14 @@ void __init pid_idr_init(void)
>   	BUILD_BUG_ON(PID_MAX_LIMIT >= PIDNS_ADDING);
>   
>   	/* bump default and minimum pid_max based on number of cpus */
> -	pid_max = min(pid_max_max, max_t(int, pid_max,
> +	pid_max_default = min(pid_max_max, max_t(int, pid_max_default,
>   				PIDS_PER_CPU_DEFAULT * num_possible_cpus()));
> +	init_pid_ns.pid_max = max_t(int, init_pid_ns.pid_max, pid_max_default);

Do we still need "pid_ns: Forgotten initialization of pid_max" after 
this fixup? Don't we won't to avoid double initialization?

> +
>   	pid_max_min = max_t(int, pid_max_min,
>   				PIDS_PER_CPU_MIN * num_possible_cpus());
> -	pr_info("pid_max: default: %u minimum: %u\n", pid_max, pid_max_min);
> +	pr_info("pid_max: default: %u minimum: %u\n",
> +		pid_max_default, pid_max_min);
>   
>   	idr_init(&init_pid_ns.idr);
>   
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index 450d613ba8d5..15477b7f9869 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -110,7 +110,7 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns
>   	ns->user_ns = get_user_ns(user_ns);
>   	ns->ucounts = ucounts;
>   	ns->pid_allocated = PIDNS_ADDING;
> -	ns->pid_max = PID_MAX_NS_DEFAULT;
> +	ns->pid_max = pid_max_default;
>   
>   	return ns;
>   
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 924d08a5538b..23208498b523 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -705,7 +705,7 @@ int trace_pid_write(struct trace_pid_list *filtered_pids,
>   		return -ENOMEM;
>   	}
>   
> -	pid_list->pid_max = READ_ONCE(pid_max);
> +	pid_list->pid_max = init_pid_ns.pid_max;
>   
>   	/* Only truncating will shrink pid_max */
>   	if (filtered_pids && filtered_pids->pid_max > pid_list->pid_max)
> @@ -5288,7 +5288,7 @@ int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled)
>   
>   	if (mask == TRACE_ITER_RECORD_TGID) {
>   		if (!tgid_map) {
> -			tgid_map_max = pid_max;
> +			tgid_map_max = init_pid_ns.pid_max;
>   			map = kvcalloc(tgid_map_max + 1, sizeof(*tgid_map),
>   				       GFP_KERNEL);
>   
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 4a0e693000c6..30ab4375f7cb 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -648,7 +648,7 @@ extern unsigned long tracing_thresh;
>   
>   /* PID filtering */
>   
> -extern int pid_max;
> +extern struct pid_namespace init_pid_ns;
>   
>   bool trace_find_filtered_pid(struct trace_pid_list *filtered_pids,
>   			     pid_t search_pid);

-- 
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.