2015-04-01 07:25:03

by Yunlong Song

[permalink] [raw]
Subject: Re: [PATCH 3/9] perf sched replay: Alloc the memory of pid_to_task dynamically to adapt to the unexpected change of pid_max

On 2015/3/31 22:32, David Ahern wrote:
> On 3/31/15 7:46 AM, Yunlong Song wrote:
>> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
>> index c466104..20d887b 100644
>> --- a/tools/perf/builtin-sched.c
>> +++ b/tools/perf/builtin-sched.c
>> @@ -23,6 +23,7 @@
>> #include <semaphore.h>
>> #include <pthread.h>
>> #include <math.h>
>> +#include <api/fs/fs.h>
>>
>> #define PR_SET_NAME 15 /* Set process name */
>> #define MAX_CPUS 4096
>> @@ -124,7 +125,7 @@ struct perf_sched {
>> struct perf_tool tool;
>> const char *sort_order;
>> unsigned long nr_tasks;
>> - struct task_desc *pid_to_task[MAX_PID];
>> + struct task_desc **pid_to_task;
>> struct task_desc **tasks;
>> const struct trace_sched_handler *tp_handler;
>> pthread_mutex_t start_work_mutex;
>> @@ -326,8 +327,14 @@ static struct task_desc *register_pid(struct perf_sched *sched,
>> unsigned long pid, const char *comm)
>> {
>> struct task_desc *task;
>> + static int pid_max;
>>
>> - BUG_ON(pid >= MAX_PID);
>> + if (sched->pid_to_task == NULL) {
>> + if (sysctl__read_int("kernel/pid_max", &pid_max) < 0)
>> + pid_max = MAX_PID;
>> + BUG_ON((sched->pid_to_task = calloc(pid_max, sizeof(struct task_desc *))) == NULL);
>> + }
>> + BUG_ON(pid >= (unsigned long)pid_max);
>>
>
> so why the previous patch bumping the MAX_PID count if you move to dynamic here? And shouldn't MAX_PID get dropped here as well?
>
> So attached is what i put together last week; just have not had time to send it out.

MAX_PID in the previous patch can handle the case when sysctl__read_int("kernel/pid_max", &pid_max) < 0
here in this patch, so I still keep it. However, maybe sysctl__read_int is very unlikely to fail and does
not need MAX_PID at all, but keeping MAX_PID here will not introduce any trouble, thus I choose not to drop
it.

--
Thanks,
Yunlong Song