Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <11a4cec1-8169-0891-b9c8-4d3a7b689d86@amd.com>
Date:   Sun, 7 Nov 2021 15:55:45 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
From:   Ravi Bangoria <ravi.bangoria@amd.com>
Subject: Re: [PATCH v3] perf evsel: Fix missing exclude_{host,guest} setting
To:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>
Cc:     Jiri Olsa <jolsa@redhat.com>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Ian Rogers <irogers@google.com>,
        Stephane Eranian <eranian@google.com>,
        Kim Phillips <kim.phillips@amd.com>,
        Ravi Bangoria <ravi.bangoria@amd.com>
References: <20211029224929.379505-1-namhyung@kernel.org>
 <20211103072112.32312-1-ravi.bangoria@amd.com>
 <CAM9d7chQH+Br6NJhDdjjOdV7FsODS0_Rj+w-UsfzUud27iPNbQ@mail.gmail.com>
 <YYbWUetkc6keL/Xa@kernel.org>
Content-Language: en-US
In-Reply-To: <YYbWUetkc6keL/Xa@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SGZxd3Y3Nm9vN0I5eFJlRHlUOGk3MmtTbWsrT3B0MXRyN1d2ZnNxRStzb1pu?=
 =?utf-8?B?ZEd0Uk9oaCtPNms1UjBUR0ltVzJraEY0U0FBeFVOM0c2Uzg3Z3BRcDdPZ0kr?=
 =?utf-8?B?UkFvTFIxc1lkR0xGd3RzWWdEblpJVGd1VU1ybWQ1L0ZzTklObVVENy8zTC96?=
 =?utf-8?B?UklSdWtQSWptYUhiOXRsZjUyckQ2NTI2dTdaczduMzRUV2doZGN5N2IxZkRX?=
 =?utf-8?B?QmZNQjlxZWdjVUVvbUxMWDhUNVV3QVBha0RJT1FjemEyL2FPYXdoQU5MWDJ4?=
 =?utf-8?B?YjhLcTRlcnkvblFYdHJpU3ZVSVMwWGo3Y0hnRzBSV2l6RklUbCtIU3NRY0l0?=
 =?utf-8?B?RlFySVFMZ2t3T2QrU1N3c0FYUFc1eWRBWDZxOGovWDVUZTRIb1ZpOVlsd2Rq?=
 =?utf-8?B?QXFzTzh4dHVkRVkyWi80TWhacDdMc2JabmVuZmdnTkh5QzJnVkhjaTg1a2lK?=
 =?utf-8?B?clNmVlBvU0l3aER4eFBTUTFhYTl6MEJMWVB0ZTAwUlI1a3pUWWg1eUlCTnpM?=
 =?utf-8?B?blRHamFja2g2cDhlcUNPWTV2bFRhU3JocTFBMTErcU5BeDJURnFEcFQ4RFZG?=
 =?utf-8?B?SVBTQ3pqTmpuQllRdmFUTFgxNjVaTXpPL2hvRVpLVnFrb2JPamwvb2ZFc2RL?=
 =?utf-8?B?eVNLaHZPTzhRSlhxbG9Fdy9RUGJHOCtGSjBJQ3JKaE03ZlRMaG85UDd4ZWhV?=
 =?utf-8?B?cHVGVnE4TDVUYmw4L0VEcUlyVENHTWJEZ1lLcFUwTzdxTm5INEp1NURrT2s3?=
 =?utf-8?B?OFdKOFNSc0ZwMVB3SGFYYW53R3dCRzBIL3pRaUltZUltdHhyMmZlV1dJdXdJ?=
 =?utf-8?B?RkF2cFZMQm5DSStRU1ZDTDI3U3ZHOTd3WkR6c3JuNkJCdHlTQ05JNXZnY3Y4?=
 =?utf-8?B?SE9XeFIvcG16Yzk1T1Qxc0gzc3BGcm9VNk9SMWF1bjVrQ3BRb2lhTWQxNE1W?=
 =?utf-8?B?bkMzTndtbVhyVmxKTzVqMWFkNTZEYmJ4UHNkKzloelAvMkd1Yjd3Um5odEF4?=
 =?utf-8?B?dHpVQkxsYVN4L2luQW5qNTJRSkVGbW1JM3U0Q3grNFYwc3pOWmtaQVZjcnd1?=
 =?utf-8?B?VTRrZTBML1VCbU94OEFUTncrTVdmK1pqVnE0aHJNYlZtYkhLdG5HZU5rZjJW?=
 =?utf-8?B?MGRuSG1JUWNQL2J1aDBpUnBKQWlnNzRzYldTZC9oR3FxaTZnMnpKSUZKdGdu?=
 =?utf-8?B?cUoyVzE0QjFHaFhjWkVVcFVDeDdDQ0VRcTdmZWNRU2Vra3VJMmNIZFcvU0hk?=
 =?utf-8?B?ZW1KclB5MVNCVE8vVjRGcDZDWmhUQ2Z3T3AvdW8zdU5Ta09wemhkS1VwWm5T?=
 =?utf-8?B?ZEdZTmloZmNmUERjbU84K0g4alBhcWRLOFBiUFg2NVhtTCtPMkg0d1pKSnZM?=
 =?utf-8?B?VTJqZ3NxV1BWdUkyYk5vQWFwTW45R01qL0loeVRIK3R5TEI3UlV1WTVUNHp1?=
 =?utf-8?B?bzlKc0JpaCt1K2IzUTM3MUdUZ2I5eEJCZXZKbytzSW15OWlodDFIYjhLb2dB?=
 =?utf-8?B?SEJWVU91YUdlc2Y2WVZSU1J2QklOTitNL01ZbDZQZ3VyOUw5Vm5SelRHTmNV?=
 =?utf-8?B?UjhGSmtibmh2M04rdVpVeHhVYXRDQjZzenBHUXh2aGtsdVVqdUVza3ZWTEM2?=
 =?utf-8?B?elNtVFptM0ZwMFc4SVgzbzNsU2JYV2JYcGdId0ZVMWR2azI4ZjI1TTZ1M2ds?=
 =?utf-8?B?ai9YdkZ5Y2taOVd6WklPcG5RZng4blNZMHgzaEJaQ2dGSXhzZXZsZm8wWUJM?=
 =?utf-8?B?UFpsRDdWdlAzSmdFa2tkWFVtK2x3S2xDbUJFeGVwYk45ZURuTGtUUnZ2QkE5?=
 =?utf-8?B?MWtkdEdIUlVYbkZuTDVmTTJnZ3BhUDEvWVIybWc5YTVna09QR1VmZTd3b3ly?=
 =?utf-8?B?UWlZNUNDV2dsOXF4SE54REJOaFUxc1ZWV1V2OXJJYnFkTmFKeGxnSU5nSWxz?=
 =?utf-8?B?MDEzLzhJYWtpNzNRcFRKTXgvTlRVQTd5UVZHMUJ0QUZHbGdCUGJkTkVEaCtz?=
 =?utf-8?B?QTc4djZZZk14ajdncVVQd1JpZHVwT28raUhnM1hkR0RjK2Ewb081alQybmR0?=
 =?utf-8?B?V1pvMmxDcGtwSW1BNzRPYU8xL1pUOWFrQnloaFZ4VTlBN0E5alBGSnNNY01Z?=
 =?utf-8?B?WEJwOXB0bHI1NVZURHNDK2grQkZaVTZtVDFvZ0FSZFlqQ1lyMk9rQ0dCQ2JT?=
 =?utf-8?Q?fcpiuhewwzTiUKYyi8XnXqQ=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ad470cce-a546-4db0-f5e4-08d9a1d8fd8e
