From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750857AbdAOCgc (ORCPT ); Sat, 14 Jan 2017 21:36:32 -0500 Received: from mail-by2nam03on0050.outbound.protection.outlook.com ([104.47.42.50]:43808 "EHLO NAM03-BY2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750767AbdAOCg3 (ORCPT ); Sat, 14 Jan 2017 21:36:29 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Suravee.Suthikulpanit@amd.com; Subject: Re: [PATCH v7 1/7] perf/amd/iommu: Misc fix up perf_iommu_read To: Peter Zijlstra References: <1484019227-11473-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1484019227-11473-2-git-send-email-Suravee.Suthikulpanit@amd.com> <20170111115735.GI3107@twins.programming.kicks-ass.net> CC: , , , , From: Suravee Suthikulpanit Message-ID: <8d529523-21b4-d917-e83f-ed616a29083c@amd.com> Date: Sun, 15 Jan 2017 09:36:10 +0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170111115735.GI3107@twins.programming.kicks-ass.net> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [58.9.54.97] X-ClientProxiedBy: SG2PR01CA0058.apcprd01.prod.exchangelabs.com (10.165.10.26) To DM5PR12MB1450.namprd12.prod.outlook.com (10.172.38.139) X-MS-Office365-Filtering-Correlation-Id: 919bf99c-31a7-47ef-2f3c-08d43cef4e36 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:DM5PR12MB1450; X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1450;3:V7zmzXegq0t1yotlsILAxIcObK7RsQcM0sGxvnoKMp1kuj5DwQEv2uWDKJXvsht6nEUIFzpP1zSFrmyRUPA0w1eHu9TfA3PoE5QD1U/QgGkV+BZktFjcrKbfP5+WCiHgu451DaxGe5YWDyFyKSujJ4ztMHT61bDpRxdQKOtPqyyYj1x6VvUVP7kXuLV4wV51X21vqzxcy7SeIKyw+1Xxxm8fLMiW9RSd1BBoKInAwez7ZW8PNbUH9oIdYrxFqED9BDTqHvJraEFvG2QxppYB3w== X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1450;25:ra7PHJxrtqYunSowPHUb6ZCWZKR6yyVXPS8id2rHasJc2//w5dVZiPNik0vxG/ed24glFSU64WDTrjQ2X57kmT+/D5Z+L83s654p+Gq5gf/S1LlAwdE/Oq5ld/LPaLCokZigj+mp2znZAOzFe5xUZHmk0XLYjXOCc62q/DmScWRy2UzjP4uZWrbtSEzGAteLK4vxMrXOnyFiFlQMtwXkXe0VcjY4bvZsC+tFH5GTMHAIQJP0s0t0k/8T2ukvYEhIf5tCWnbPKimSX/+dtsRUaa7A6veeyzJhvpOyfb+imDchPsCV8UUa/t9GVYoczPgKo+o37dt924xBgUQF4ADs/F3iZoxy+H8XK3FwFcKcFspZViv/WIZoGDhWmGQQCMlyuIhh9Aqba3yoOKGppB+lWzrK2TVqkIwb1O1hZvSxZ4wC3b++NIaAGFLYKWh5zUtF4RHdFrU5s9rHY/7kRKYR/aQdKf7kLy23KtX8rXsT4bUbLhjJ1QYtnPJhMgR93q1suTSrCxCsYY9bHuyOARVNZz58tuYy9pKtwQoZ7GajHPQ14RwAp9+J1iecOCejOmwfSCnv8ZTxMlYunEOxh8k2oVHHDYw4sA9T9Ie7r/cO4kSnWMAF1tcPF3u3YpeicODh2hP1x6HNygW1Wvk9ZIShzqhefKxrNTIGOh0BlEa6s8I9jIHZfNmsiXnGEVpbFhXuNJua5hMMnXyvvnmenCJn7mf11L8OEGDRFwCXFRDHts/Cku7vi2vyFPomSg2Tot/YQ0XL9vTBhvA6Gv6duILdgbcKnL00OoZQXgxrvpvA1kFFrCKFVe7D/C/OZUQnyvadwpNuavAE7HgOoSu3Zw2z8fZDlwwGwTfe52DYcaAt0S39q12u1RfQF4Rb7A3Zn+oJ2xd/elND6F4d3r5dy21qeg== X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1450;31:0h4elZfBbomTEDBapdkPlMACiVFqLWJ/s2Q2pmkIMsyd29q/OwsxEDXRI2fEs3yYteZ79aiTmf9KiTFPAtsTporvphbKLfhxu2Z/qRt1lTEqf2V/LHPTGWYSRjZpqafgss6BEOLQzRpJzu0nJP0HTEqBJz/4NAtnCv5jc8IQmtpvr88jOjAEfcTeQHmSCJCcAWAVPjq5cLviLL7rx4zT1qYQ0MOkt8ibRjfV/XYwZRYd8d3wufbYJhotoANFNyRMrwDxcSOx6gE9fHmCi96J8g==;20:77SE/4bDcrUbYjIENhtS4D2csaW6sLtCT7oOGCd4gPtsoClcCqsSAkMgf57W0hzodtaFOeDePI0nl6TRF4DrojMNb+U0Zqt7lQQlvMdmgomqSRvZaDeOQGRMPRug9TVFTvpwFIdyIto0rXSEdHZ0LiQ1l/Vq/sRiicumrfTxJE1kPJw1ec0SXC2xKlVkqv46OpTXOQv6wuPOZQG/v0WOtxAkyF3TzPwJqaYWFZdc7ZLHX/o7SjycaqJDDAdFsasCrLZPBs/HhxYRnDI4oBcRjSTiS3WkpYDfkI+3i6sre4y/9jqNu2+2GTBqAsp8RTLme0e//+Dzy3//yRWvM52I8xIztW70oPjcyLnq4WgvNXZQ9XgjllyQkKDo31kzigba0ohUwPqyy40qAT+PM1+g+wM3G9u+tdw700KP5AGeL7xY1vDJ/YWQV8uVLJLX8emOddeCXmKsXtMPP1q3ikkXS1p8SoEYCBRY6MFTqxZSaRaEUDTJh7Ifg6fsnMAA76Mg X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026)(6041248)(20161123555025)(20161123560025)(20161123564025)(20161123562025)(6072148);SRVR:DM5PR12MB1450;BCL:0;PCL:0;RULEID:;SRVR:DM5PR12MB1450; X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1450;4:A7nTHJ5aZBtCsaqHwsRFWcqowpPHP3WMfVW8cl7unIZp/ofvod3rMbj9sU4LyXmYGFmvQzG3hs86UTtN1kKsW4E8xKJMsPA/O/RCQbSGIwCuE39HVSDGuLc0xwI8rbDj8qNrMFbEXl4ArUNjagxQPXBibnx/TkJwpwt6jwiM8iiQ43CS7gqVlV63Al7z7aDs445lqqNAyk66C/s3DIJ+YUKVRLDuwQFyEEwXdy8p6OSwJ7ClCOXjyS9aLmX+u+pAjD2Efvdhg4nL1qtY0HE40XgRtTXY3snBBziaFrKgs6Xif6W7PqV3YMWc9xJJuotwvjzulLyzqcTNFdAbSTeHu7ezIv18LD55X6j4n0fm56Ds6Z/psJ4Gzqta/JG2KsCHIWvHy4Ca+QfTKh54P752OjvCdJjd6j1iuYEAOeINRtcnDYYaWPDC7YmGSrVhdeJ7oeeMDLtx+BAWBjUlu77w6Kbg1QVKpQ8rRi345npOC08MOYPkXUhoZGiMm8PL2ngl+zLRYVv10ITARybDXsAJh9A2OLHH9pGi1jd8rkuxrAUeUQ6q7rIyqcAltdkrdyN6VFMbKSVc+SuwX6lGKkmyqYMev5/I1T9Y1ADPZfAF9Ao= X-Forefront-PRVS: 0188D66E61 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(7916002)(39860400002)(39840400002)(39410400002)(39850400002)(39450400003)(189002)(199003)(24454002)(5660300001)(6666003)(83506001)(2906002)(38730400001)(65826007)(110136003)(229853002)(27001)(86362001)(31696002)(92566002)(4326007)(2950100002)(6116002)(3846002)(6916009)(230700001)(6486002)(8676002)(81156014)(23746002)(106356001)(6306002)(64126003)(6506006)(42186005)(7736002)(31686004)(50466002)(305945005)(36756003)(33646002)(101416001)(105586002)(50986999)(76176999)(25786008)(54356999)(6512007)(189998001)(65806001)(47776003)(65956001)(54906002)(4001350100001)(97736004)(66066001)(81166006)(68736007);DIR:OUT;SFP:1101;SCL:1;SRVR:DM5PR12MB1450;H:Suravees-MacBook-Pro.local;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1;DM5PR12MB1450;23:pUGFoKM52cfXqhR1pWGNqo6KAxvPFIHaZVI+b?= =?Windows-1252?Q?v/XVT244jmgyiR8TTkIPDkX/lBHrhGquaI3v3+UIuH/xO8vap9fdpGXs?= =?Windows-1252?Q?De/dQaY4Of8pSxpw52EtzjVRx060wXO6m2ADA6HbG81crYd5aUHoO9o5?= =?Windows-1252?Q?y2xXGf206F/sg13mgTewzb7dfRIIT49b62fJDNcEFc495B+I9+/hHQ6n?= =?Windows-1252?Q?011c2wPlunXbT+MBecLwXMWimfo3u6/bexzsMi/lVnQvhfQaaZiof7Tu?= =?Windows-1252?Q?WvZhsqI521Uf7IBQZappSW4uh67hWq5TEgcCj4TvHVJlFbk12dd9XSdk?= =?Windows-1252?Q?LUNDHzoKE2tRM1NO5Fa40roFVZrYsXvvGeyyNOFyGxB71QXV2Wv7XRE7?= =?Windows-1252?Q?EKzdscplhJraRZ/PgYuwu/HZWJl30y7y1bSsKNlDLQDFId99OA3cApsC?= =?Windows-1252?Q?xyXiF9vZwHAS8em6tzmuQ9IVwobTR4ntlBbmtBrdZmoNhMaNU2BVaRuq?= =?Windows-1252?Q?9yq8mYzh/6QbMHlM4TujwvqnFQFMezntl1mgBq21A5JjKqmtDSpM/8zK?= =?Windows-1252?Q?S18PHdOnrFObxOV61lUk/b66Dd6OO7EuhkGLLdIWwJAJZGjfzVQ5h5N5?= =?Windows-1252?Q?lcZs4cEiSezAjnHBKBjSbYODl5SEAxpEHs0VYHSDQmR3sWDlnFHpFDn/?= =?Windows-1252?Q?UPnP4kjRxEqyRE7EprBQY1ApHpjbB/9GiQ94wY8WDa3ctyfqWvilAT68?= =?Windows-1252?Q?YHnyBDu2gLCUUziXv3Xf89mjgHg/cwIa/TB0ZOKObODvq1goaawi7rHB?= =?Windows-1252?Q?NayJt/xCbbVyJ6JFImVse50K96zybERt/yaZlleCHY1gYSRY2Uif+jAa?= =?Windows-1252?Q?llQgjcKD6zOGKrdqlTKwrXIyUh2+Ov1fmmAsp/dU2R1ySE1EjmglClaQ?= =?Windows-1252?Q?6g6Wzk+pFYzQmymCf6hoV+B3YgFeYV4CvXUvJd9T3GplFwXbnzVkeze9?= =?Windows-1252?Q?vAIyAHZScuOxk1+ZQF6iVLVLO6rxASIoetzcViQyPt3hIC6XHobd/dh6?= =?Windows-1252?Q?4SrOqj+Ue3/UDHdfMoZG9df7cBHOkZcP1WBtDOKg/YgFlXWM5Y2qxEJ5?= =?Windows-1252?Q?+AzAWe7Y8YHqUjqlpLpsICSRlV70EdZmjlA5UvQe4HbZbeKWladoXod1?= =?Windows-1252?Q?SRExrxkVoIFhlV2qfxqXGhw28uQ88Am49OaoIT9Rv2748w3ie5f4tDKm?= =?Windows-1252?Q?WVbr91waTvz1p6zAJNdFqWmk/whKZbP1mMd3xkC+r3HSpj8A8Q4JLDDn?= =?Windows-1252?Q?sG+fPtfXxZasX58oTLMfJOMOY2TSh7yflAU1FvqGP8tCU/uCmyiE96b2?= =?Windows-1252?Q?EhMCzreigFnlgO+4T+jADjr9bj7c6TpVOfzJuX3OiPzmHXEBLBSkDxhX?= =?Windows-1252?Q?foqr7DPwWtpoCQEQF+5Nke4sNuhiWn+3n1YNWWGg+CEKBffSe6HbNOKS?= =?Windows-1252?Q?8H+ZtjCWg47EmV25/9PSXPubLWOAke5OH7tdiT6TzZdTN04o9BegopN3?= =?Windows-1252?Q?3Xjp63HPxZkZkdAgqQiWqttu5RdVLKHcxe/?= X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1450;6:m/ffIFM6mWRt9tg8klllThqzhGpH+N9rm5TzU9Yh+qhqzsFz4LzsKU0chNWzQ8aCWRSbPgRKIsrpcJ42jzUpSveVLM6NrOU6mSGemPNtrofMr+ElaEJ1z+xGI1tp9inL62s7rnNzAdF10RZvEVfAWFhfE/7oYb5Mle7wixT8CKoSXgJRgvF02Alm15QVo9Q1XVce22Yg904LVzYsewiw62I4/YePi9xtJDSSpys9fnu9x31RQ0+ftBi3nz0pLg4pSf+u9IY3pecOXXNv3b9IVL1e+rW//4dY7TE3T0lwAOYZB9aNiQ1AOMiE5FPj6UOi+UMWIlJagjhR1Et1/jRZkwftBvyGxCkhHNlugcYRh6bgC5HSZxZ1QBAusA+PwAre10tV64CLe1NCpQoRIRZQaj6v0vYVpxvrClj6SKl2ORNkpk4lphEjkMbUAKnm8f2iKB6igIJv4mm89wePoHV5jA==;5:Auy1CCpG/cs3Ht1eP1f1wA541LnznUt4BgueV4OtslUh2UBKffhOd24zVfWoZ81vIYPu0qyzLqPvZ/hQMNDY4k5ySP39LP0KRKrXkIudFHA8Iat2iBGBvs3CqAmRYAF87kggvki4PTC0j+FbenEeun1gNH6wWIyehiqaVYLszAg=;24:lk9guDpfVohBBod4RO/Xp/l4DJgWCEOJLMu/tnzBayptEuO78dluLQAkzzgx+rpIz2ogq08nZBr+W1h0opbCV/HMkfjIChtA1/wgMfBdq58= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1450;7:TocAKpcI4dew0qydZ9Pce0v8o3hjl4kg+1ViwapUAJ3t2JtQ7qkDokq3l28z2m+9m1RfGiX5j1TnvEs1OmMm3j689C0jZXSP1MVS0aI8PPHas3zVcHK8ONH9dPijJ4rNq0f2GPmxR297ZR+aAocEFdjTOzc1Mo740QHoK0JbciqoADEtMB4sJlOLW5YUU6by1+DADYsLTToR39lYbofMchuifk3HPjloxrUmBeNH+dgwAJQFUsA5fQpUZ+YiBfWYiTgFs+imZymvORsF7W5zU06TE2jN4AdaObk0S5kI0jytG1BWCv/g4RxaJgyDTs07R9p4MXr98OW6/ZXaX43JyoZH4Si6jFX2CAQ2vJcjQphfhJjk4/vMjEVKyf5ClokDmYATx7RG9gfLVByHEFFa+88QccHH6p8gvwdMEs/nRN0tVgIVGnY8XIDqcW3+cjCYTbh5tUPaac2KBawY3ehgzA==;20:vhe0YaLYnvFEVHCYIvImJTWhotoRKXAZSYOWpbP8aLiI45r1kT3nah3mMcMZiItT29PVRVjZMf2n+RqSJgUTUCOq5OhR0Dshbo254RYRKLrUOHy8AIPBAhUzuVrwQfVfFYnnCv0Qkpb8npe+lW9JOcI0/Y6wN4lfF8kuJSgs1n3Ql76y889czZHt9iN0I2K/K4SW7OAIzOE8NVyoqAFBwrUakX3B0Q4PBaFfUFikNhvkvOYpoq7UUV485LxuwZ5W X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jan 2017 02:36:25.2819 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1450 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Peter, On 1/11/17 18:57, Peter Zijlstra wrote: > On Mon, Jan 09, 2017 at 09:33:41PM -0600, Suravee Suthikulpanit wrote: >> This patch contains the following minor fixup: >> * Fixed overflow handling since u64 delta would lose the MSB sign bit. > > Please explain.. afaict this actually introduces a bug. I'm changing the u64 to s64 ..... (see below) > >> diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c >> index b28200d..f387baf 100644 >> --- a/arch/x86/events/amd/iommu.c >> +++ b/arch/x86/events/amd/iommu.c >> @@ -319,29 +319,30 @@ static void perf_iommu_start(struct perf_event *event, int flags) >> >> static void perf_iommu_read(struct perf_event *event) >> { >> - u64 count = 0ULL; >> - u64 prev_raw_count = 0ULL; >> - u64 delta = 0ULL; >> + u64 cnt, prev; >> + s64 delta; .... (here) because we had a discussion (https://lkml.org/lkml/2016/2/18/325), and you suggested the following: Your overflow handling is broken, you want delta to be s64. Otherwise: delta >>= COUNTER_SHIFT; ends up as a SHR and you loose the MSB sign bits. >> struct hw_perf_event *hwc = &event->hw; >> pr_debug("perf: amd_iommu:perf_iommu_read\n"); >> >> amd_iommu_pc_get_set_reg_val(_GET_DEVID(event), >> _GET_BANK(event), _GET_CNTR(event), >> - IOMMU_PC_COUNTER_REG, &count, false); >> + IOMMU_PC_COUNTER_REG, &cnt, false); >> >> /* IOMMU pc counter register is only 48 bits */ >> - count &= 0xFFFFFFFFFFFFULL; >> + cnt &= GENMASK_ULL(48, 0); >> >> - prev_raw_count = local64_read(&hwc->prev_count); >> - if (local64_cmpxchg(&hwc->prev_count, prev_raw_count, >> - count) != prev_raw_count) >> - return; >> + prev = local64_read(&hwc->prev_count); >> >> - /* Handling 48-bit counter overflowing */ >> - delta = (count << COUNTER_SHIFT) - (prev_raw_count << COUNTER_SHIFT); >> + /* >> + * Since we do not enable counter overflow interrupts, >> + * we do not have to worry about prev_count changing on us. >> + */ > > So you cannot group this event with a software event that reads this > from their sample? Not sure if I understand you point here. When you say sample, I assume you mean the profiling mode used w/ perf record. These counters are not supported for sampling mode. So, we only perf stat (i.e. counting mode). Thanks, Suravee