Messages in this thread | | | From | James Morse <> | Subject | Re: [PATCH 05/24] x86/resctrl: Pass the schema in resdir's private pointer | Date | Fri, 12 Mar 2021 17:11:06 +0000 |
| |
Hi Reinette,
On 17/11/2020 21:49, Reinette Chatre wrote: > It is not clear what "resdir" mentioned in subject line refers to.
rdtgroup_mkdir_info_resdir(), it looks I picked the wrong bit to identify it. ('info' in a name usually conveys no information at all!)
> Could it be changed to "info dir"?
Sure,
> On 10/30/2020 9:11 AM, James Morse wrote: >> Moving properties that resctrl exposes to user-space into the core >> 'fs' code, (e.g. the name of the schema), means some of the functions >> that back the filesystem need the schema struct, but currently take the >> resource. > > I think a simple addition would help to parse the above ... > > " ... need the schema struct (to where the properties are moved), ..." > >> >> Once the CDP resources are merged, the resource doesn't reflect the >> right level of information. >> >> For the info dirs that represent a control, the information needed >> is in the schema, as this is how the resource is being used. For the >> monitors, its the resource as L3CODE_MON doesn't make sense, and would >> monitor data too. >> >> This difference means the type of the private pointers varies >> between control and monitor info dirs. >> >> If the flags are RF_MON_INFO, its a struct rdt_resource. If the >> flags are RF_CTRL_INFO, its a struct resctrl_schema. Nothing in >> res_common_files[] has both flags.
>> @@ -1794,6 +1803,7 @@ static int rdtgroup_mkdir_info_resdir(struct rdt_resource *r, char >> *name, >> static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn) >> { >> + struct resctrl_schema *s; >> struct rdt_resource *r; >> unsigned long fflags; >> char name[32]; >> @@ -1809,9 +1819,10 @@ static int rdtgroup_create_info_dir(struct kernfs_node *parent_kn) >> if (ret) >> goto out_destroy; >> - for_each_alloc_enabled_rdt_resource(r) { >> + list_for_each_entry(s, &resctrl_all_schema, list) { >> + r = s->res; >> fflags = r->fflags | RF_CTRL_INFO; >> - ret = rdtgroup_mkdir_info_resdir(r, r->name, fflags); >> + ret = rdtgroup_mkdir_info_resdir(s, r->name, fflags); >> if (ret) >> goto out_destroy; >> }
> I think it would be helpful to add a comment here to compensate for the symmetry that is > removed ("for_each_alloc_enabled_rdt_resource()" followed by a > "for_each_mon_enabled_rdt_resource()").
Sure, the thing to convey is the first loop is for 'alloc_enabled' controls.
Thanks,
James
| |