Received-SPF: pass (google.com: domain of linux-kernel+bounces-30076-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 18 Jan 2024 13:02:43 +0100
From: Maciej =?utf-8?Q?Wiecz=C3=B3r-Retman?= <maciej.wieczor-retman@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
CC: Fenghua Yu <fenghua.yu@intel.com>, Shuah Khan <shuah@kernel.org>,
	<ilpo.jarvinen@linux.intel.com>, <linux-kernel@vger.kernel.org>,
	<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] selftests/resctrl: Add non-contiguous CBMs CAT
 test
Message-ID: <su2d6i22icynusoykmowfnzery6hlqugovaw4xcrhb7r3ymbx2@aeimvrxsc5xw>
References: <cover.1702392177.git.maciej.wieczor-retman@intel.com>
 <10c3afd7f62c63db31a3d4af86529144a5d7bbf9.1702392177.git.maciej.wieczor-retman@intel.com>
 <fd6acf6a-2610-406b-b363-220121a45aee@intel.com>
 <6v4hgra7bd5nwubcgtbsidmoohrgxdw7no4faa4a54g4vrxo63@c6qnujvwn5ej>
 <73cea732-3757-4aec-a39c-4f0e50f6fb70@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <73cea732-3757-4aec-a39c-4f0e50f6fb70@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?C01U2tM8GamWk7Y+gqwR9gaEW+AjREylhdHDxGQeOfFoSNb30Wqo1EkBjh?=
 =?iso-8859-1?Q?B6JjgozEJNMO7zfSYdxAd7neJYmdN5dUaWpV7PfnZo3satJiBhhqDL7yE3?=
 =?iso-8859-1?Q?VDDdHhyMcwIsx7QQGIaWRqODJCRsPqN58RsLzfX62dfn1GEfLUCdHXl0rM?=
 =?iso-8859-1?Q?/KP7ZsT7H6qC+wJdINgpryLj1grBG5fKF6MrFKzTvWQ3BWbxLLqg0b0k4W?=
 =?iso-8859-1?Q?SVjihX6DQEV9m7s/NWXqBSubz2e2ieggfI4ohkVUPXTc8Pzl659faLpMEn?=
 =?iso-8859-1?Q?vXxs7f6II2PDw9r4rwydAzRdmnATC9x4Fgu1jnpPvsZpB8rQfg6XVNrq0X?=
 =?iso-8859-1?Q?rBeRsap0llc92LP5Va/OGOxQitHxrhnoUo6obh+CDcatVYDgTNNZJWUVF9?=
 =?iso-8859-1?Q?7Gtb7uWEXnwPl5wKtz1MvvLRq3uEKRbRgjJTe8iVVj3iqUEMrh10+bedhS?=
 =?iso-8859-1?Q?akwCq0f6MLcZUh2sOL+R2JsjgfE58BiZyn2+uf5g0fzDns59MQgkDQztWK?=
 =?iso-8859-1?Q?pQ2o+xKMisN3Ewtz+YfL8a5dD5LeVL0nsuA6AeCPuzbHA4lJp1b5BSCwEK?=
 =?iso-8859-1?Q?XcoV2yl6YhqHvl9cDtzR6LW5fGcsS6Y+H1ahaWsjeddv9id8J4q3aWX92C?=
 =?iso-8859-1?Q?It65JDTLvdF/8AhH76NgRN+pLImnWEPlQz0tNjbUGhaf9BzuAaW/6b0Pe7?=
 =?iso-8859-1?Q?KQbFZAXRaeDLpcPuP86T1zpY/dk/rVsKz92KEbNKJjNNEPnrVd5XVrwzbI?=
 =?iso-8859-1?Q?e3+gH2sCJOGVdwDZguIMCwVn0m0p8R3T04PLudBsHC00MqdJNvzseFBgA5?=
 =?iso-8859-1?Q?m/2vvSdA+a9AzmgfVfPVr6YjEPO2fCQi9hN6wM1ClFuayPmqusivL9A/o+?=
 =?iso-8859-1?Q?iRRnWQG6ImuSCR9ISzWa4mMNxlZKZhKG4UGskrVkK9NklzcIG/ns4CqCv8?=
 =?iso-8859-1?Q?SzxpYdcMrRHw7jlPg8WGsaTUFSMGSTDE8/NFCG042PkolOnlZwhtiwbFMC?=
 =?iso-8859-1?Q?g9ZFthsfU2IB6sfHbuN6XTfKBG9nLiq/c+ZLYE3NrgKyZ6o08TeZy353Eb?=
 =?iso-8859-1?Q?NB/PSogihM2ThoBACsxHk3wBr+kklbu4FBa15ar1ZYoFX13ipeQJ4WD8h5?=
 =?iso-8859-1?Q?jJtmXCeOhQgg03Rcuszd6WodasUv3OiQ6BlLL2ad9GObvj6UCCI506LiwP?=
 =?iso-8859-1?Q?zeyALI7bKMk28cPNc5OEvrmzQ1cXvnxFhk+3R4qxZtIYBqGrUTMOk08tz4?=
 =?iso-8859-1?Q?oLO78q/Fd297lxk1MkMhafnDCraShk8o8IEjxbnBk7tMM5HNaUXVgNnTWz?=
 =?iso-8859-1?Q?T+19nbt2uC4sheXFvHTXJiw7OsCE6rarTOABYf4j/YalHcUAxrrcLw35as?=
 =?iso-8859-1?Q?k7oloNxeo21/+XqWtNU5vWgykhDoFosBhz4w1pYYnDwKPzSu3XYN+CcAFO?=
 =?iso-8859-1?Q?aAcg9VHSSHefwAsk9uKm5DF2U/K4yMOnEGPj1Qn18EJx3deEB1U3RVXZE/?=
 =?iso-8859-1?Q?xr2CvmUbgbUZWIPcC4qi8BjW3qZlQdU+uLvq1uSWmmLaK6Z6AEIp3s21V8?=
 =?iso-8859-1?Q?BGRUyWy5ljq2p2G0XlcfrL/WxoA1KgfsFESm0GI1EVRPg9xfnIkM6wTNUW?=
 =?iso-8859-1?Q?rv8JLRfR9OfFD1dE8Zi2RPHXclerFJQ0newOt1MsOid7zVUaaV6dxADU96?=
 =?iso-8859-1?Q?0GBse7kbqohXiE3GDgY=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 92434c2c-ba1e-499a-eb0f-08dc181d66a2
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jan 2024 12:02:53.6215
 (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: dgCYA6NTspAvzjGO7FXc7S8YI5ITEa+CrGBHxlBz6Kk1pioD05jAUG6qug0raSeCwcT/3xCKzHFuaBUbomsLkgWtLBNdnTigYx6rrFxzaKg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR11MB8531
X-OriginatorOrg: intel.com

On 2024-01-17 at 10:49:06 -0800, Reinette Chatre wrote:
>Hi Maciej,
>
>On 1/17/2024 12:26 AM, Maciej Wiecz?r-Retman wrote:
>> On 2024-01-08 at 14:42:11 -0800, Reinette Chatre wrote:
>>> On 12/12/2023 6:52 AM, Maciej Wieczor-Retman wrote:
>
>
>>>> +
>>>> +	if (sparse_masks != ((ecx >> 3) & 1))
>>>> +		return -1;
>>>
>>> Can a message be displayed to support the debugging this test failure?
>> 
>> Sure, that is a very good idea. I'll add ksft_perror() here.
>
>I do not think ksft_perror() is appropriate since perror() is for
>system error messages (something that sets errno). Perhaps just
>ksft_print_msg().

Thanks for the suggestion!

>
>>>> +	bit_center = count_bits(full_cache_mask) / 2;
>>>> +	cont_mask = full_cache_mask >> bit_center;
>>>> +
>>>> +	/* Contiguous mask write check. */
>>>> +	snprintf(schemata, sizeof(schemata), "%lx", cont_mask);
>>>> +	ret = write_schemata("", schemata, uparams->cpu, test->resource);
>>>> +	if (ret)
>>>> +		return ret;
>>>
>>> How will user know what failed? I am seeing this single test exercise a few scenarios
>>> and it is not obvious to me if the issue will be clear if this test,
>>> noncont_cat_run_test(), fails.
>> 
>> write_schemata() either succeeds with '0' or errors out with a negative value. If
>> the contiguous mask write fails, write_schemata should print out what was wrong
>> and I believe that the test will report an error rather than failure.
>
>Right. I am trying to understand whether the user will be able to decipher what failed
>in case there is an error. Seems like in this case the user is expected to look at the
>source code of the test to understand what the test was trying to do at the time it
>encountered the failure. In this case user may be "lucky" that this test only has
>one write_schemata() call _not_ followed by a ksft_print_msg() so user can use that
>reasoning to figure out which write_schemata() failed to further dig what test was
>trying to do. 

When a write_schemata() is executed the string that is being written gets
printed. If there are multiple calls in a single tests and one fails I'd imagine
it would be easy for the user to figure out which one failed.

On a side note I'm not sure if that's true but I'm getting a feeling that the
harder errors (not just test failures) are more of a clue for developers working
on the tests. Would you agree that it seems like users probably won't see
write_schemata() fail here (if the test execution managed to get to this point
without erroring out due to bad parameters or kernel compiled without required
options)?

>
>Reinette

-- 
Kind regards
Maciej Wiecz?r-Retman