From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21617C6FD1D for ; Tue, 4 Apr 2023 06:59:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233346AbjDDG7T (ORCPT ); Tue, 4 Apr 2023 02:59:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36170 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230193AbjDDG7T (ORCPT ); Tue, 4 Apr 2023 02:59:19 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E254030E3 for ; Mon, 3 Apr 2023 23:58:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1680591495; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fNPp2+ZNXETTv95TLGejYgL6e8HCBOjsfufr9g7el2c=; b=UGuQkWNt2Xkk10dUtw7qfUCE7VTjY/7uYPP6R+7dFR285Yamt19m0cofN3k2KAOkko1xIP ENvq5WI47V0LhyZAe3/T4xCSdUIh5JAOlEIFM2MzDlnhQIq5iSIb2om/ueWqXwuD3epmNP 5hhSRH74SDm9ojh0Zn2jCpdRr16Fuow= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-175-kWcNkGAzP2eF7ajQflutQw-1; Tue, 04 Apr 2023 02:58:11 -0400 X-MC-Unique: kWcNkGAzP2eF7ajQflutQw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 345C42999B32; Tue, 4 Apr 2023 06:58:11 +0000 (UTC) Received: from samus.usersys.redhat.com (unknown [10.43.17.26]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3AF7E2027061; Tue, 4 Apr 2023 06:58:09 +0000 (UTC) Date: Tue, 4 Apr 2023 08:58:07 +0200 From: Artem Savkov To: Namhyung Kim Cc: Arnaldo Carvalho de Melo , Adrian Hunter , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, Milian Wolff , Masami Hiramatsu , Andrii Nakryiko Subject: Re: [PATCH 0/1] perf report: append inlines to non-dwarf callchains Message-ID: <20230404065807.GB56712@samus.usersys.redhat.com> References: <20230316133557.868731-1-asavkov@redhat.com> <8f7077e8-bcce-a13f-48d3-92a3cb80b02a@intel.com> <20230331085224.GA688995@samus.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org On Mon, Apr 03, 2023 at 10:47:37PM -0700, Namhyung Kim wrote: > On Mon, Apr 3, 2023 at 1:30 PM Arnaldo Carvalho de Melo wrote: > > > > Em Fri, Mar 31, 2023 at 10:52:24AM +0200, Artem Savkov escreveu: > > > On Thu, Mar 30, 2023 at 08:06:20AM +0300, Adrian Hunter wrote: > > > > On 22/03/23 21:44, Arnaldo Carvalho de Melo wrote: > > > > > Em Wed, Mar 22, 2023 at 11:18:49AM -0700, Namhyung Kim escreveu: > > > > >> On Fri, Mar 17, 2023 at 12:41 AM Artem Savkov wrote: > > > > >>> > > > > >>> On Thu, Mar 16, 2023 at 02:26:18PM -0700, Namhyung Kim wrote: > > > > >>>> Hello, > > > > >>>> > > > > >>>> On Thu, Mar 16, 2023 at 6:36 AM Artem Savkov wrote: > > > > >>>>> > > > > >>>>> In an email to Arnaldo Andrii Nakryiko suggested that perf can get > > > > >>>>> information about inlined functions from dwarf when available and then > > > > >>>>> add it to userspace stacktraces even in framepointer or lbr mode. > > > > >>>>> Looking closer at perf it turned out all required bits and pieces are > > > > >>>>> already there and inline information can be easily added to both > > > > >>>>> framepointer and lbr callchains by adding an append_inlines() call to > > > > >>>>> add_callchain_ip(). > > > > >>>> > > > > >>>> Looks great! Have you checked it with perf report -g callee ? > > > > >>>> I'm not sure the ordering of inlined functions is maintained > > > > >>>> properly. Maybe you can use --no-children too to simplify > > > > >>>> the output. > > > > >>> > > > > >>> Thanks for the suggestion. I actually have another test program with > > > > >>> functions being numbered rather than (creatively) named, so it might be > > > > >>> easier to use it to figure out ordering. Here's the code: > > > > >> > > > > >> Yep, looks good. > > > > >> > > > > >> Acked-by: Namhyung Kim > > > > > > > > > > So, I'll apply this shorter patch instead, ok? > > > > > > > > > > - Arnaldo > > > > > > > > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > > > > > index 803c9d1803dd26ef..abf6167f28217fe6 100644 > > > > > --- a/tools/perf/util/machine.c > > > > > +++ b/tools/perf/util/machine.c > > > > > @@ -44,6 +44,7 @@ > > > > > #include > > > > > > > > > > static void __machine__remove_thread(struct machine *machine, struct thread *th, bool lock); > > > > > +static int append_inlines(struct callchain_cursor *cursor, struct map_symbol *ms, u64 ip); > > > > > > > > > > static struct dso *machine__kernel_dso(struct machine *machine) > > > > > { > > > > > @@ -2322,6 +2323,10 @@ static int add_callchain_ip(struct thread *thread, > > > > > ms.maps = al.maps; > > > > > ms.map = al.map; > > > > > ms.sym = al.sym; > > > > > + > > > > > + if (append_inlines(cursor, &ms, ip) == 0) > > > > > + return 0; > > > > > + > > > > > srcline = callchain_srcline(&ms, al.addr); > > > > > return callchain_cursor_append(cursor, ip, &ms, > > > > > branch, flags, nr_loop_iter, > > > > > > > > This seems to be breaking --branch-history. I am not sure > > > > append_inlines() makes sense for branches. Maybe this should be: > > > > > > > > if (!branch && !append_inlines(cursor, &ms, ip)) > > > > return 0; > > > > > > > > > > Right. So when cllchain_cursor is appended through append_inlines it > > > always discards branch information, even for the non-inlined function. > > > So adding !branch makes sense to me. Does anyone else see any problems > > > with that? > > > > I'm no expert in this specific area, so for now till we get to a > > conclusion on this, I'll follow Andi's suggestion and revert this patch. > > I think we can simply apply Adrian's patch above. I can send a v2 with this fix included if that'll be more convenient. -- Artem