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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 261FEC3DA4A for ; Thu, 8 Aug 2024 07:43:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mtA8TobpQsyBk46aK6kx4aM0hpW52mRmAODQbBxugTI=; b=yvtaFT85sf30puIZwZBRLN9vDg b0N1VDTxXyPfDIUhIIuD0Qe7qx4FP4M9woQ6Ic8VTczLa1h2i0Tf7p+BS9GozNg+ktNWK5xI6kd38 U/17LgY6/lkJvMiTR6d/YZp9R0RFDwVAC43oXn2oZ0U3lPnBjPA5Pu80vjdlHaVVnXa1RiRGOyEy0 l6woSMCQ+a1ZDiFlCjxSjaH8la0JwLnoeTYr7VhRfGec77HQYr05EuvCnPoB5RZwvTa1C+N8TYJX2 yxg0hEtz0z17o+5xlTvTva2tBPHn7UCLnGy0bVirjeOvjOrIvifVA8DrProtlELvsAUaY8gt9CEOK xAmMxZVg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sbxnY-00000007T4I-3Iqb; Thu, 08 Aug 2024 07:43:00 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sbxmz-00000007SwV-1qBu for linux-arm-kernel@lists.infradead.org; Thu, 08 Aug 2024 07:42:27 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4ADE7FEC; Thu, 8 Aug 2024 00:42:47 -0700 (PDT) Received: from [10.1.36.15] (PF4Q20KV.arm.com [10.1.36.15]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 0ED1B3F71E; Thu, 8 Aug 2024 00:42:18 -0700 (PDT) Message-ID: Date: Thu, 8 Aug 2024 08:42:14 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] perf scripts python arm-cs-trace-disasm.py: Skip disasm if address continuity is broken To: Ganapatrao Kulkarni , James Clark , scclevenger@os.amperecomputing.com Cc: acme@redhat.com, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, darren@os.amperecomputing.com, james.clark@arm.com, suzuki.poulose@arm.com, Al.Grant@arm.com, Mike Leach References: <20240719092619.274730-1-gankulkarni@os.amperecomputing.com> <543813f6-cb1f-4759-b26f-75246750814d@linaro.org> <00fac24c-d664-4ebb-8c60-f4697b7f76c1@linaro.org> <8b53a424-19f7-4042-a2db-e1c5d051f9cc@os.amperecomputing.com> <6adf84fa-b755-4d7a-957a-9bf01e442238@linaro.org> <6f535bb6-2cee-48e6-93f1-ea19887bae74@os.amperecomputing.com> <027c76a9-9bd4-43e9-a170-8391a0037291@linaro.org> <3d7a6f93-0555-48fa-99cb-bf26b53c2da5@os.amperecomputing.com> <4ba157c2-4a56-4d77-9a15-071e46adc33b@os.amperecomputing.com> <915fefb2-b0bc-4306-83ec-22570719e8e4@os.amperecomputing.com> Content-Language: en-US From: Leo Yan In-Reply-To: <915fefb2-b0bc-4306-83ec-22570719e8e4@os.amperecomputing.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240808_004225_585873_80935C4E X-CRM114-Status: GOOD ( 19.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 8/8/2024 5:36 AM, Ganapatrao Kulkarni wrote: > > On 08-08-2024 12:50 am, Leo Yan wrote: >> On 8/7/2024 5:18 PM, Ganapatrao Kulkarni wrote: >> >>> Is below diff with force option looks good? >>> >>> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py >>> b/tools/perf/scripts/python/arm-cs-trace-disasm.py >>> index d973c2baed1c..efe34f308beb 100755 >>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py >>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py >>> @@ -36,7 +36,10 @@ option_list = [ >>>                      help="Set path to objdump executable file"), >>>          make_option("-v", "--verbose", dest="verbose", >>>                      action="store_true", default=False, >>> -                   help="Enable debugging log") >>> +                   help="Enable debugging log"), >>> +       make_option("-f", "--force", dest="force", >>> +                   action="store_true", default=False, >>> +                   help="Force decoder to continue") >>>   ] >>> >>>   parser = OptionParser(option_list=option_list) >>> @@ -257,6 +260,12 @@ def process_event(param_dict): >>>                  print("Stop address 0x%x is out of range [ 0x%x .. 0x%x >>> ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso)) >>>                  return >>> >>> +       if (stop_addr < start_addr): >>> +               if (options.verbose == True or options.force): >>> +                       print("Packet Discontinuity detected [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr, dso)) >>> +               if (options.force): >>> +                       return >> >> I struggled a bit for the code - it is confused that force mode bails out >> and the non-force mode continues to run. I prefer to always bail out for >> the discontinuity case, as it is pointless to continue in this case. > > Kept bail out with force option since I though it is not good to hide > the error in normal use, otherwise we never able to notice this error in > the future and it becomes default hidden. Eventually this error should > be fixed. As James said, the issue should be fixed in OpenCSD or Perf decoding flow. Thus, perf tool should be tolerant errors - report warning and drop discontinuous samples. This would be easier for developers later if face the same issue, they don't need to spend time to locate issue and struggle for overriding the error. If you prefer to use force option, it might be better to give reasoning and *suggestion* in one go, something like: if (stop_addr < start_addr): print("Packet Discontinuity detected [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr, dso)) print("Use option '-f' following the script for force mode" if (options.force) return Either way is fine for me. Thanks a lot for taking time on the issue. Leo > Having said that, It is also seems OK to avoid the error with the > warning message as you suggested. > >> >>            if (stop_addr <= start_addr): >>                print("Packet Discontinuity detected [stop_add:0x%x start_addr:0x%x ] for dso %s" % \ >>                      (stop_addr, start_addr, dso)) >>                return >> > > Thanks, > Ganapat