Re: [PATCH v9 15/15] selftests/sgx: Add scripts for EPC cgroup testing

From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Michal Koutný" <mkoutny@suse.com>
Cc: "Haitao Huang" <haitao.huang@linux.intel.com>,
	<dave.hansen@linux.intel.com>, <tj@kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-sgx@vger.kernel.org>,
	<x86@kernel.org>, <cgroups@vger.kernel.org>, <tglx@linutronix.de>,
	<mingo@redhat.com>, <bp@alien8.de>, <hpa@zytor.com>,
	<sohil.mehta@intel.com>, <tim.c.chen@linux.intel.com>,
	<zhiquan1.li@intel.com>, <kristen@linux.intel.com>,
	<seanjc@google.com>, <zhanb@microsoft.com>,
	<anakrish@microsoft.com>, <mikko.ylinen@linux.intel.com>,
	<yangjie@microsoft.com>, <chrisyan@microsoft.com>
Subject: Re: [PATCH v9 15/15] selftests/sgx: Add scripts for EPC cgroup testing
Date: Tue, 02 Apr 2024 14:58:40 +0300	[thread overview]
Message-ID: <D09MB26IPFFW.3UBD7M0S17SG6@kernel.org> (raw)
In-Reply-To: <htiz5jgsby5v262saphhomcsxtixb2u7ot6jcghpfhvgz65ht6@qlz3gpdwapaa>

On Tue Apr 2, 2024 at 2:23 PM EEST, Michal Koutný wrote:
> Hello.
>
> On Sat, Mar 30, 2024 at 01:26:08PM +0200, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > > > It'd be more complicated and less readable to do all the stuff without the  
> > > > cgroup-tools, esp cgexec. I checked dependency, cgroup-tools only depends  
> > > > on libc so I hope this would not cause too much inconvenience.
> > >
> > > As per cgroup-tools, please prove this. It makes the job for more
> > > complicated *for you* and you are making the job more  complicated
> > > to every possible person in the planet running any kernel QA.
> > >
> > > I weight the latter more than the former. And it is exactly the
> > > reason why we did custom user space kselftest in the first place.
> > > Let's keep the tradition. All I can say is that kselftest is 
> > > unfinished in its current form.
> > >
> > > What is "esp cgexec"?
> > 
> > Also in kselftest we don't drive ultimate simplicity, we drive
> > efficient CI/QA. By open coding something like subset of
> > cgroup-tools needed to run the test you also help us later
> > on to backtrack the kernel changes. With cgroups-tools you
> > would have to use strace to get the same info.
>
> FWIW, see also functions in
> tools/testing/selftests/cgroup/cgroup_util.{h,c}.
> They likely cover what you need already -- if the tests are in C.
>
> (I admit that stuff in tools/testing/selftests/cgroup/ is best
> understood with strace.)

Thanks!

My conclusions are that:

1. We probably cannot move the test part of cgroup test itself
   given the enclave payload dependency.
2. I think it makes sense to still follow the same pattern as
   other cgroups test and re-use cgroup_util.[ch] functionaltiy.

So yeah I guess we need two test programs instead of one.

Something along the lines:

1. main.[ch] -> test_sgx.[ch]
2. introduce test_sgx_cgroup.c

And test_sgx_cgroup.c would be implement similar test as the shell
script and would follow the structure of existing cgroups tests.

>
> HTH,
> Michal

BR, Jarkko