[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aQEOIfMFo1Dqv-sP@agluck-desk3>
Date: Tue, 28 Oct 2025 11:40:33 -0700
From: "Luck, Tony" <tony.luck@...el.com>
To: Reinette Chatre <reinette.chatre@...el.com>
CC: Fenghua Yu <fenghuay@...dia.com>, Maciej Wieczor-Retman
	<maciej.wieczor-retman@...el.com>, Peter Newman <peternewman@...gle.com>,
	James Morse <james.morse@....com>, Babu Moger <babu.moger@....com>, "Drew
 Fustini" <dfustini@...libre.com>, Dave Martin <Dave.Martin@....com>, Chen Yu
	<yu.c.chen@...el.com>, <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
	<patches@...ts.linux.dev>
Subject: Re: [PATCH v12 20/31] fs/resctrl: Refactor mkdir_mondata_subdir()
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;
}
Powered by blists - more mailing lists
 
