| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <op.2h0cx9pdwjvjmi@hhuan26-mobl.amr.corp.intel.com>
Date: Tue, 23 Jan 2024 10:19:11 -0600
From: "Haitao Huang" <haitao.huang@...ux.intel.com>
To: dave.hansen@...ux.intel.com, tj@...nel.org, mkoutny@...e.com,
linux-kernel@...r.kernel.org, linux-sgx@...r.kernel.org, x86@...nel.org,
cgroups@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
hpa@...or.com, sohil.mehta@...el.com, "Jarkko Sakkinen" <jarkko@...nel.org>
Cc: zhiquan1.li@...el.com, kristen@...ux.intel.com, seanjc@...gle.com,
zhanb@...rosoft.com, anakrish@...rosoft.com, mikko.ylinen@...ux.intel.com,
yangjie@...rosoft.com
Subject: Re: [PATCH v7 01/15] cgroup/misc: Add per resource callbacks for CSS
events
On Mon, 22 Jan 2024 14:14:01 -0600, Jarkko Sakkinen <jarkko@...nel.org>
wrote:
> On Mon Jan 22, 2024 at 7:20 PM EET, Haitao Huang wrote:
>> From: Kristen Carlson Accardi <kristen@...ux.intel.com>
>>
>> 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 <kristen@...ux.intel.com>
>> Co-developed-by: Haitao Huang <haitao.huang@...ux.intel.com>
>> Signed-off-by: Haitao Huang <haitao.huang@...ux.intel.com>
>> ---
>> 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 | 60 +++++++++++++++++++++++++++++++++++--
>> 2 files changed, 68 insertions(+), 3 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..b8c32791334c 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.
>> @@ -383,23 +416,37 @@ static struct cftype misc_cg_files[] = {
>> static struct cgroup_subsys_state *
>> misc_cg_alloc(struct cgroup_subsys_state *parent_css)
>> {
>> + struct misc_cg *parent_cg, *cg;
>> enum misc_res_type i;
>> - struct misc_cg *cg;
>> + int ret;
>>
>> if (!parent_css) {
>> - cg = &root_cg;
>> + 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);
>> + if (misc_res_ops[i]) {
>> + ret = misc_res_ops[i]->alloc(cg);
>> + if (ret)
>> + goto alloc_err;
>
> So I'd consider pattern like this to avoid repetition:
>
> if (ret) {
> __misc_cg_free(cg);
> return PERR_PTR(ret);
> }
>
> and call __misc_cg_free() also in misc_cg_free().
>
Will change to do that.
Thanks
Haitao
Powered by blists - more mailing lists