Received-SPF: pass (google.com: domain of linux-kernel+bounces-29335-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <73cea732-3757-4aec-a39c-4f0e50f6fb70@intel.com>
Date: Wed, 17 Jan 2024 10:49:06 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 4/4] selftests/resctrl: Add non-contiguous CBMs CAT
 test
Content-Language: en-US
To: =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?= <maciej.wieczor-retman@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>
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>
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <6v4hgra7bd5nwubcgtbsidmoohrgxdw7no4faa4a54g4vrxo63@c6qnujvwn5ej>
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?VzloNXNHS0czeFkyKzV4OWFFU25OSWdYbGdYa2NzYWYyZFdsVFFGMnJnZVV5?=
 =?utf-8?B?L0pkUW01Y1JxMjErZjYyUHhrTjRCdTZldm9zM1owcTFZSURQWjZHNnNNd3Bu?=
 =?utf-8?B?SW1jcUV2cjlwTGJmSURZQU5WZE5MZTVNZGlYYm1kTGRNNXR4V1VqU2t5cVBw?=
 =?utf-8?B?SlRhbVorQ2dTUmQ2LzRoZkJIU2JEL0V2eDRRazgyYy9HMjlaSFdxMGluQ3Vk?=
 =?utf-8?B?M3VTeWJTSkoyVjZELzBKUTZtaVd1TVBFTy9VTzFoUEhNNnZXbXZPUnhuK1Z6?=
 =?utf-8?B?K0k0aTJ2QzhqbUJhRm0rb3pySktsV2pCZFBZUmdpL1JSTFVsN2VrdGtML3cy?=
 =?utf-8?B?SUtnNmFLRVp6aGZOVkIyTlVKK3Fnc1JwMUczcS9GbnNja0dPdzF5am5LT1Uv?=
 =?utf-8?B?SUU5V2pPN3BBbmxMUjZvQnE1eDh1RkFzNU5jOTV6UzV1STJ4QUFuNUdoSDNt?=
 =?utf-8?B?UlZyUk50R0tEYVNRZnAySGlIcWhTb2krVWpuVkhzK3Y4S3NBTkM2OENQZHFB?=
 =?utf-8?B?c25zNnRjbjhHbHFNVng4VUg1cC9RK1dzNTQ1TUZXelR0akU4M1ZmWmJJcVZx?=
 =?utf-8?B?eEVEUG1LWlRPbVZ2SG92U0pxRXZRdTJ2NzdhUTA4V09jVGc3a1F3MXdTVnlC?=
 =?utf-8?B?MGJiYXE4cGVQOVB2dENCRnArNUk5bDZUSE9sUEdXTE9paG1seVhveDRsMWtD?=
 =?utf-8?B?aU5vc05LYklzOEY3ZmRlTGY2ZFlCdDhvcHBUMkwzTWZSS3B6S1pyYUZFOGl3?=
 =?utf-8?B?c2V2UElHY0hSUDNmUy8xTXhqRmhNaXBVazIyZzJhd3lEWmpDanB0c3FRMkt3?=
 =?utf-8?B?YmxaQTdKaDd1MjdrcTZENzlBZmk1ZC9JbXluVFpJTEdBVlI5MFF4RnhvU0d1?=
 =?utf-8?B?QUp3amdkUGF3bUJ0enhQb1krQXhrMWxpNExjMW9lUzN1QWJGQ3FCcXh6cEQz?=
 =?utf-8?B?M3RJaWRnSjFZazVlaVV3REtjbTF6WmdCdWh2a2lyNnRJYzUwaHRmMHgrdVhy?=
 =?utf-8?B?VkU2Um12Qll4Vyt1N3NqK0hrc0VpMjJJb1BBTWVPdTJTQzhmcG1HdS9QeGF2?=
 =?utf-8?B?SHlBQXF3elM3ZVd5Vk5zdkc5ckUzNDk3V09IRnhWZ05UTVg0MXNlaHpwcjkz?=
 =?utf-8?B?a0JycXFtTjBQT0NoU3NTOHVYNHZyK1VvNkp6ay9SRzNpMWR4ejVpVnVWc29r?=
 =?utf-8?B?SzRRcXBTY0pSckRjL2U4T0ZJL2FlSTlUaE5pYU5BY2xwNVROYVd6ZzEvZEpY?=
 =?utf-8?B?ckJoY0RybWxScHNZTHFCZ1MzZkJlWGJROVkzZ2ZYMHZ5Mjk1ZVpkeTNiTUwx?=
 =?utf-8?B?TnNyRVNzWTVCOE52dldsemZjeVJ6ZTFZOHhPK0l4T2l4dkNZT214WUpNZFlX?=
 =?utf-8?B?S0ZQNkQ1bjZEdlA1eWJUVG5FdndLMmx6VVNCWElsbjN2czNhUUxUWHR4TG1w?=
 =?utf-8?B?TmZXcm1nNWxqNEFQbzNJZUp5ejVOVHBBY3BHQzVLZmdLMnNsTnQwMzZvdkRN?=
 =?utf-8?B?NE04eVlQVXZFUHFjWEZZUTR2eHozeGFOZDB4M0JzbmZGTkRBQytER3gvV0Fx?=
 =?utf-8?B?MGhWbXFUQ3Y0N05UZHJNN0EvOHFiSjdNYUdlZk1oUTdFb3BSeGE5NGJaVng0?=
 =?utf-8?B?a2xGSEVjQStSMXpnRWt1S1VzOXd2UWRBUzh6bmttSkZ0RXNDdGNUV0FrSTd5?=
 =?utf-8?B?K1BJOTVNU3VhYTBwTTluYklxc2pqMytqYks1czlsbitZYjk1T3JrOHpTU2Jh?=
 =?utf-8?B?d3FqNzliWEtoMjEza05xcXo4aUV4ZTZ4L0F6NUFqcUx4bHBrNHVlQlVFQ1g1?=
 =?utf-8?B?eWl5MHpKajFaM1hQOFg1K1pFM2pIMU1wSHFRUEsrMVBmS2FkVkJLWnh4OUpz?=
 =?utf-8?B?c2lCbmxpQ3Z3ckxTSkJRMFZXMytOeDdMTjBONUtqK0NPYjUzY0VsZHdIT294?=
 =?utf-8?B?OHlhbk5kWXFiWVF0aHRpcDZHMUsrcmpSMnE2WUpKbmdlYzhXVVE4K3hjSzRX?=
 =?utf-8?B?VkNKS01pSlhINXJ3QkY2S2d6WmlSQUxNTXdONkxNM2kzbzlaV1BWa3dIZkg4?=
 =?utf-8?B?V29ZTzVMRFBBU1d1L1pMSGdqNkk1bUJacFhzbjUwQUNVbW1pL0NGeXIwaEFM?=
 =?utf-8?B?V0c3T25heTk1QmttU05FZDQ1NWhOYXVlMFBXanYzeDJwblZvUGxLbFVMWmdW?=
 =?utf-8?B?SkE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 888ae059-78a9-4b00-f164-08dc178cfc32
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2024 18:49:07.4902
 (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: d/Inqkn9iNXaaofpOn+0yNKOmTJ22gqOX8p8SUwbBTD4ErNi5fpyCRigT9Bwmm02zFtpiGhpwnIAK8CvkNUltsLDZWsLIl8KynVo01zBscI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7367
X-OriginatorOrg: intel.com

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().

>>> +	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. 

Reinette