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 X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 33D8DC5519F for ; Sun, 22 Nov 2020 13:43:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D93B820773 for ; Sun, 22 Nov 2020 13:43:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="INjCAaq6" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727424AbgKVNnt (ORCPT ); Sun, 22 Nov 2020 08:43:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44066 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726436AbgKVNnt (ORCPT ); Sun, 22 Nov 2020 08:43:49 -0500 Received: from mail-pg1-x542.google.com (mail-pg1-x542.google.com [IPv6:2607:f8b0:4864:20::542]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 954E7C0613CF for ; Sun, 22 Nov 2020 05:43:47 -0800 (PST) Received: by mail-pg1-x542.google.com with SMTP id s63so1853879pgc.8 for ; Sun, 22 Nov 2020 05:43:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VMo/8v8YIHV7YCxMTFk+wlmDiZzGS+1fgOlxp3NW+zk=; b=INjCAaq6oOv/oBDuqHTLj0/biF5ioqGgLs9OLO0CYfhKajkiiN6LU2cPOypmixoCYu PXxUEZlAyPlwdfvOY3PxhfOMk0Q7opZ3GODIYqzT7cFgenAuTr+2rqP3SdT94Sp1YAPE 9HNhMO2qbPKXJDgF9vuTuWfwHkZbG6IwCEVfXWfHC7OXkRY/WDEwP2BgAXUj44xpUATs brVNtivLEiZSzir4bXKv0T4dmz12kJJlyHbEqKCNdxdRl3SMmkVuOwZ3WcKJrr0Qyj8I LXCz7+MiLdQ3bNb8sKWwV25xdR6phfF51OItO0uUOIUQzyL2Y78M5yqKoG4oI8AsyHvU MLoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VMo/8v8YIHV7YCxMTFk+wlmDiZzGS+1fgOlxp3NW+zk=; b=t9N8RyhG+5lbTOpK0zxUsnn1fzy9D3obIH3OzytbzmYv6WWNNVJYDRnCJTVNFK62Lg +NuyxUh65/yZwOXRR3lR+gzPFO6fr+4N38S+xXRpZlBqW3U5vRfVbUOaj9pqFE0HfeEJ v2nQWybSZyArN0Cq84ze25/0UIwFUmIjbYZWODwBE8IwFtzJ4tHVHulsUWdss69Lts+4 N4Lg2yRpm1rk3VNz9Jf9hzEYUlrqWKwW2N28l2aZHdrYJKDw8Ow6vXCjeDMKCYY/LIU8 8Z2fzqIQBAKfH4ZW7mOcuLYMiKKzLxXmYxTnYmIxvXsFuCYhmlAppEg6Ps8D6yeKvpMF oE8A== X-Gm-Message-State: AOAM533bUEd5dhNLiUnecTGc5tTVXVSvCoTgI4xtrwTrsf0rlGkYLtLp ZIRn289tMBnp/qftp552Pk7wUg== X-Google-Smtp-Source: ABdhPJxI5pg/SCpTmhDzOWHOPGG4pg40SBSvv9qqkrmRSXS4xQslIcnQzAfzrxeXVx3NIiUKLy0k6w== X-Received: by 2002:aa7:969d:0:b029:196:59ad:ab93 with SMTP id f29-20020aa7969d0000b029019659adab93mr21527507pfk.16.1606052626829; Sun, 22 Nov 2020 05:43:46 -0800 (PST) Received: from leoy-ThinkPad-X240s ([103.127.239.100]) by smtp.gmail.com with ESMTPSA id d25sm1243320pfo.172.2020.11.22.05.43.42 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Sun, 22 Nov 2020 05:43:46 -0800 (PST) Date: Sun, 22 Nov 2020 21:43:39 +0800 From: Leo Yan To: Salvatore Bonaccorso Cc: Andrey Zhizhikin , stable@vger.kernel.org, Alexander Shishkin , Jiri Olsa , Mark Rutland , Namhyung Kim , Peter Zijlstra , Suzuki Poulouse , Tor Jeremiassen , linux-arm-kernel@lists.infradead.org, Arnaldo Carvalho de Melo , Guenter Roeck , Greg Kroah-Hartman Subject: Re: [PATCH] Revert "perf cs-etm: Move definition of 'traceid_list' global variable from header file" Message-ID: <20201122134339.GA6071@leoy-ThinkPad-X240s> References: <20201120073909.357536-1-carnil@debian.org> <20201120133400.GA405401@eldamar.lan> <20201120155317.GA502412@eldamar.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201120155317.GA502412@eldamar.lan> User-Agent: Mutt/1.9.4 (2018-02-28) Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org Hi Salvatore, Andrey, On Fri, Nov 20, 2020 at 04:53:17PM +0100, Salvatore Bonaccorso wrote: > Hi Andrey, > > On Fri, Nov 20, 2020 at 03:29:39PM +0100, Andrey Zhizhikin wrote: > > Hello Salvatore, > > > > On Fri, Nov 20, 2020 at 2:34 PM Salvatore Bonaccorso wrote: > > > > > > Hi Andrey, > > > > > > On Fri, Nov 20, 2020 at 10:54:22AM +0100, Andrey Zhizhikin wrote: > > > > On Fri, Nov 20, 2020 at 8:39 AM Salvatore Bonaccorso wrote: > > > > > > > > > > This reverts commit 168200b6d6ea0cb5765943ec5da5b8149701f36a upstream. > > > > > (but only from 4.19.y) > > > > > > > > This revert would fail the build of 4.19.y with gcc10, I believe the > > > > original commit was introduced to address exactly this case. If this > > > > is intended behavior that 4.19.y is not compiled with newer gcc > > > > versions - then this revert is OK. > > > > > > TTBOMK, this would not regress the build for newer gcc (specifically > > > gcc10) as 4.19.158 is failing perf tool builds there as well (without > > > the above commit reverted). Just as an example v4.19.y does not have > > > cff20b3151cc ("perf tests bp_account: Make global variable static") > > > which is there in v5.6-rc6 to fix build failures with 10.0.1. > > > > > > But it did regress builds with older gcc's as for instance used in > > > Debian buster (gcc 8.3.0) since 4.19.152. > > > > > > Do I possibly miss something? If there is a solution to make it build > > > with newer GCCs and *not* regress previously working GCC versions then > > > this is surely the best outcome though. > > > > I guess (and from what I understand in Leo's reply), porting of > > 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to > > traceID-metadata") should solve the issue for both older and newer gcc > > versions. > > > > The breakage is now in > > [tools/perf/util/cs-etm-decoder/cs-etm-decoder.c] file (which uses > > traceid_list inside). This is solved with the above commit, which > > concealed traceid_list internally inside [tools/perf/util/cs-etm.c] > > file and exposed to [tools/perf/util/cs-etm-decoder/cs-etm-decoder.c] > > via cs_etm__get_cpu() call. > > > > Can you try out to port that commit to see if that would solve your > > regression? > > So something like the following will compile as well with the older > gcc version. > > I realize: I mainline the order of the commits was: > > 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata") > 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header f > ile") > > But to v4.19.y only 168200b6d6ea was backported, and while that was > done I now realize the comment was also changed including the change > fom 95c6fe970a01. > > Thus the proposed backported patch would drop the change in > tools/perf/util/cs-etm.c to the comment as this was already done. > Thecnically currently the comment would be wrong, because it reads: > > /* RB tree for quick conversion between traceID and metadata pointers */ > > but backport of 95c6fe970a01 is not included. > > Would the right thing to do thus be: > > - Revert b801d568c7d8 "perf cs-etm: Move definition of 'traceid_list' global variable from header file" > - Backport 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata") > - Backport 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header file") > > ? > > Leo ist that what you were proposing? Though this isn't my proposing, I totally agree with this :) Just some notes: prior to apply the commit 95c6fe970a01, tools/perf/util/cs-etm-decoder/cs-etm-decoder.c is the only source file which uses the variable "traceid_list"; after applied the commit 95c6fe970a01, the code for using the variable "traceid_list" has been moved out from tools/perf/util/cs-etm-decoder/cs-etm-decoder.c and moved in tools/perf/util/cs-etm.c. So the commit 168200b6d6ea moves the definition of "traceid_list" from the header file to the source file tools/perf/util/cs-etm.c and it defines the variable as "static". As you mentioned, backporting 95c6fe970a01 and 168200b6d6ea can fix both for the older (8.3.0) and new GCC (10.0.1). And I confirmed this should not cause logic issue. Thanks for looking at this issue. Leo 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 X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C1D5C5519F for ; Sun, 22 Nov 2020 13:45:21 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C1BE220771 for ; Sun, 22 Nov 2020 13:45:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="k91mAvY3"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="INjCAaq6" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C1BE220771 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Apzo86vL3xT+OBiy14fNztao0o63vieox9meCjrTHQg=; b=k91mAvY3gUd1NssVkgaYur6eR DTkQCi+GZ4javp+o3j00dmuFifa1djjw43ex+gXiJGGauXYmLj/9rBZjkQpG5raxQIqVlz8ltE2y6 hHU6iObcJbzlWkigmVE3UM9tcDDy52R6/+KC7tmgN+J5G56dBcKnCxMgTtsauYay3c/fjnYEvRQ79 XfdCbecusYaxRspwHG5u74oH9CQRDEtI+BVXyltK3vWe2srjzGQv0rzt5HagsgwegR+INJzX6M+Sd 0mbW5NFQ2YOjzQ8f5ejxiPnbJ3M1dPNsrYczJDtuOHyuCD5SJ0fqFkQD18lZEfieLNbniXFEksQYE 5/Gj6SK/A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kgpej-0001AO-7e; Sun, 22 Nov 2020 13:43:53 +0000 Received: from mail-pf1-x444.google.com ([2607:f8b0:4864:20::444]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kgpeg-00019j-Rg for linux-arm-kernel@lists.infradead.org; Sun, 22 Nov 2020 13:43:52 +0000 Received: by mail-pf1-x444.google.com with SMTP id b6so1925233pfp.7 for ; Sun, 22 Nov 2020 05:43:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=VMo/8v8YIHV7YCxMTFk+wlmDiZzGS+1fgOlxp3NW+zk=; b=INjCAaq6oOv/oBDuqHTLj0/biF5ioqGgLs9OLO0CYfhKajkiiN6LU2cPOypmixoCYu PXxUEZlAyPlwdfvOY3PxhfOMk0Q7opZ3GODIYqzT7cFgenAuTr+2rqP3SdT94Sp1YAPE 9HNhMO2qbPKXJDgF9vuTuWfwHkZbG6IwCEVfXWfHC7OXkRY/WDEwP2BgAXUj44xpUATs brVNtivLEiZSzir4bXKv0T4dmz12kJJlyHbEqKCNdxdRl3SMmkVuOwZ3WcKJrr0Qyj8I LXCz7+MiLdQ3bNb8sKWwV25xdR6phfF51OItO0uUOIUQzyL2Y78M5yqKoG4oI8AsyHvU MLoA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=VMo/8v8YIHV7YCxMTFk+wlmDiZzGS+1fgOlxp3NW+zk=; b=MrgPXOln/7j8RWhe0qlyEXvvyOKnk995mkorUBPN4gyeFTE6ezAgYbdhrqNLoLuEr2 YnrbgzXo692chsbHofTHAPXEef7nktx0uE66h2RxwXEeV/1D1L/+/YoKTvm2FRtVifh8 eUnHUD8RZrsc5YRbz64rkVD7hxQOwIa5yC5F4ivToUsOTYi4gBo1xr3W0SfZhWxTv6bg RDiiFmVwPWF1rWqkZxv+ZSoyf/a2GVXSTJazI0Zvye3/Kvb2CaozqabC5a4a/2M2eVMs ZXxTXgiTP8WcRhW2lKUZffigwaDSvIbsrxlSYZ/tP56BZiqvngy/04HA53qd5rfwPV/g 14JQ== X-Gm-Message-State: AOAM532DxvtksnDCZtWXj2XpnaOqIlu7M4//7aj0J8jz+4Ruc1oCxneo IMGlVVesUrSAdIFYLL1QB4zEbg== X-Google-Smtp-Source: ABdhPJxI5pg/SCpTmhDzOWHOPGG4pg40SBSvv9qqkrmRSXS4xQslIcnQzAfzrxeXVx3NIiUKLy0k6w== X-Received: by 2002:aa7:969d:0:b029:196:59ad:ab93 with SMTP id f29-20020aa7969d0000b029019659adab93mr21527507pfk.16.1606052626829; Sun, 22 Nov 2020 05:43:46 -0800 (PST) Received: from leoy-ThinkPad-X240s ([103.127.239.100]) by smtp.gmail.com with ESMTPSA id d25sm1243320pfo.172.2020.11.22.05.43.42 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Sun, 22 Nov 2020 05:43:46 -0800 (PST) Date: Sun, 22 Nov 2020 21:43:39 +0800 From: Leo Yan To: Salvatore Bonaccorso Subject: Re: [PATCH] Revert "perf cs-etm: Move definition of 'traceid_list' global variable from header file" Message-ID: <20201122134339.GA6071@leoy-ThinkPad-X240s> References: <20201120073909.357536-1-carnil@debian.org> <20201120133400.GA405401@eldamar.lan> <20201120155317.GA502412@eldamar.lan> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20201120155317.GA502412@eldamar.lan> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201122_084351_185854_0A4FB16C X-CRM114-Status: GOOD ( 36.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Andrey Zhizhikin , Mark Rutland , Suzuki Poulouse , Alexander Shishkin , Greg Kroah-Hartman , stable@vger.kernel.org, Tor Jeremiassen , Arnaldo Carvalho de Melo , Peter Zijlstra , Guenter Roeck , Namhyung Kim , Jiri Olsa , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Salvatore, Andrey, On Fri, Nov 20, 2020 at 04:53:17PM +0100, Salvatore Bonaccorso wrote: > Hi Andrey, > > On Fri, Nov 20, 2020 at 03:29:39PM +0100, Andrey Zhizhikin wrote: > > Hello Salvatore, > > > > On Fri, Nov 20, 2020 at 2:34 PM Salvatore Bonaccorso wrote: > > > > > > Hi Andrey, > > > > > > On Fri, Nov 20, 2020 at 10:54:22AM +0100, Andrey Zhizhikin wrote: > > > > On Fri, Nov 20, 2020 at 8:39 AM Salvatore Bonaccorso wrote: > > > > > > > > > > This reverts commit 168200b6d6ea0cb5765943ec5da5b8149701f36a upstream. > > > > > (but only from 4.19.y) > > > > > > > > This revert would fail the build of 4.19.y with gcc10, I believe the > > > > original commit was introduced to address exactly this case. If this > > > > is intended behavior that 4.19.y is not compiled with newer gcc > > > > versions - then this revert is OK. > > > > > > TTBOMK, this would not regress the build for newer gcc (specifically > > > gcc10) as 4.19.158 is failing perf tool builds there as well (without > > > the above commit reverted). Just as an example v4.19.y does not have > > > cff20b3151cc ("perf tests bp_account: Make global variable static") > > > which is there in v5.6-rc6 to fix build failures with 10.0.1. > > > > > > But it did regress builds with older gcc's as for instance used in > > > Debian buster (gcc 8.3.0) since 4.19.152. > > > > > > Do I possibly miss something? If there is a solution to make it build > > > with newer GCCs and *not* regress previously working GCC versions then > > > this is surely the best outcome though. > > > > I guess (and from what I understand in Leo's reply), porting of > > 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to > > traceID-metadata") should solve the issue for both older and newer gcc > > versions. > > > > The breakage is now in > > [tools/perf/util/cs-etm-decoder/cs-etm-decoder.c] file (which uses > > traceid_list inside). This is solved with the above commit, which > > concealed traceid_list internally inside [tools/perf/util/cs-etm.c] > > file and exposed to [tools/perf/util/cs-etm-decoder/cs-etm-decoder.c] > > via cs_etm__get_cpu() call. > > > > Can you try out to port that commit to see if that would solve your > > regression? > > So something like the following will compile as well with the older > gcc version. > > I realize: I mainline the order of the commits was: > > 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata") > 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header f > ile") > > But to v4.19.y only 168200b6d6ea was backported, and while that was > done I now realize the comment was also changed including the change > fom 95c6fe970a01. > > Thus the proposed backported patch would drop the change in > tools/perf/util/cs-etm.c to the comment as this was already done. > Thecnically currently the comment would be wrong, because it reads: > > /* RB tree for quick conversion between traceID and metadata pointers */ > > but backport of 95c6fe970a01 is not included. > > Would the right thing to do thus be: > > - Revert b801d568c7d8 "perf cs-etm: Move definition of 'traceid_list' global variable from header file" > - Backport 95c6fe970a01 ("perf cs-etm: Change tuple from traceID-CPU# to traceID-metadata") > - Backport 168200b6d6ea ("perf cs-etm: Move definition of 'traceid_list' global variable from header file") > > ? > > Leo ist that what you were proposing? Though this isn't my proposing, I totally agree with this :) Just some notes: prior to apply the commit 95c6fe970a01, tools/perf/util/cs-etm-decoder/cs-etm-decoder.c is the only source file which uses the variable "traceid_list"; after applied the commit 95c6fe970a01, the code for using the variable "traceid_list" has been moved out from tools/perf/util/cs-etm-decoder/cs-etm-decoder.c and moved in tools/perf/util/cs-etm.c. So the commit 168200b6d6ea moves the definition of "traceid_list" from the header file to the source file tools/perf/util/cs-etm.c and it defines the variable as "static". As you mentioned, backporting 95c6fe970a01 and 168200b6d6ea can fix both for the older (8.3.0) and new GCC (10.0.1). And I confirmed this should not cause logic issue. Thanks for looking at this issue. Leo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel