[<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