Received-SPF: pass (google.com: domain of linux-kernel+bounces-203649-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Thu, 06 Jun 2024 09:20:16 +0300
Message-Id: <D1SPTDBBDU6F.2WLRZKYZWWTRB@kernel.org>
Cc: <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 v14 14/14] selftests/sgx: Add scripts for EPC cgroup
 testing
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Huang, Kai" <kai.huang@intel.com>, "Haitao Huang"
 <haitao.huang@linux.intel.com>, <dave.hansen@linux.intel.com>,
 <tj@kernel.org>, <mkoutny@suse.com>, <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>
References: <20240531222630.4634-1-haitao.huang@linux.intel.com>
 <20240531222630.4634-15-haitao.huang@linux.intel.com>
 <D1RKK8CENNXI.1KMNDADV9C1YM@kernel.org>
 <op.2owf5xiwwjvjmi@hhuan26-mobl.amr.corp.intel.com>
 <7cbf3583-a23e-4437-afc2-1faeb4a1f436@intel.com>
In-Reply-To: <7cbf3583-a23e-4437-afc2-1faeb4a1f436@intel.com>

On Thu Jun 6, 2024 at 1:30 AM EEST, Huang, Kai wrote:
>
> >> Reorg:
> >>
> >> void sgx_cgroup_init(void)
> >> {
> >> =C2=A0=C2=A0=C2=A0=C2=A0struct workqueue_struct *wq;
> >>
> >> =C2=A0=C2=A0=C2=A0=C2=A0/* eagerly allocate the workqueue: */
> >> =C2=A0=C2=A0=C2=A0=C2=A0wq =3D alloc_workqueue("sgx_cg_wq", wq_unbound=
 | wq_freezable,=20
> >> wq_unbound_max_active);
> >> =C2=A0=C2=A0=C2=A0=C2=A0if (!wq) {
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pr_warn("sgx_cg_wq creation=
 failed\n");
> >> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 return;
> >=20
> > sgx_cgroup_try_charge() expects sgx_cg_wq, so it would break unless we=
=20
> > check and return 0 which was the initially implemented in v12. But then=
=20
> > Kai had some concern on that we expose all the interface files to allow=
=20
> > user to set limits but we don't enforce. To keep it simple we settled=
=20
> > down back to BUG_ON().=20
>
> [...]
>
> > This would only happen rarely and user can add=20
> > command-line to disable SGX if s/he really wants to start kernel in thi=
s=20
> > case, just can't do SGX.
>
> Just to be clear that I don't like BUG_ON() either.  It's inevitable you=
=20
> will get attention because of using it.

Just then plain disable SGX if it fails to start.

Do not take down the whole system.

> This is a compromise that you don't want to reset "capacity" to 0 when=20
> alloc_workqueue() fails.

BUG_ON() is not a "compromise".

> There are existing code where BUG_ON() is used during the kernel early=20
> boot code when memory allocation fails (e.g., see cgroup_init_subsys()),=
=20
> so it might be acceptable to use BUG_ON() here, but it's up to=20
> maintainers to decide whether it is OK.

When it is not possible continue to run the system at all, and only
then.

Here it is possible. Without SGX.

BR, Jarkko