linux-kernel - Re: [PATCH v2 24/24] selftests/resctrl: Rewrite Cache Allocation Technology (CAT) test

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Apr 2023 08:15:29 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC:     <linux-kselftest@...r.kernel.org>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Shuah Khan <shuah@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "Shaopeng Tan" <tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH v2 24/24] selftests/resctrl: Rewrite Cache Allocation
 Technology (CAT) test

Hi Ilpo,

On 4/27/2023 1:04 AM, Ilpo Järvinen wrote:
> On Wed, 26 Apr 2023, Reinette Chatre wrote:
>> On 4/26/2023 6:58 AM, Ilpo Järvinen wrote:
>>> On Fri, 21 Apr 2023, Reinette Chatre wrote:
>>>> On 4/18/2023 4:45 AM, Ilpo Järvinen wrote:
>>
>> ...
>>
>>>>> diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c
>>>>> index 4b505fdb35d7..85053829b9c5 100644
>>>>> --- a/tools/testing/selftests/resctrl/cat_test.c
>>>>> +++ b/tools/testing/selftests/resctrl/cat_test.c
>>>>> @@ -11,11 +11,12 @@
>>>>>  #include "resctrl.h"
>>>>>  #include <unistd.h>
>>>>>  
>>>>> -#define RESULT_FILE_NAME1	"result_cat1"
>>>>> -#define RESULT_FILE_NAME2	"result_cat2"
>>>>> -#define NUM_OF_RUNS		5
>>>>> -#define MAX_DIFF_PERCENT	4
>>>>> -#define MAX_DIFF		1000000
>>>>> +#define RESULT_FILE_NAME		"result_cat"
>>>>> +#define NUM_OF_RUNS			5
>>>>> +#define MIN_DIFF_PERCENT_PER_BIT	2
>>>>
>>>> Could you please start a new trend that adds documentation
>>>> that explains what this constant means and how it was chosen?
>>>
>>> I can try although that particular 2 was a bit handwavy that just seems to 
>>> work with the tests I performed.
>>
>> The changelog claims that the existing CAT test does not work with
>> this new test offered as replacement. Considering that I do think it
>> is important to have confidence that this test is able to test CAT.
>> The words "handwave" and "seems to work" are red flags to me.
>> When merged, these tests will be run on a variety of platforms with
>> various configurations. Using test criteria based on measurements
>> from one particular system may work but there needs to be confidence
>> that the criteria maps to all systems these tests will be run on.
> 
> My "tests" (in plural) were not limited to one particular system but 
> included systems from different generations.
> 

Thank you very much for your thorough testing. Having this information
accompany this change will surely help to increase confidence in the
value chosen. 

Thank you very much

Reinette