linux-kernel - RE: [PATCH 1/3] selftests/resctrl: Make resctrl

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: <TYAPR01MB63309592B2B82329246319878B6D9@TYAPR01MB6330.jpnprd01.prod.outlook.com>
Date:   Mon, 6 Dec 2021 06:57:50 +0000
From:   "tan.shaopeng@...itsu.com" <tan.shaopeng@...itsu.com>
To:     'Reinette Chatre' <reinette.chatre@...el.com>
CC:     Fenghua Yu <fenghua.yu@...el.com>, Shuah Khan <shuah@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: RE: [PATCH 1/3] selftests/resctrl: Make resctrl_tests run using
 kselftest framework

Hi Reinette,
 
> On 12/2/2021 11:21 PM, tan.shaopeng@...itsu.com wrote:
> >> On 11/30/2021 6:36 PM, tan.shaopeng@...itsu.com wrote:
> >>>> On 11/10/2021 1:33 AM, Shaopeng Tan wrote:
> >>>>> From: "Tan, Shaopeng" <tan.shaopeng@...fujitsu.com>
> 
> >>>>> To ensure the resctrl_tests finish in limited time, this commit
> >>>>> changed the default limited time(45s) to 120 seconds for
> >>>>> resctrl_tests by adding "setting" file.
> >>>>
> >>>> How is changing the timeout related to the resctrl framework changes?
> >>>> Is it not a separate change?
> >>>
> >>> In selftest framwork, the default limited time of all tests is 45
> >>> seconds which is specified by common file
> >> tools/testing/selftests/kselftest/runner.sh.
> >>> Each test can change the limited time individually by adding a "setting"
> >>> file into its own directory. I changed the limited time of resctrl
> >>> to120s because 45s was not enough in my environment.
> >>
> >> Understood. My question was if this can be a separate change with its
> >> own patch? It seems to me that this deserves its own patch ... but
> >> actually it also raises an important issue that the resctrl tests are taking a
> long time.
> >>
> >> I do see a rule for tests in Documentation/dev-tools/kselftest.rst:
> >> "Don't take too long". This may be a motivation _not_ to include the
> >> resctrl tests in the regular kselftest targets because when a user
> >> wants to run all tests on a system it needs to be quick and the resctrl tests
> are not quick.
> >
> > I think 120s is not long, there are 6 tests with a limited time over
> > 120s, for example, the limited time of net test is set 300s.
> 
> I am not familiar with the specific kselftest requirements in this regard but the
> test duration is surely something that needs to be kept in mind. Consider the
> systems performing integration testing on kernels everywhere - running the
> kselftest framework is a reasonable thing to do and test delays that may seem
> palatable on an individual run may not be appropriate for all test
> infrastructures.
> 
> Needing to almost triple the needed time from the default time is a red flag and
> really deserves to be in its own patch with a motivation. I would also
> recommend highlighting this change in the cover letter. This will bring the issue
> to the attention of the kselftest audience who will provide a better informed
> opinion (whether they want a long running test as part of the default framework
> or not).

Thanks for your advice.
I will separate the part about default limited time to a new patch.
In order to get some opinions about change default limited time,
I will add a description in the cover letter,
when posting the next version of this patch.

Regards,
Shaopeng Tan