Received-SPF: pass (google.com: domain of linux-kernel+bounces-172606-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <076d9990-6253-438d-bc92-70eba6dfbca9@intel.com>
Date: Tue, 7 May 2024 18:37:42 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] selftests/resctrl: fix clang build warnings related to
 abs(), labs() calls
To: John Hubbard <jhubbard@nvidia.com>, Shuah Khan <shuah@kernel.org>
CC: Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers
	<ndesaulniers@google.com>, Bill Wendling <morbo@google.com>, Justin Stitt
	<justinstitt@google.com>, Fenghua Yu <fenghua.yu@intel.com>, Valentin Obst
	<kernel@valentinobst.de>, <linux-kselftest@vger.kernel.org>, LKML
	<linux-kernel@vger.kernel.org>, <llvm@lists.linux.dev>
References: <20240503234051.21217-1-jhubbard@nvidia.com>
 <9ae11dcb-62e8-4361-9f78-971d4c6e6054@intel.com>
 <997d7fe0-46c8-4b38-824d-083ab29f54ce@nvidia.com>
 <d67f4f57-4e9a-4715-b6dd-7b83a240b7dd@intel.com>
 <7dd4b09e-b9ba-459d-bfa4-150e712f54bc@nvidia.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <7dd4b09e-b9ba-459d-bfa4-150e712f54bc@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Vk9FemVJaHpvYmxRRXNGeFNkMXB1RkNKLzRkaGRBK090UlpRRW1xS1BzTEdo?=
 =?utf-8?B?UFdoYi9wVHlxYWI0dDVCTWszWTIyUEc1RVVNSXczbmdqZ0U1MTlvZHJaY1Fz?=
 =?utf-8?B?cTZlaTZOd2pzWHptWFBkdHJldlhqN1BkYjFIVFJxRGlhVXpPa0V2VFlTTEJy?=
 =?utf-8?B?T3dmdWRFbGZaRGswdi80VzdwalppVjlCbThnbXFrZGhqOExicEw2bC9nVXlU?=
 =?utf-8?B?Z0ZGVDkzalUxclgybURwZjdlZnVCbmN5V202R0RpMUdvWFRXS3lldCtmUy93?=
 =?utf-8?B?b3hDcGJFNWpBTTlUMGZreHZKUEFPN0x5V01ra0dqUERka0Z2WnliTHA4eG11?=
 =?utf-8?B?MmRKWmZCVytaU0hkTnBIRXhtRzBucWpsaHd2K25PRnV1OExsMzhXUWRFMXFC?=
 =?utf-8?B?bENCRzlCV0I5c2FSTjBTYXpnUU1GaCtqdzRoRGlDQWh4Sk44N09pVnRpNGt0?=
 =?utf-8?B?eHZBUk5aeFlVM3VYcFljVlBHQWU1M0h6RGw5c3c1Q0dseHJoNXlWK1hXbHFE?=
 =?utf-8?B?RWJvRmxtTjZ4WFNURERUSmc2MFJwRk5ISmR1SWRQeGp2QW5hN0pnaUc1RDZL?=
 =?utf-8?B?ZHVjTENQbE1rTnI4dnVmc2JDTUhuaGpuV3ErSThZS1RpNnExNm0yK29CcDgz?=
 =?utf-8?B?ckpvTi9SZ2xBSUluakdKTTkyNUtBN0h4dTUxQlZyc3dUUEMvS1Nyby80Yzdm?=
 =?utf-8?B?OFFYcTBuMUI0emZVYndmdk5ZWjVqbStZVDlCSnNPWWptTnJ0bDQ2RHUxa082?=
 =?utf-8?B?SEVacVhMWGhzcUNlbi9GWUF4ZG1rZkovY3pRSUgwVzNtRXFiTzNTUThqZTZm?=
 =?utf-8?B?QlM2Q0N4TXFsMXpWejJscE1taHR2Qy9EUm44SldDQmZOOGtESW54U25pL294?=
 =?utf-8?B?anhGVllOMk40SnVHZUh2YVc2WlZXZkswN2JSTDlxUnVabng5aXllUXduN3BB?=
 =?utf-8?B?eFIwQjBCa2lONnAyUDcrM09SVTdwdWZBM3NGQTdkRmNHNTZ4YkRUdS9ycUJJ?=
 =?utf-8?B?K3lYVWpONWRsS2g1Q1g1TmdpdVhsNjk0QllGREdrMjFRT3JGeGhvZXJSZkMz?=
 =?utf-8?B?VXE1UWQ2U1hsMnpETnh3QnA4STdIbnoxT0Rkc1FwNFA0RU1BMUx1Yy9rU3NW?=
 =?utf-8?B?UUtHMzlLTzhYOFZRMGxic250c2RTWlhuVGlnYWN5OTdxMDZuRUk5Z1dlT0Nl?=
 =?utf-8?B?dU9rNURDcFFhSDhaNzlNczdUNU1FV0UwU2FDVG8yU2x1eWIrSXMvTUZIZTNX?=
 =?utf-8?B?VlZHcXh5eU0yalliRW5TdnA2S2tvaHp5bExlZVBTYldoTHVZcDF2cUNSUHBy?=
 =?utf-8?B?Wk5JTUhVek56OVRCQnA2L3ZPRGlWOGJ6L1NLaDJhZTkrMkhWajNpUll4QTNN?=
 =?utf-8?B?SEZQcFNoM0IwNnV3djNNTmNtaVZMUVZsYTV1eUlzR0s5WEwzenVpeTdmb1NY?=
 =?utf-8?B?eVQvd0RodllEenJHR2xWRXlGdUhhZCtDUmYzdHNUVEVWN0laRmxaYTYvbDRV?=
 =?utf-8?B?SndtanltdUpLY0hWMFFvK1hidnNSbFI4UkZNMGRxWHpETC81VjFYL2czWklD?=
 =?utf-8?B?TUVVN0pMUmE2VUpvL3FwMUZ2Qk5FWFEwQzFib2xxUERMbm9qWWw4RGNXZHdP?=
 =?utf-8?B?UFB6U3p4VXg0anhud1VrYjVreW9iNFNsY0RwY3NaK2lEMUNycUxjTDl6aENC?=
 =?utf-8?B?Ylh2eWNWdEFIeW0wTzhMZHY4UHJnMGJtKzRLQnBCdGNidGFSMXlJZEdjcmlW?=
 =?utf-8?B?WDRrUFNHZTJ6UUtMaXBLK1FIYVEyNEZVM1VySWhDUDBRaDdBTC9HSHRVdjVu?=
 =?utf-8?B?dWVUMEdMRnlSeC81bFEyNEFYUEFuQlNzWkxrOTdmWWEwaUl3d1oyclduWVpz?=
 =?utf-8?B?Y1FDeldpeStQcHRXbDRzNXVvNzFGUGFjUENlNnBsMFk3aHI0TUxSVjJlSHJX?=
 =?utf-8?B?UCsvN1dWa3pWNUxrTlN3dzE2WVI2di96RUZ4eDRXckx1L3lwdHRQUTNXKzBy?=
 =?utf-8?B?YTlFV2pKcWx0alhzVm9ic1RJSTg0SlpXaGhYd2xTd0E2QVkreEtUZGNNY1lZ?=
 =?utf-8?B?MG5UNjJTQ1pYM1BHRS8vMENLUmlNRlhTZm1EZXJob3VhZ3UyL0ZCc3gwQ1ox?=
 =?utf-8?B?MjNWUTJqTSs0OFBYcS9hS05Zd1phUEo2TUg2V2p1dFltNUluRnZLSnBLYXRm?=
 =?utf-8?B?T3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 60557a27-4687-469e-64c1-08dc6eff75aa
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2024 01:37:45.0878
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zI2FoHvuaICe6CTIh7aR1ydid8K7r6B4hLwt+JVXuH5+gthnENZH82m3ENk6b7QtzAQ+61wKjnwxBDXFxPr4PVB+WgdUsz3Z1NV1cSW+a2g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV2PR11MB5997
X-OriginatorOrg: intel.com

