On Wed, May 08, 2019 at 10:36:48PM +0800, Wei Li wrote:
> After thread is added to machine->threads[i].dead in
> __machine__remove_thread, the machine->threads[i].dead is freed
> when calling free(session) in perf_session__delete(). So it get a
> Segmentation fault when accessing it in thread__put().
> 
> In this patch, we delay the perf_session__delete until all threads
> have been deleted.
> 
> This can be reproduced by following steps:
>       ulimit -c unlimited
>       export MALLOC_MMAP_THRESHOLD_=0
>       perf sched record sleep 10
>       perf sched latency --sort max
>       Segmentation fault (core dumped)
> 
> Signed-off-by: Zhipeng Xie <xiezhipe...@huawei.com>
> Signed-off-by: Wei Li <liwei...@huawei.com>

Acked-by: Namhyung Kim <namhy...@kernel.org>

Thahks,
Namhyung


> ---
>  tools/perf/builtin-sched.c | 63 ++++++++++++++++++++++++++------------
>  1 file changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 275f2d92a7bf..8a4841fa124c 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -1774,7 +1774,8 @@ static int perf_sched__process_comm(struct perf_tool 
> *tool __maybe_unused,
>       return 0;
>  }
>  
> -static int perf_sched__read_events(struct perf_sched *sched)
> +static int __perf_sched__read_events(struct perf_sched *sched,
> +                                     struct perf_session *session)
>  {
>       const struct perf_evsel_str_handler handlers[] = {
>               { "sched:sched_switch",       process_sched_switch_event, },
> @@ -1783,30 +1784,17 @@ static int perf_sched__read_events(struct perf_sched 
> *sched)
>               { "sched:sched_wakeup_new",   process_sched_wakeup_event, },
>               { "sched:sched_migrate_task", process_sched_migrate_task_event, 
> },
>       };
> -     struct perf_session *session;
> -     struct perf_data data = {
> -             .path  = input_name,
> -             .mode  = PERF_DATA_MODE_READ,
> -             .force = sched->force,
> -     };
> -     int rc = -1;
> -
> -     session = perf_session__new(&data, false, &sched->tool);
> -     if (session == NULL) {
> -             pr_debug("No Memory for session\n");
> -             return -1;
> -     }
>  
>       symbol__init(&session->header.env);
>  
>       if (perf_session__set_tracepoints_handlers(session, handlers))
> -             goto out_delete;
> +             return -1;
>  
>       if (perf_session__has_traces(session, "record -R")) {
>               int err = perf_session__process_events(session);
>               if (err) {
>                       pr_err("Failed to process events, error %d", err);
> -                     goto out_delete;
> +                     return -1;
>               }
>  
>               sched->nr_events      = session->evlist->stats.nr_events[0];
> @@ -1814,9 +1802,28 @@ static int perf_sched__read_events(struct perf_sched 
> *sched)
>               sched->nr_lost_chunks = 
> session->evlist->stats.nr_events[PERF_RECORD_LOST];
>       }
>  
> -     rc = 0;
> -out_delete:
> +     return 0;
> +}
> +
> +static int perf_sched__read_events(struct perf_sched *sched)
> +{
> +     struct perf_session *session;
> +     struct perf_data data = {
> +             .path  = input_name,
> +             .mode  = PERF_DATA_MODE_READ,
> +             .force = sched->force,
> +     };
> +     int rc;
> +
> +     session = perf_session__new(&data, false, &sched->tool);
> +     if (session == NULL) {
> +             pr_debug("No Memory for session\n");
> +             return -1;
> +     }
> +
> +     rc = __perf_sched__read_events(sched, session);
>       perf_session__delete(session);
> +
>       return rc;
>  }
>  
> @@ -3130,12 +3137,25 @@ static void perf_sched__merge_lat(struct perf_sched 
> *sched)
>  
>  static int perf_sched__lat(struct perf_sched *sched)
>  {
> +     struct perf_session *session;
> +     struct perf_data data = {
> +             .path  = input_name,
> +             .mode  = PERF_DATA_MODE_READ,
> +             .force = sched->force,
> +     };
>       struct rb_node *next;
> +     int rc = -1;
>  
>       setup_pager();
>  
> -     if (perf_sched__read_events(sched))
> +     session = perf_session__new(&data, false, &sched->tool);
> +     if (session == NULL) {
> +             pr_debug("No Memory for session\n");
>               return -1;
> +     }
> +
> +     if (__perf_sched__read_events(sched, session))
> +             goto out_delete;
>  
>       perf_sched__merge_lat(sched);
>       perf_sched__sort_lat(sched);
> @@ -3164,7 +3184,10 @@ static int perf_sched__lat(struct perf_sched *sched)
>       print_bad_events(sched);
>       printf("\n");
>  
> -     return 0;
> +     rc = 0;
> +out_delete:
> +     perf_session__delete(session);
> +     return rc;
>  }
>  
>  static int setup_map_cpus(struct perf_sched *sched)
> -- 
> 2.17.1
> 

Reply via email to