From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753245AbeAIN1u (ORCPT + 1 other); Tue, 9 Jan 2018 08:27:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56410 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753065AbeAIN1t (ORCPT ); Tue, 9 Jan 2018 08:27:49 -0500 Date: Tue, 9 Jan 2018 14:27:45 +0100 From: Jiri Olsa To: Arnaldo Carvalho de Melo Cc: Namhyung Kim , Jiri Olsa , Peter Zijlstra , lkml , Ingo Molnar , David Ahern , Andi Kleen , Alexander Shishkin , kernel-team@lge.com Subject: Re: [PATCH 12/12] perf report: Add --task option to display monitored tasks Message-ID: <20180109132745.GA13256@krava> References: <20180107160356.28203-1-jolsa@kernel.org> <20180107160356.28203-13-jolsa@kernel.org> <20180109015607.GA7138@sejong> <20180109091551.GA11520@krava> <20180109130524.GA30494@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180109130524.GA30494@kernel.org> User-Agent: Mutt/1.9.1 (2017-09-22) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 09 Jan 2018 13:27:48 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Tue, Jan 09, 2018 at 10:05:24AM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Jan 09, 2018 at 10:15:51AM +0100, Jiri Olsa escreveu: > > On Tue, Jan 09, 2018 at 10:56:07AM +0900, Namhyung Kim wrote: > > > > SNIP > > > > > > +static struct task *task_list(struct task *task, struct machine *machine) > > > > +{ > > > > + struct thread *parent_thread, *thread = task->thread; > > > > + struct task *parent_task; > > > > + > > > > + /* Already listed. */ > > > > + if (!list_empty(&task->list)) > > > > + return NULL; > > > > + > > > > + /* Last one in the chain. */ > > > > + if (thread->ppid == -1) > > > > + return task; > > > > + > > > > + parent_thread = machine__findnew_thread(machine, -1, thread->ppid); > > > > > > I think it should be machine__find_thread() since creating a new > > > thread at this stage would lack thread->priv anyway. > > > > ugh, that's right.. I tried to stay safe, but the NULL > > in priv would bring it down anyway > > > > Arnaldo, > > I already see changed version on top of your branch, > > please let me know if you make also this change or > > I should send v2 > > I can make the change, thanks! > > Just to make sure we're on the same page, just this, right? > > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index 3685ac101b16..e60709fe31ed 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -633,9 +633,9 @@ static struct task *tasks_list(struct task *task, struct machine *machine) > if (thread->ppid == -1) > return task; > > - parent_thread = machine__findnew_thread(machine, -1, thread->ppid); > + parent_thread = machine__find_thread(machine, -1, thread->ppid); yes > if (!parent_thread) > - return ERR_PTR(-ENOMEM); > + return ERR_PTR(-ENOENT); right, I haven't noticed this one.. then we should also do in the attached change thanks, jirka --- diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index f7338b594844..67c74d6dab24 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -710,7 +710,7 @@ static int tasks_print(struct report *rep, FILE *fp) if (IS_ERR(task)) { pr_err("Error: failed to process tasks\n"); free(tasks); - return -ENOMEM; + return PTR_ERR(task); } if (task)