Re: [PATCH v5 02/18] cgroup/misc: Add SGX EPC resource type and export APIs for SGX driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 27 Sep 2023 22:59:12 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:

On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
From: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>

Add SGX EPC memory, MISC_CG_RES_SGX_EPC, to be a valid resource type
for the misc controller.

Add per resource type private data so that SGX can store additional per
cgroup data in misc_cg->misc_cg_res[MISC_CG_RES_SGX_EPC].

To be honest I don't quite understand why putting the above two changes in this
patch together with exporting misc_cg_root/parent() below.

Any reason why the above two cannot be done together with patch (" x86/sgx: Limit process EPC usage with misc cgroup controller"), where these changes are
actually related?

We all already know that a new EPC misc cgroup will be added. There's no need to actually introduce the new type here only to justify exporting some helper
functions.


I think previous authors intended to separate all prerequisite misc changes from SGX changes.
I can combine them if maintainers are fine with it.


Export misc_cg_root() so the SGX driver can initialize and add those
additional structures to the root misc cgroup as part of initialization
for EPC cgroup support. This bootstraps the same additional
initialization for non-root cgroups in the 'alloc()' callback added in the
previous patch.

The SGX driver, as the EPC memory provider, will have a background
worker to reclaim EPC pages to make room for new allocations in the same
cgroup when its usage counter reaches near the limit controlled by the
cgroup and its ancestors. Therefore it needs to do a walk from the
current cgroup up to the root. To enable this walk, move parent_misc()
into misc_cgroup.h and make inline to make this function available to
SGX, rename it to misc_cg_parent(), and update kernel/cgroup/misc.c to
use the new name.

Looks too many details in the above two paragraphs.  Could we have a more
concise justification for exporting these two functions?


This was added to address Jarkko's question, "why does SGX driver need to do iterative walks?"
See: https://lore.kernel.org/all/CVHOU5G1SCUT.RCBVZ3W8G2NJ@suppilovahvero/

And if it were me, I would put it at a relatively later position (e.g., before the patch actually implements EPC cgroup) for better review. This also applies
to the first patch.


I was told to move all prerequisites to the front or separate out.

https://lore.kernel.org/linux-sgx/CU4H43P3H35X.1BCA3CE4D1250@seitikki/





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux