2024-01-30 03:37:23

by Haitao Huang

[permalink] [raw]
Subject: [PATCH v8 01/15] cgroup/misc: Add per resource callbacks for CSS events

From: Kristen Carlson Accardi <[email protected]>

The misc cgroup controller (subsystem) currently does not perform
resource type specific action for Cgroups Subsystem State (CSS) events:
the 'css_alloc' event when a cgroup is created and the 'css_free' event
when a cgroup is destroyed.

Define callbacks for those events and allow resource providers to
register the callbacks per resource type as needed. This will be
utilized later by the EPC misc cgroup support implemented in the SGX
driver.

Signed-off-by: Kristen Carlson Accardi <[email protected]>
Co-developed-by: Haitao Huang <[email protected]>
Signed-off-by: Haitao Huang <[email protected]>
---
V8:
- Abstract out _misc_cg_res_free() and _misc_cg_res_alloc() (Jarkko)
V7:
- Make ops one per resource type and store them in array (Michal)
- Rename the ops struct to misc_res_ops, and enforce the constraints of required callback
functions (Jarkko)
- Moved addition of priv field to patch 4 where it was used first. (Jarkko)

V6:
- Create ops struct for per resource callbacks (Jarkko)
- Drop max_write callback (Dave, Michal)
- Style fixes (Kai)
---
include/linux/misc_cgroup.h | 11 +++++
kernel/cgroup/misc.c | 84 +++++++++++++++++++++++++++++++++----
2 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
index e799b1f8d05b..0806d4436208 100644
--- a/include/linux/misc_cgroup.h
+++ b/include/linux/misc_cgroup.h
@@ -27,6 +27,16 @@ struct misc_cg;

#include <linux/cgroup.h>

+/**
+ * struct misc_res_ops: per resource type callback ops.
+ * @alloc: invoked for resource specific initialization when cgroup is allocated.
+ * @free: invoked for resource specific cleanup when cgroup is deallocated.
+ */
+struct misc_res_ops {
+ int (*alloc)(struct misc_cg *cg);
+ void (*free)(struct misc_cg *cg);
+};
+
/**
* struct misc_res: Per cgroup per misc type resource
* @max: Maximum limit on the resource.
@@ -56,6 +66,7 @@ struct misc_cg {

u64 misc_cg_res_total_usage(enum misc_res_type type);
int misc_cg_set_capacity(enum misc_res_type type, u64 capacity);
+int misc_cg_set_ops(enum misc_res_type type, const struct misc_res_ops *ops);
int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg, u64 amount);
void misc_cg_uncharge(enum misc_res_type type, struct misc_cg *cg, u64 amount);

diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
index 79a3717a5803..14ab13ef3bc7 100644
--- a/kernel/cgroup/misc.c
+++ b/kernel/cgroup/misc.c
@@ -39,6 +39,9 @@ static struct misc_cg root_cg;
*/
static u64 misc_res_capacity[MISC_CG_RES_TYPES];

