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 ADE65C3DA4A for ; Thu, 8 Aug 2024 10:51:59 +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:References:Cc:To:From: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=4rFtwmjq+6gSRzGjHdsp3/4i675ak1VK4xDruVkNphg=; b=kYPBSqCRn7zFlK4vlilUNMnZ/B M4K0OQcaXY3zrIMQfsSGxrlVtitOPJL7yvtajjk+py8FX1Uw13nYWRLiowpu3gwxXS5TTxyRurumx NaFNLRf+mDcM2SbNpObcTY7zllN5mqj+Z6seNqfkr8Ss4843RbUotFz5eRiH5doOrWNHRMZxwhCTY H2LkdTxexySNeJYqWzIligk9jxAc1AUDZCYu2DtoaZ9lcS7+0+66F7S+koX69NToDuKfbYDpVbz9e ZtOZD0wvxJuoJN/CwRmMwo6TybfNLUkUmbxhAze1DJkdGI0oK2uq+xzWUZruTdmyQFJfpxFQ+J6PP JeVuU+VQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sc0kI-00000007yJ3-2fPl; Thu, 08 Aug 2024 10:51:50 +0000 Received: from mail-wr1-x42d.google.com ([2a00:1450:4864:20::42d]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sc0jY-00000007yA3-1Xon for linux-arm-kernel@lists.infradead.org; Thu, 08 Aug 2024 10:51:05 +0000 Received: by mail-wr1-x42d.google.com with SMTP id ffacd0b85a97d-368380828d6so440429f8f.1 for ; Thu, 08 Aug 2024 03:51:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1723114262; x=1723719062; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:from:subject:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=4rFtwmjq+6gSRzGjHdsp3/4i675ak1VK4xDruVkNphg=; b=QC2CBVLAlQeQjXAKDQEYizHq7Wjrqf55PpV0UcIS5UL2FEfpcrDnBAYbBBFG0U1XCs Lv3Zi84DslWjYULynHQ07zSYoDqJHZ/etPi1I29N4WQ0P7nH6nH/8PQA40ti7C5xZr19 glt8v63qbw9XZKlaZVRip2LDEQTWjSujfeFV6eTu8uysymYlacxv49OsqGD4orlmDXX/ 29EU+iq0Lm1W3SlAq3aZAtZn/Nd6Sw1mVpXtlx4DVXqmARnHkXp2CQrlcgWypsvmFF0r RSI6dxob5dV2wMGGfoHMTKmNbCrmSX8hbvRA0vusLMGwJ0ZxatZWAX9hq5416+x+DU1W iRFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723114262; x=1723719062; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:from:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=4rFtwmjq+6gSRzGjHdsp3/4i675ak1VK4xDruVkNphg=; b=SsdMgcaRAte47qRI9RCCjUyrTRjTCXFe/QIwgAKy0uPq77K3b+GBg3c8ShfImLq7UF zyPSf4OHgwkIJ7fEAfOQU5ufJKMRRax1UirrScXwUFpu8EaqF0j/+1kOkeNMRyvOmx+H zz3jJrzvkjhvMRaF9674zoRmPoGe3e3f5gX3d6Hd3hf5UohuA3h3jJ3X2JBAL0G57D52 RT3y8oICwKTZQDqcyJwW6BPwIo66s8vaLesWukeT9ohzGT2QycWPViFBeiIl+AdGZXgD iBBM/4kuxMvp2/UIR7Y+2BIMtcebCW1ZC+WgxKkkIk16j36fY2aHYQJnJLn0x1nIqMFK bDGw== X-Forwarded-Encrypted: i=1; AJvYcCXI0+d5L84JiD2wUKeLb8XgrB00q21p0OH6kaXbvJJz9HBrTs4614ErhKIO0F+yKOZ0utM8eDANwINV20K3ALV1t63ZUJ92orAm/aQUdCrELvnKZFA= X-Gm-Message-State: AOJu0Yz1uS4Ci3cppfYbFDMtq45KK4NhCj0pRdyYm7N4iyaDFjiOG6Gp 8ngXzvHm3viUBPz79z+y84IchExkyayTplPBT5J/ERMxJ5Ynb8sMkxTVxxyu9vU9DMb9tJ0F0Fc c X-Google-Smtp-Source: AGHT+IHYODqKwPOkMQH3b0u677P1na0+txhJF5Z0f9RBhMkkYBGsORMwRzJ90PQBs6LpH+5KU3ZylA== X-Received: by 2002:adf:f3c4:0:b0:369:e72c:875f with SMTP id ffacd0b85a97d-36d2756eeb3mr1100344f8f.48.1723114262310; Thu, 08 Aug 2024 03:51:02 -0700 (PDT) Received: from [192.168.1.3] ([89.47.253.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36d27180b97sm1497150f8f.46.2024.08.08.03.51.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Aug 2024 03:51:01 -0700 (PDT) Message-ID: <16ea091e-0f3b-44cf-b3b4-b07efabe9c02@linaro.org> Date: Thu, 8 Aug 2024 11:51:00 +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 From: James Clark To: Leo Yan , Ganapatrao Kulkarni , 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> <2c0cd5b7-1ca6-4088-817c-209026266d58@linaro.org> Content-Language: en-US In-Reply-To: <2c0cd5b7-1ca6-4088-817c-209026266d58@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240808_035104_437923_23650D8C X-CRM114-Status: GOOD ( 23.48 ) 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 08/08/2024 10:21 am, James Clark wrote: > > > On 08/08/2024 8:42 am, Leo Yan wrote: >> 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)) > > The options.force for the print should be "options.verbose or not > options.force" I think? You want to print the error until the user adds > -f, then hide it. Unless verbose is on. > >>>>> +               if (options.force): >>>>> +                       return > > Oops I had this one the wrong way around in my example. This way is > correct. > >>>> >>>> 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 > > But your diff looks good Ganapat, I think send a patch with Leo's extra > help message added and the first force flipped. One other small detail about Leo's suggestion print out. Can you add an instruction of how to keep the warnings as well: print("Use option '-f' following the script for force mode. Add '-v' \ to continue printing decode warnings.")