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 B7061C52D71 for ; Thu, 8 Aug 2024 09:22:20 +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=T9Aj0hImGedN0r8XBp+EEG6ozZaXiN1c6ZDFb+vzCUs=; b=peJkjhTfqHtBeQ2rMxpHL8JxFD O7jdbU01xDBpVDu75ynNwAH7wiyQQVTEG0p6BT+leU4ru1dHGHkajM7P6/q/E4C5MniE5Vc3D8EPM bt9MGVvrnltHEfXrxiM8njfea2HEfu4TBDdcCkTdTo8ijtOxw+QdIbzGj3yQO4ZC+Dn/u6+aXe+xU /b0vNCjJiGhX0/7zUn3nBxB7KR5qukXZSAZy18S4v/9ruJ5qm0J8GbMBLxqUKQGFaiNhAxWlJ6Hii qpQiFDMzR2Ecy4NtT/SGWVB8GyuygtIsX7k87AaEWWVLoVdR4JusOIGLkYBpjzP87bXlEPtaWND30 Q5tkEC0w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sbzLV-00000007hoP-0H7T; Thu, 08 Aug 2024 09:22:09 +0000 Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sbzKt-00000007hgP-3Ady for linux-arm-kernel@lists.infradead.org; Thu, 08 Aug 2024 09:21:33 +0000 Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-427fc97a88cso5488945e9.0 for ; Thu, 08 Aug 2024 02:21:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1723108888; x=1723713688; darn=lists.infradead.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=T9Aj0hImGedN0r8XBp+EEG6ozZaXiN1c6ZDFb+vzCUs=; b=J3qud0tbodCP0h94BLv2vJxMtUhwiH6JkXv3DjgTcT+LRJoaUneVVvwRBxtxsMmyF+ EmvAlbbXQBlbPGfq48QmOLf2jxbHYDtQFlIcLNTAMYcpkkFmWPHlbTWURi1bRrGpu1Z2 317AID37+bVVEAUPMQfLnAu+3cmV63uTAt1gBmrpBHBHilseppE++tJdPcqPX6cO9SuJ 1YFsfRjwF2nMLYOWOUn2Ai+1TA2V7QqSykqf3eLZBAITmgtBUJ5knUmoXZQLa7xSibdd ezi62X5KzxUobvw+xi5dDdr11DSSO4avhZdVAlIYhYnKsmQk4Y9T3dxVUfV3vjDVZs6F nk+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723108888; x=1723713688; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=T9Aj0hImGedN0r8XBp+EEG6ozZaXiN1c6ZDFb+vzCUs=; b=FPOTlTTTDGxQ7E1jI5g7NFeeaEZUmVYPQcmd7k+4PHjdhUJH+7GXfzpsyvm0nv19hl cgMFmRyn0+FKu5n+HOn3HWBZ0gilLd/pHhBHP5MwasmCXC88b3nMBTXxs6q2JqiesBYj NHAEMrPAHoOK49tSHTmg/hLLmLbHt3O7LZ/p6S07bSbosHfzxyFACceGmRR8OXvaHWTZ Q5GYOVyP+0SIG14f85FTGF9jev8678NXSeaA848XiMdiuAmAjd7aZXNsoFHuPv/X2ZWO dfRPdpK7l9kmcx+SJEYomHaY0ksX8FtoMNIoyQyFOcu3TvpT8rbwiupDkB0fkz3lYjvZ fcgA== X-Forwarded-Encrypted: i=1; AJvYcCV/2lPxSXQ9yIZhSLlJK/F59jxEpDhKH4F8WJQC6zouHGDmGqIHt0HBYMULKsF9YcB3MluyBJxK1UJKDjHSS2k3tGgqtSC2BPW0vM7nki3NF6hUmgY= X-Gm-Message-State: AOJu0YzbYV9PbQJFGaf/AfHPt+grmY0U3qcqnQMZhJ8AH/kqP2a7+8+j zaTBeFcd7jn9kSiRe4gEB0NKrDbfzXcdMXAHbBehU7BAHTwGe8YbxR3BhRDsGVEp8MDXq36qWeI J X-Google-Smtp-Source: AGHT+IEiA4CDXCv8OTzI+UG0NobqYhcCCUan0vO2KXfbL7Xq1am0SRp8jHGYihoFGGgtmvDpYoz01A== X-Received: by 2002:a05:600c:5117:b0:426:6220:cb57 with SMTP id 5b1f17b1804b1-4290af0b1f2mr8901255e9.25.1723108888458; Thu, 08 Aug 2024 02:21:28 -0700 (PDT) Received: from [192.168.1.3] ([89.47.253.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36d2718d680sm1246882f8f.60.2024.08.08.02.21.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Aug 2024 02:21:28 -0700 (PDT) Message-ID: <2c0cd5b7-1ca6-4088-817c-209026266d58@linaro.org> Date: Thu, 8 Aug 2024 10:21:24 +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: 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> Content-Language: en-US From: James Clark In-Reply-To: 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_022131_834216_FBA089CE X-CRM114-Status: GOOD ( 23.00 ) 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 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.