Hi John,

On 5/7/2024 6:25 PM, John Hubbard wrote:
> On 5/7/24 6:21 PM, Reinette Chatre wrote:
>> Hi John,
> ...
>>
>> The following (what was in v1) looks good to me. What am I missing?
>>
>> diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c
>> index a81f91222a89..05a241519ae8 100644
>> --- a/tools/testing/selftests/resctrl/cmt_test.c
>> +++ b/tools/testing/selftests/resctrl/cmt_test.c
>> @@ -40,11 +40,11 @@ static int show_results_info(unsigned long sum_llc_val, int no_of_bits,
>>       int ret;
>>         avg_llc_val = sum_llc_val / num_of_runs;
>> -    avg_diff = (long)abs(cache_span - avg_llc_val);
>> +    avg_diff = (long)(cache_span - avg_llc_val);
> 
> This deletes the abs() call, because I incorrectly let clang's warning
> lead me to believe that the abs() call was a no-op. But both you and Ilpo
> pointed out that the math breaks if you do that.

To me the extra abs() was unnecessary anyway ...

> 
>>       diff_percent = ((float)cache_span - avg_llc_val) / cache_span * 100;
>>         ret = platform && abs((int)diff_percent) > max_diff_percent &&
>> -          abs(avg_diff) > max_diff;
>> +          labs(avg_diff) > max_diff;
> 

.. because it is repeated here.

> This hunk is OK.
> 
>>         ksft_print_msg("%s Check cache miss rate within %lu%%\n",
>>                  ret ? "Fail:" : "Pass:", max_diff_percent);
>>
>> Reinette
> 
> thanks,

Reinette