X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2654.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Nov 2021 10:25:59.1906
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: kzAKOhlDyunxurjBDDDd15P+M02nFt0jCIJTlIIZp49YCHjh5QEOGWzs6P3VFAnR0Wy8kJiwM367gsDRTkBlrw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2846
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 07-Nov-21 12:54 AM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 05, 2021 at 11:00:29AM -0700, Namhyung Kim escreveu:
>> Hello,
>>
>> On Wed, Nov 3, 2021 at 12:22 AM Ravi Bangoria <ravi.bangoria@amd.com> wrote:
>>>
>>>> The current logic for the perf missing feature has a bug that it can
>>>> wrongly clear some modifiers like G or H.  Actually some PMUs don't
>>>> support any filtering or exclusion while others do.  But we check it
>>>> as a global feature.
>>>
>>> (Sorry to pitch in bit late)
>>>
>>> AMD has one more problem on a similar line. On AMD, non-precise and
>>> precise sampling are provided by core and IBS pmu respectively. Plus,
>>> core pmu has filtering capability but IBS does not. Perf by default
>>> sets precise_ip=3 and exclude_guest=1 and goes on decreasing precise_ip
>>> with exclude_guest set until perf_event_open() succeeds. This is
>>> causing perf to always fallback to core pmu (non-precise mode) even if
>>> it's perfectly feasible to do precise sampling. Do you guys think this
>>> problem should also be addressed while designing solution for Namhyung's
>>> patch or solve it seperately like below patch:
>>>
>>> ---><---
>>>
>>> From 48808299679199c39ff737a30a7f387669314fd7 Mon Sep 17 00:00:00 2001
>>> From: Ravi Bangoria <ravi.bangoria@amd.com>
>>> Date: Tue, 2 Nov 2021 11:01:12 +0530
>>> Subject: [PATCH] perf/amd/ibs: Don't set exclude_guest by default
>>>
>>> Perf tool sets exclude_guest by default while calling perf_event_open().
>>> Because IBS does not have filtering capability, it always gets rejected
>>> by IBS PMU driver and thus perf falls back to non-precise sampling. Fix
>>> it by not setting exclude_guest by default on AMD.
>>>
>>> Before:
>>>   $ sudo ./perf record -C 0 -vvv true |& grep precise
>>>     precise_ip                       3
>>>   decreasing precise_ip by one (2)
>>>     precise_ip                       2
>>>   decreasing precise_ip by one (1)
>>>     precise_ip                       1
>>>   decreasing precise_ip by one (0)
>>>
>>> After:
>>>   $ sudo ./perf record -C 0 -vvv true |& grep precise
>>>     precise_ip                       3
>>>   decreasing precise_ip by one (2)
>>>     precise_ip                       2
>>>
>>> Reported-by: Kim Phillips <kim.phillips@amd.com>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>>
>> It'd be nice if it can cover explicit -e cycles:pp as well.  Anyway,
> 
> Ravi, please consider Namhyung's request, a patch on top as I'm adding
> this already.

For explicit :pp modifier, evsel->precise_max does not get set and thus perf
does not try with different attr->precise_ip values while exclude_guest set.
So no issue with explicit :pp:

  $ sudo ./perf record -C 0 -e cycles:pp -vvv |& grep "precise_ip\|exclude_guest"
    precise_ip                       2
    exclude_guest                    1
    precise_ip                       2
    exclude_guest                    1
  switching off exclude_guest, exclude_host
    precise_ip                       2
  ^C

Also, with :P modifier, evsel->precise_max gets set but exclude_guest does
not and thus :P also works fine:

  $ sudo ./perf record -C 0 -e cycles:P -vvv |& grep "precise_ip\|exclude_guest"
    precise_ip                       3
  decreasing precise_ip by one (2)
    precise_ip                       2
  ^C

Thanks,
Ravi