[LTP] [PATCH] cpuset_hotplug_test.sh: Fix a race condition

Qais Yousef
2020-07-16T15:30:37Z

On 07/16/20 16:21, Petr Vorel wrote:
> Hi Qais,
>
> > > > Let me know what would be the preferred approach.
> > > Just please try to send a patch using TST_RETRY_FUNC (thus you need to figure
> > > out the verifier), or let us know and we either figure that or just simply use
> > > your original patch.
>
> > So I have cooked something quick but I either get
>
> > /root/arm64-ltp/testcases/bin/cpuset_hotplug_test.sh: line 159: TST_RETRY_FUNC: command not found
> I'm sorry, I have forgotten that the tests haven't been ported to the new API
> yet (tst_test.sh). Thus, I'm ok to take the original patch, unless anybody
> objects.
>
> Why? Rewriting is going to be bigger task, as shell scripts has interdependencies,
> thus turning cpuset_funcs.sh to use new API requires turning the other tests
> which use it (it's not acceptable to load both legacy and new API in a test).
> Besides new API needs to rename some functions (s/tst_resm/tst_res/, ... that's
> easy task), but also works differently, so it's more work that renaming these
> functions. Also tests need heavy cleanup (style is not optimal, using full path
> like /bin/kill is ugly).
>
> Although this should be done, the bigger problem is that these tests test just
> cgroup v1 (and only early implementations, see notes about 2.6.29 in the code),
> has other bugs than just one you spotted. There might be similar approach we
> took with NUMA tests: shell tests are in maintenance mode and new tests are
> written from the scratch in C.
>
> BTW there is some support for cgroup in lib/tst_cgroup.c.

Yeah that's what I thought if the test will need porting to a different
infrastructure. It won't be just this test that will need changing..

>
> > or
>
> > cpuset_hotplug_test 1 TBROK: Test /root/arm64-ltp/testcases/bin/cpuset_hotplug_test.sh must call tst_run!
>
> > Depending whether I impost test.sh or tst_test.sh.
>
> > It seems there's a dependency on the overall test construction depending on
> > what file is imported. And one can import one or the other. It seems
> > tst_test.sh is the legacy approach?
> No, test.sh is the legacy one.
> There is a docs for new API (C and shell), if you're interested.
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines

Ah cool thanks.

>
> > My WIP patch is attached in case there's somethign obvious to be done here that
> > I missed.
>
> > It looks like the current patch is the simplest thing to do otherwise? :-/
> yes :).

Thanks for your review!

Cheers

--
Qais Yousef