+/* Resource type specific operations */
+static const struct misc_res_ops *misc_res_ops[MISC_CG_RES_TYPES];
+
/**
* parent_misc() - Get the parent of the passed misc cgroup.
* @cgroup: cgroup whose parent needs to be fetched.
@@ -105,6 +108,36 @@ int misc_cg_set_capacity(enum misc_res_type type, u64 capacity)
}
EXPORT_SYMBOL_GPL(misc_cg_set_capacity);

+/**
+ * misc_cg_set_ops() - set resource specific operations.
+ * @type: Type of the misc res.
+ * @ops: Operations for the given type.
+ *
+ * Context: Any context.
+ * Return:
+ * * %0 - Successfully registered the operations.
+ * * %-EINVAL - If @type is invalid, or the operations missing any required callbacks.
+ */
+int misc_cg_set_ops(enum misc_res_type type, const struct misc_res_ops *ops)
+{
+ if (!valid_type(type))
+ return -EINVAL;
+
+ if (!ops->alloc) {
+ pr_err("%s: alloc missing\n", __func__);
+ return -EINVAL;
+ }
+
+ if (!ops->free) {
+ pr_err("%s: free missing\n", __func__);
+ return -EINVAL;
+ }
+
+ misc_res_ops[type] = ops;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(misc_cg_set_ops);
+
/**
* misc_cg_cancel_charge() - Cancel the charge from the misc cgroup.
* @type: Misc res type in misc cg to cancel the charge from.
@@ -371,6 +404,33 @@ static struct cftype misc_cg_files[] = {
{}
};

+static inline int _misc_cg_res_alloc(struct misc_cg *cg)
+{
+ enum misc_res_type i;
+ int ret;
+
+ for (i = 0; i < MISC_CG_RES_TYPES; i++) {
+ WRITE_ONCE(cg->res[i].max, MAX_NUM);
+ atomic64_set(&cg->res[i].usage, 0);
+ if (misc_res_ops[i]) {
+ ret = misc_res_ops[i]->alloc(cg);
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static inline void _misc_cg_res_free(struct misc_cg *cg)
+{
+ enum misc_res_type i;
+
+ for (i = 0; i < MISC_CG_RES_TYPES; i++)
+ if (misc_res_ops[i])
+ misc_res_ops[i]->free(cg);
+}
+
/**
* misc_cg_alloc() - Allocate misc cgroup.
* @parent_css: Parent cgroup.
@@ -383,20 +443,25 @@ static struct cftype misc_cg_files[] = {
static struct cgroup_subsys_state *
misc_cg_alloc(struct cgroup_subsys_state *parent_css)
{
- enum misc_res_type i;
- struct misc_cg *cg;
+ struct misc_cg *parent_cg, *cg;
+ int ret;

- if (!parent_css) {
- cg = &root_cg;
+ if (unlikely(!parent_css)) {
+ parent_cg = cg = &root_cg;
} else {
cg = kzalloc(sizeof(*cg), GFP_KERNEL);
if (!cg)
return ERR_PTR(-ENOMEM);
+ parent_cg = css_misc(parent_css);
}

- for (i = 0; i < MISC_CG_RES_TYPES; i++) {
- WRITE_ONCE(cg->res[i].max, MAX_NUM);
- atomic64_set(&cg->res[i].usage, 0);
+ ret = _misc_cg_res_alloc(cg);
+ if (ret) {
+ _misc_cg_res_free(cg);
+ if (likely(parent_css))
+ kfree(cg);
+
+ return ERR_PTR(ret);
}

return &cg->css;
@@ -410,7 +475,10 @@ misc_cg_alloc(struct cgroup_subsys_state *parent_css)
*/
static void misc_cg_free(struct cgroup_subsys_state *css)
{
- kfree(css_misc(css));
+ struct misc_cg *cg = css_misc(css);
+
+ _misc_cg_res_free(cg);
+ kfree(cg);
}

/* Cgroup controller callbacks */
--
2.25.1



2024-02-01 23:24:56

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v8 01/15] cgroup/misc: Add per resource callbacks for CSS events

On Tue Jan 30, 2024 at 4:09 AM EET, Haitao Huang wrote:
> From: Kristen Carlson Accardi <[email protected]>
>
> The misc cgroup controller (subsystem) currently does not perform
> resource type specific action for Cgroups Subsystem State (CSS) events:
> the 'css_alloc' event when a cgroup is created and the 'css_free' event
> when a cgroup is destroyed.
>
> Define callbacks for those events and allow resource providers to
> register the callbacks per resource type as needed. This will be
> utilized later by the EPC misc cgroup support implemented in the SGX
> driver.
>
> Signed-off-by: Kristen Carlson Accardi <[email protected]>
> Co-developed-by: Haitao Huang <[email protected]>
> Signed-off-by: Haitao Huang <[email protected]>
> ---
> V8:
> - Abstract out _misc_cg_res_free() and _misc_cg_res_alloc() (Jarkko)
> V7:
> - Make ops one per resource type and store them in array (Michal)
> - Rename the ops struct to misc_res_ops, and enforce the constraints of required callback
> functions (Jarkko)
> - Moved addition of priv field to patch 4 where it was used first. (Jarkko)
>
> V6:
> - Create ops struct for per resource callbacks (Jarkko)
> - Drop max_write callback (Dave, Michal)
> - Style fixes (Kai)

This version looks nice and smooth:

Reviewed-by: Jarkko Sakkinen <[email protected]>

BR, Jarkko

2024-02-02 16:35:10

by Haitao Huang

[permalink] [raw]
Subject: Re: [PATCH v8 01/15] cgroup/misc: Add per resource callbacks for CSS events

On Thu, 01 Feb 2024 17:24:40 -0600, Jarkko Sakkinen <[email protected]>
wrote:

