Received-SPF: pass (google.com: domain of linux-kernel+bounces-172871-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Wed, 8 May 2024 11:00:15 +0300 (EEST)
To: Reinette Chatre <reinette.chatre@intel.com>
cc: John Hubbard <jhubbard@nvidia.com>, Shuah Khan <shuah@kernel.org>, 
    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
Subject: Re: [PATCH v2] selftests/resctrl: fix clang build warnings related
 to abs(), labs() calls
In-Reply-To: <076d9990-6253-438d-bc92-70eba6dfbca9@intel.com>
Message-ID: <7d4d8a31-00db-c7ba-aa8f-0483c8d93700@linux.intel.com>
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>
 <076d9990-6253-438d-bc92-70eba6dfbca9@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1785506523-1715155215=:3164"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1785506523-1715155215=:3164
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Tue, 7 May 2024, Reinette Chatre wrote:

> Hi John,
>=20
> 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/testin=
g/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,
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 int ret;
> >> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 avg_llc_val =3D sum_llc_val / nu=
m_of_runs;
> >> -=C2=A0=C2=A0=C2=A0 avg_diff =3D (long)abs(cache_span - avg_llc_val);
> >> +=C2=A0=C2=A0=C2=A0 avg_diff =3D (long)(cache_span - avg_llc_val);
> >=20
> > 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 Il=
po
> > pointed out that the math breaks if you do that.
>=20
> To me the extra abs() was unnecessary anyway ...
>=20
> >=20
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 diff_percent =3D ((float)cache_span - a=
vg_llc_val) / cache_span * 100;
> >> =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D platform && abs((int)dif=
f_percent) > max_diff_percent &&
> >> -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 abs(avg_diff) =
> max_diff;
> >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 labs(avg_diff)=
 > max_diff;
> >=20
>=20
> .. because it is repeated here.

Yes, there are two *abs() calls in this function.

In this case is okay to remove the first one since it didn't remove=20
absolute value completely, whereas in the MBA/MBM cases v1 removed *abs()=
=20
call entirely which was wrong thing to do.

I explicitly noted in my v1 comment that this CMT change is okay but the=20
other two were not.

--=20
 i.

--8323328-1785506523-1715155215=:3164--