Received-SPF: pass (google.com: domain of linux-kernel+bounces-30856-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 19 Jan 2024 08:37:25 +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: <wvehk3j2dcf3lkuantb7shefymxj2mnzv4pgbiqbk4jkmmzeid@lurk3dam5tb6>
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>
 <su2d6i22icynusoykmowfnzery6hlqugovaw4xcrhb7r3ymbx2@aeimvrxsc5xw>
 <05db3bdc-9d54-44d2-8aae-07d66053892e@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <05db3bdc-9d54-44d2-8aae-07d66053892e@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?7weSkVE78NIxdDe0hJSj2FgOVThhret3LAjj0a4baEE2VgjuZWVW8JlXVs?=
 =?iso-8859-1?Q?65D4YMNLHEFwwFt1OemAmjNHhJKTTGI/80+pFmoWSM/vNAtm72s1mLGAhb?=
 =?iso-8859-1?Q?TCwK2Fk8U2emq+V06ua0abep2BTJbaw8BGNV9Yf63STJuRPgfYuqYNsovR?=
 =?iso-8859-1?Q?S8Ob47jWTIvLtq0PqRoRP+KVXfpeciGXQTHUbFxjPUUcjfqXnxnFfCroLG?=
 =?iso-8859-1?Q?VYV9pPFNnH3rN77hN22raF0FsISfE4DoBENlbziJfMWgKpkGfsCFxjhqYJ?=
 =?iso-8859-1?Q?zxcX3J14uIXmho5ar0ZCqOMNgB4sBFTD7fqW2mD4qvZDjAk7tsqs0XDcnb?=
 =?iso-8859-1?Q?rfbvw4u16GbyDjEmQCJKD6PhX1AB26bh3TcijTAfGwZNTb+8r0fkssB2tK?=
 =?iso-8859-1?Q?7aOWauqUVglj1oVimEcTRFZJvG439UGRNOZOCDg5mN5lhXErOPX4Usi0DF?=
 =?iso-8859-1?Q?8z11w8uyATE9qCiCpUo67AxUJ1ADbEaDrGwNZS1AGBGw54owJ6s1wdBMYm?=
 =?iso-8859-1?Q?4QTmUNCMuvg/UEjqcVAxpI/1EXxAKqrTlqXhhoEgJBHnmGFF/lkyxA9daj?=
 =?iso-8859-1?Q?RodWIIAvWbQK+ZAewYElpvqkA5UH+ZcnrahFC2GsqOPt5Y5E1M0gJ2/EWk?=
 =?iso-8859-1?Q?EejeI4eI4q+n8fAK09Zyfgz6HhMU5p5g6/50QpvVP+DXdgM1RwpNUubKWw?=
 =?iso-8859-1?Q?fQokL+20+edpynIWuNprnBhC7jMRaWw9A+z3ZjP//51TLyO7bQJJeyl6jJ?=
 =?iso-8859-1?Q?U3zE9/CcDS3BtJ+6BK/z3W79ocggHr8K/lOQiVRpSXHhh5gAqXR2+iCK3T?=
 =?iso-8859-1?Q?r/7pu/WlNljrv37dc4A9g5ofC5dgbmpxiCjOYcB82shT512/1fnxDjZJVf?=
 =?iso-8859-1?Q?AJVPUqxvyi9aEpwFuyp9OTjEICHm4ttIrUaPv7Z2FKkF1YnUIYrbscrTMj?=
 =?iso-8859-1?Q?GdDSlDEJ0riBB45CONKI2NY0jqstSkXXN5MIjwuHLmSK+nVGbUtsPHxjvy?=
 =?iso-8859-1?Q?Q9bqvZSIbHfvJnpopwBNbDaqACFh4xvQkGadTND/haeV5JqzLIvV3eJrdo?=
 =?iso-8859-1?Q?FZIVPbSbIKyohTCcHUxP0JsLUsnUGoKt7ZQtCUNWZ9eALxdTmiyJRdg0p7?=
 =?iso-8859-1?Q?na3/IOxsyve6m72QAQ+pzYK4T2Lz2Yn/uYHegym3lLyx9JKtpb6FCZjRDH?=
 =?iso-8859-1?Q?hQP8Wx/JTupFXWnb6upVIlE1bVzrMAhR86zqhhCHFaLFzn3/YJzj0SZXdk?=
 =?iso-8859-1?Q?sdXUGfFDOGgIOR4tMTuFdULmXCxa2eb1rw9IaCgHATBRodrzYfe/0SK0n7?=
 =?iso-8859-1?Q?0lRXGWa+n9HCQ1T0/ygVnogDWuaRkCWVqhozJY/+nB0+qsVVBN6RVQ417y?=
 =?iso-8859-1?Q?R0kRRqEMQKPzsJAXPXF/iclONQDLgbyeuXqmPL269+GW3XSf/k4M65CLSn?=
 =?iso-8859-1?Q?aW+CjRBroOjH1Eps5Vfe8qfNejaPZkrpZ/qPffob0PHRsvAUtgd+ZV5oEG?=
 =?iso-8859-1?Q?rSdoMue0huESsRMRpU97ct+Tj18BclYuf+DDlcrVxaDm1tEIOZsLSpsvMY?=
 =?iso-8859-1?Q?qURKOVZa+Lr/mvXSNXngvcUVUjCuvoARvaXetm+0yhnZ8ZsZ0q/aHwveQP?=
 =?iso-8859-1?Q?bq1/fdnxW7idU+4Wv7TTNF7tvmNeRyE8IlSVhWpmcniS3e6QhvQHoMDCVu?=
 =?iso-8859-1?Q?eiE06OoBt/RuLDVps6c=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 71fb576e-d8c4-40b4-dc7a-08dc18c17fd2
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2024 07:37:33.3676
 (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: 16BRCpd/AXY12ZP0RXWOu9TlJZQ6oFfaHxCvvWiF2jk93aGc8dYOiMZ4BdrnzOtndfjozlfGxxtJuST7/Ma09Ad0PWsJnBMHfqhuzcPrN1c=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8493
X-OriginatorOrg: intel.com

Hello!

On 2024-01-18 at 09:15:46 -0800, Reinette Chatre wrote:
>Hi Maciej,
>
>On 1/18/2024 4:02 AM, Maciej Wiecz?r-Retman wrote:
>> On 2024-01-17 at 10:49:06 -0800, Reinette Chatre wrote:
>>> 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:
>
>>>>>> +	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.
>
>It would be easy for the user the figure out if (a) it is obvious to the user
>what schema a particular write_schema() call attempted to write and (b) all the
>write_schema() calls attempt to write different schema.

Okay, your comment made me wonder if on error the schemata still is printed. I
double checked in the code and whether write_schemata() fails or not it has a
goto path where before returning it will print out the schema. So I believe that
satisfies your (a) condition.

As for (b) depends on what you meant. Other tests that run more than one
write_schemata() use different ones every time (CAT, MBM, MBA). Do you suggest
that the non-contiguous test should attempt more schematas? For example shift
the bit hole from one side to the other? I assumed one CBM with a centered bit
hole would be enough to check if non-contiguous CBM feature works properly and
more CBMs would be redundant.

>
>> 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)?
>
>I do agree that users probably won't see such failures. I do not think these
>errors are clues to developers working on the tests though, but instead clues
>to resctrl developers or kernel development CI systems.

Right, I agree, the target group I mentioned was too narrow.

>
>Reinette
>

-- 
Kind regards
Maciej Wiecz?r-Retman