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]
Message-ID: <eed9454b-a276-e545-64a7-5d236dd38d82@intel.com>
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