Received-SPF: pass (google.com: domain of linux-kernel+bounces-128344-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Content-Type: text/plain; charset=iso-8859-15; format=flowed; delsp=yes
To: =?iso-8859-15?Q?Michal_Koutn=FD?= <mkoutny@suse.com>, "Jarkko Sakkinen"
 <jarkko@kernel.org>
Cc: 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
References: <20240205210638.157741-1-haitao.huang@linux.intel.com>
 <20240205210638.157741-16-haitao.huang@linux.intel.com>
 <4be7b291010973c203ed8c7bcd25b626c1290231.camel@kernel.org>
 <D04OVW6I8MUA.1OAIHFQ8943SM@kernel.org>
 <op.2lbjl0oawjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <D071OAFZ80O6.XEDXJ8AF4PK9@kernel.org> <D071QIHLW7MP.UM9R3VYETIOK@kernel.org>
 <htiz5jgsby5v262saphhomcsxtixb2u7ot6jcghpfhvgz65ht6@qlz3gpdwapaa>
 <D09MB26IPFFW.3UBD7M0S17SG6@kernel.org>
Date: Tue, 02 Apr 2024 11:20:21 -0500
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: Quoted-Printable
From: "Haitao Huang" <haitao.huang@linux.intel.com>
Organization: Intel
Message-ID: <op.2llzn7wgwjvjmi@hhuan26-mobl.amr.corp.intel.com>
In-Reply-To: <D09MB26IPFFW.3UBD7M0S17SG6@kernel.org>
User-Agent: Opera Mail/1.0 (Win32)

On Tue, 02 Apr 2024 06:58:40 -0500, Jarkko Sakkinen <jarkko@kernel.org> =
 =

wrote:

> On Tue Apr 2, 2024 at 2:23 PM EEST, Michal Koutn=FD 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 on=
ly  =

>> 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  complicate=
d
>> > > 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
>
Do we really want to have it implemented in c? There are much fewer line=
s  =

of code in shell scripts. Note we are not really testing basic cgroup  =

stuff. All we needed were creating/deleting cgroups and set limits which=
 I  =

think have been demonstrated feasible in the ash scripts now.

Given Dave's comments, and test scripts being working and cover the case=
s  =

needed IMHO, I don't see much need to move to c code. I can add more cas=
es  =

if needed and fall back a c implementation later  if any case can't be  =

implemented in scripts. How about that?

Haitao