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

[Yunlong Song]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/