RE: [PATCH 1/3] selftests/resctrl: Make resctrl_tests run using kselftest framework

[tan.shaopeng@xxxxxxxxxxx]

Hi Reinette,
 
> On 12/2/2021 11:21 PM, tan.shaopeng@xxxxxxxxxxx wrote:
> >> On 11/30/2021 6:36 PM, tan.shaopeng@xxxxxxxxxxx wrote:
> >>>> On 11/10/2021 1:33 AM, Shaopeng Tan wrote:
> >>>>> From: "Tan, Shaopeng" <tan.shaopeng@xxxxxxxxxxxxxx>
> 
> >>>>> 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