> On Tue Jan 30, 2024 at 4:09 AM EET, Haitao Huang wrote:
>> From: Kristen Carlson Accardi <[email protected]>
>>
>> The misc cgroup controller (subsystem) currently does not perform
>> resource type specific action for Cgroups Subsystem State (CSS) events:
>> the 'css_alloc' event when a cgroup is created and the 'css_free' event
>> when a cgroup is destroyed.
>>
>> Define callbacks for those events and allow resource providers to
>> register the callbacks per resource type as needed. This will be
>> utilized later by the EPC misc cgroup support implemented in the SGX
>> driver.
>>
>> Signed-off-by: Kristen Carlson Accardi <[email protected]>
>> Co-developed-by: Haitao Huang <[email protected]>
>> Signed-off-by: Haitao Huang <[email protected]>
>> ---
>> V8:
>> - Abstract out _misc_cg_res_free() and _misc_cg_res_alloc() (Jarkko)
>> V7:
>> - Make ops one per resource type and store them in array (Michal)
>> - Rename the ops struct to misc_res_ops, and enforce the constraints of
>> required callback
>> functions (Jarkko)
>> - Moved addition of priv field to patch 4 where it was used first.
>> (Jarkko)
>>
>> V6:
>> - Create ops struct for per resource callbacks (Jarkko)
>> - Drop max_write callback (Dave, Michal)
>> - Style fixes (Kai)
>
> This version looks nice and smooth:
>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
>
> BR, Jarkko
>
Thanks Jarkko!

@tj, I had your Reviewed-by tags previously but dropped it due to some
changes for refactoring suggested by other reviewers. Could you take a
look at patches 1-2 for MiscController and confirm if I can have your
Reviewed-by tags again?
Thanks

Haitao

2024-02-02 17:05:29

by Tejun Heo

[permalink] [raw]
Subject: Re: [PATCH v8 01/15] cgroup/misc: Add per resource callbacks for CSS events

Hello,

On Fri, Feb 02, 2024 at 10:34:29AM -0600, Haitao Huang wrote:
> @tj, I had your Reviewed-by tags previously but dropped it due to some
> changes for refactoring suggested by other reviewers. Could you take a look
> at patches 1-2 for MiscController and confirm if I can have your Reviewed-by
> tags again?

Yeah, they look pretty sane to me. Please feel free to add my Reviewed-by's.

Thanks.

--
tejun

2024-02-12 19:31:23

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v8 01/15] cgroup/misc: Add per resource callbacks for CSS events

On Fri Feb 2, 2024 at 6:34 PM EET, Haitao Huang wrote:
> On Thu, 01 Feb 2024 17:24:40 -0600, Jarkko Sakkinen <[email protected]>
> wrote:
>
> > On Tue Jan 30, 2024 at 4:09 AM EET, Haitao Huang wrote:
> >> From: Kristen Carlson Accardi <[email protected]>
> >>
> >> The misc cgroup controller (subsystem) currently does not perform
> >> resource type specific action for Cgroups Subsystem State (CSS) events:
> >> the 'css_alloc' event when a cgroup is created and the 'css_free' event
> >> when a cgroup is destroyed.
> >>
> >> Define callbacks for those events and allow resource providers to
> >> register the callbacks per resource type as needed. This will be
> >> utilized later by the EPC misc cgroup support implemented in the SGX
> >> driver.
> >>
> >> Signed-off-by: Kristen Carlson Accardi <[email protected]>
> >> Co-developed-by: Haitao Huang <[email protected]>
> >> Signed-off-by: Haitao Huang <[email protected]>
> >> ---
> >> V8:
> >> - Abstract out _misc_cg_res_free() and _misc_cg_res_alloc() (Jarkko)
> >> V7:
> >> - Make ops one per resource type and store them in array (Michal)
> >> - Rename the ops struct to misc_res_ops, and enforce the constraints of
> >> required callback
> >> functions (Jarkko)
> >> - Moved addition of priv field to patch 4 where it was used first.
> >> (Jarkko)
> >>
> >> V6:
> >> - Create ops struct for per resource callbacks (Jarkko)
> >> - Drop max_write callback (Dave, Michal)
> >> - Style fixes (Kai)
> >
> > This version looks nice and smooth:
> >
> > Reviewed-by: Jarkko Sakkinen <[email protected]>
> >
> > BR, Jarkko
> >
> Thanks Jarkko!
>
> @tj, I had your Reviewed-by tags previously but dropped it due to some
> changes for refactoring suggested by other reviewers. Could you take a
> look at patches 1-2 for MiscController and confirm if I can have your
> Reviewed-by tags again?
> Thanks
>
> Haitao

you can add tags

BR, Jarkko