Hi Reinette,
On 7/17/25 22:54, Reinette Chatre wrote:
> Hi Babu,
>
> On 7/8/25 3:17 PM, Babu Moger wrote:
>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>> index 15d10c346307..bb28ef7e4600 100644
>> --- a/fs/resctrl/rdtgroup.c
>> +++ b/fs/resctrl/rdtgroup.c
>> @@ -86,6 +86,8 @@ enum resctrl_event_id mba_mbps_default_event;
>>
>> static bool resctrl_debug;
>>
>> +extern struct mbm_transaction mbm_transactions[NUM_MBM_TRANSACTIONS];
>> +
>
> Please move this extern to fs/resctrl/internal.h.
No required anymore as event_filter_show() is moved to monitor.c
>
>> void rdt_last_cmd_clear(void)
>> {
>> lockdep_assert_held(&rdtgroup_mutex);
>> @@ -1895,6 +1897,25 @@ static int resctrl_available_mbm_cntrs_show(struct kernfs_open_file *of,
>> return ret;
>> }
>>
>> +static int event_filter_show(struct kernfs_open_file *of, struct seq_file *seq, void *v)
>> +{
>> + struct mon_evt *mevt = rdt_kn_parent_priv(of->kn);
>> + bool sep = false;
>> + int i;
>> +
>> + for (i = 0; i < NUM_MBM_TRANSACTIONS; i++) {
>> + if (mevt->evt_cfg & mbm_transactions[i].val) {
>
> mevt->evt_cfg could possibly be changed concurrently. This should
> be protected with the rdtgroup_mutex.
Sure.
>
>> + if (sep)
>> + seq_putc(seq, ',');
>> + seq_printf(seq, "%s", mbm_transactions[i].name);
>> + sep = true;
>> + }
>> + }
>> + seq_putc(seq, '\n');
>> +
>> + return 0;
>> +}
>> +
>> /* rdtgroup information files for one cache resource. */
>> static struct rftype res_common_files[] = {
>> {
>> @@ -2019,6 +2040,12 @@ static struct rftype res_common_files[] = {
>> .seq_show = mbm_local_bytes_config_show,
>> .write = mbm_local_bytes_config_write,
>> },
>> + {
>> + .name = "event_filter",
>> + .mode = 0444,
>> + .kf_ops = &rdtgroup_kf_single_ops,
>> + .seq_show = event_filter_show,
>> + },
>> {
>> .name = "mbm_assign_mode",
>> .mode = 0444,
>> @@ -2279,10 +2306,48 @@ int rdtgroup_kn_mode_restore(struct rdtgroup *r, const char *name,
>> return ret;
>> }
>>
>> +static int resctrl_mkdir_event_configs(struct rdt_resource *r, struct kernfs_node *l3_mon_kn)
>> +{
>> + struct kernfs_node *kn_subdir, *kn_subdir2;
>> + struct mon_evt *mevt;
>> + int ret;
>> +
>> + kn_subdir = kernfs_create_dir(l3_mon_kn, "event_configs", l3_mon_kn->mode, NULL);
>> + if (IS_ERR(kn_subdir))
>> + return PTR_ERR(kn_subdir);
>> +
>> + ret = rdtgroup_kn_set_ugid(kn_subdir);
>> + if (ret)
>> + return ret;
>> +
>> + for_each_mon_event(mevt) {
>> + if (mevt->rid != r->rid || !mevt->enabled || !resctrl_is_mbm_event(mevt->evtid))
>> + continue;
>> +
>> + kn_subdir2 = kernfs_create_dir(kn_subdir, mevt->name, kn_subdir->mode, mevt);
>> + if (IS_ERR(kn_subdir2)) {
>> + ret = PTR_ERR(kn_subdir2);
>> + goto out_config;
>> + }
>> +
>> + ret = rdtgroup_kn_set_ugid(kn_subdir2);
>> + if (ret)
>> + goto out_config;
>> +
>> + ret = rdtgroup_add_files(kn_subdir2, RFTYPE_ASSIGN_CONFIG);
>> + if (ret)
>> + break;
>> + }
>> +
>> +out_config:
>> + return ret;
>
> No "config" is happening here so the goto label can just be "out".
>
Sure.
>> +}
>> +
>> static int rdtgroup_mkdir_info_resdir(void *priv, char *name,
>> unsigned long fflags)
>> {
>> struct kernfs_node *kn_subdir;
>> + struct rdt_resource *r;
>> int ret;
>>
>> kn_subdir = kernfs_create_dir(kn_info, name,
>
>
> Reinette
>
--
Thanks
Babu Moger
