Re: [PATCH v12 20/31] fs/resctrl: Refactor mkdir_mondata_subdir()

From: Luck, Tony

Date: Tue Oct 28 2025 - 14:40:44 EST


On Tue, Oct 28, 2025 at 10:40:44AM -0700, Reinette Chatre wrote:
> Modifying mon_add_all_files() sounds good. I assume the node activation (kernfs_active())
> will still be done by the caller which would have this new function return a struct kernfs_node *?
> If so, I think it will make code easier to read if name implies that a new kn is created.
> Since the caller is already mkdir_mondata_subdir(), what do you think of _mkdir_mondata_subdir()?

Reinette,

This section of fs/resctrl/rdtgroup.c now looks like this. Call sites
are now free of duplicated code. Thanks for the nudge to do this.

-Tony

---

/*
* Create a directory for a domain and populate it with monitor files.
*/
static struct kernfs_node *_mkdir_mondata_subdir(struct kernfs_node *parent_kn, char *name,
struct rdt_domain_hdr *hdr,
struct rdt_resource *r,
struct rdtgroup *prgrp, int domid)
{
struct rmid_read rr = {0};
struct kernfs_node *kn;
struct mon_data *priv;
struct mon_evt *mevt;
int ret;

kn = kernfs_create_dir(parent_kn, name, parent_kn->mode, prgrp);
if (IS_ERR(kn))
return kn;

ret = rdtgroup_kn_set_ugid(kn);
if (ret)
goto out_destroy;

for_each_mon_event(mevt) {
if (mevt->rid != r->rid || !mevt->enabled)
continue;
priv = mon_get_kn_priv(r->rid, domid, mevt, !hdr);
if (WARN_ON_ONCE(!priv)) {
ret = -EINVAL;
goto out_destroy;
}

ret = mon_addfile(kn, mevt->name, priv);
if (ret)
goto out_destroy;

if (hdr && resctrl_is_mbm_event(mevt->evtid))
mon_event_read(&rr, r, hdr, prgrp, &hdr->cpu_mask, mevt, true);
}

return kn;
out_destroy:
kernfs_remove(kn);
return ERR_PTR(ret);
}

static int mkdir_mondata_subdir_snc(struct kernfs_node *parent_kn,
struct rdt_domain_hdr *hdr,
struct rdt_resource *r, struct rdtgroup *prgrp)
{
struct kernfs_node *ckn, *kn;
struct rdt_l3_mon_domain *d;
char name[32];

if (!domain_header_is_valid(hdr, RESCTRL_MON_DOMAIN, RDT_RESOURCE_L3))
return -EINVAL;

d = container_of(hdr, struct rdt_l3_mon_domain, hdr);
sprintf(name, "mon_%s_%02d", r->name, d->ci_id);
kn = kernfs_find_and_get(parent_kn, name);
if (kn) {
/*
* rdtgroup_mutex will prevent this directory from being
* removed. No need to keep this hold.
*/
kernfs_put(kn);
} else {
kn = _mkdir_mondata_subdir(parent_kn, name, NULL, r, prgrp, d->ci_id);
if (IS_ERR(kn))
return PTR_ERR(kn);
}

sprintf(name, "mon_sub_%s_%02d", r->name, hdr->id);
ckn = _mkdir_mondata_subdir(kn, name, hdr, r, prgrp, hdr->id);
if (IS_ERR(ckn)) {
kernfs_remove(kn);
return PTR_ERR(ckn);
}

kernfs_activate(kn);
return 0;
}

static int mkdir_mondata_subdir(struct kernfs_node *parent_kn,
struct rdt_domain_hdr *hdr,
struct rdt_resource *r, struct rdtgroup *prgrp)
{
struct kernfs_node *kn;
char name[32];

lockdep_assert_held(&rdtgroup_mutex);

if (r->rid == RDT_RESOURCE_L3 && r->mon_scope == RESCTRL_L3_NODE)
return mkdir_mondata_subdir_snc(parent_kn, hdr, r, prgrp);

sprintf(name, "mon_%s_%02d", r->name, hdr->id);
kn = _mkdir_mondata_subdir(parent_kn, name, hdr, r, prgrp, hdr->id);
if (IS_ERR(kn))
return PTR_ERR(kn);

kernfs_activate(kn);
return 0;
}