Re: [PATCH v15 28/34] fs/resctrl: Introduce mbm_assign_on_mkdir to enable assignments on mkdir

["Moger, Babu" <babu.moger@amd.com>]

Hi Reinette,

On 7/15/25 09:27, Reinette Chatre wrote:
> 
> 
> On 7/15/25 7:18 AM, Moger, Babu wrote:
>> Hi Peter,
>>
>> On 7/15/25 08:53, Peter Newman wrote:
>>> Hi Babu,
>>>
>>> On Wed, Jul 9, 2025 at 12:22 AM Babu Moger <babu.moger@amd.com> wrote:
>>>>
>>>> The "mbm_event" counter assignment mode allows users to assign a hardware
>>>> counter to an RMID, event pair and monitor the bandwidth as long as it is
>>>> assigned.
>>>>
>>>> Introduce a user-configurable option that determines if a counter will
>>>> automatically be assigned to an RMID, event pair when its associated
>>>> monitor group is created via mkdir.
>>>>
>>>> Suggested-by: Peter Newman <peternewman@google.com>
>>>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>>>> ---
>>>> v15: Fixed the static checker warning in resctrl_mbm_assign_on_mkdir_write() reported in
>>>>      https://lore.kernel.org/lkml/dd4a1021-b996-438e-941c-69dfcea5f22a@intel.com/
>>>>
>>>> v14: Added rdtgroup_mutex in resctrl_mbm_assign_on_mkdir_show().
>>>>      Updated resctrl.rst for clarity.
>>>>      Fixed squashing of few previous changes.
>>>>      Added more code documentation.
>>>>
>>>> v13: Added Suggested-by tag.
>>>>      Resolved conflicts caused by the recent FS/ARCH code restructure.
>>>>      The rdtgroup.c/monitor.c file has now been split between the FS and ARCH directories.
>>>>
>>>> v12: New patch. Added after the discussion on the list.
>>>>      https://lore.kernel.org/lkml/CALPaoCh8siZKjL_3yvOYGL4cF_n_38KpUFgHVGbQ86nD+Q2_SA@mail.gmail.com/
>>>> ---
>>>>  Documentation/filesystems/resctrl.rst | 16 ++++++++++
>>>>  fs/resctrl/monitor.c                  |  2 ++
>>>>  fs/resctrl/rdtgroup.c                 | 43 +++++++++++++++++++++++++++
>>>>  include/linux/resctrl.h               |  3 ++
>>>>  4 files changed, 64 insertions(+)
>>>>
>>>> diff --git a/Documentation/filesystems/resctrl.rst b/Documentation/filesystems/resctrl.rst
>>>> index 06c8c08d0341..f9e9a8a66cae 100644
>>>> --- a/Documentation/filesystems/resctrl.rst
>>>> +++ b/Documentation/filesystems/resctrl.rst
>>>> @@ -354,6 +354,22 @@ with the following files:
>>>>           # cat /sys/fs/resctrl/info/L3_MON/event_configs/mbm_total_bytes/event_filter
>>>>            local_reads,local_non_temporal_writes
>>>>
>>>> +"mbm_assign_on_mkdir":
>>>> +       Determines if a counter will automatically be assigned to an RMID, event pair
>>>> +       when its associated monitor group is created via mkdir. It is enabled by default
>>>> +       on boot and users can disable by writing to the interface.
>>>> +
>>>> +       "0":
>>>> +               Auto assignment is disabled.
>>>> +       "1":
>>>> +               Auto assignment is enabled.
>>>> +
>>>> +       Example::
>>>> +
>>>> +         # echo 0 > /sys/fs/resctrl/info/L3_MON/mbm_assign_on_mkdir
>>>> +         # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_on_mkdir
>>>> +         0
>>>> +
>>>>  "max_threshold_occupancy":
>>>>                 Read/write file provides the largest value (in
>>>>                 bytes) at which a previously used LLC_occupancy
>>>> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
>>>> index 0722e72f6cb1..ccca2330eebd 100644
>>>> --- a/fs/resctrl/monitor.c
>>>> +++ b/fs/resctrl/monitor.c
>>>> @@ -984,6 +984,8 @@ int resctrl_mon_resource_init(void)
>>>>                 resctrl_file_fflags_init("available_mbm_cntrs",
>>>>                                          RFTYPE_MON_INFO | RFTYPE_RES_CACHE);
>>>>                 resctrl_file_fflags_init("event_filter", RFTYPE_ASSIGN_CONFIG);
>>>> +               resctrl_file_fflags_init("mbm_assign_on_mkdir", RFTYPE_MON_INFO |
>>>> +                                        RFTYPE_RES_CACHE);
>>>>         }
>>>>
>>>>         return 0;
>>>> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
>>>> index 4889e7556cc7..b7289ce2b3a6 100644
>>>> --- a/fs/resctrl/rdtgroup.c
>>>> +++ b/fs/resctrl/rdtgroup.c
>>>> @@ -1987,6 +1987,42 @@ static ssize_t event_filter_write(struct kernfs_open_file *of, char *buf,
>>>>         return ret ?: nbytes;
>>>>  }
>>>>
>>>> +static int resctrl_mbm_assign_on_mkdir_show(struct kernfs_open_file *of,
>>>> +                                           struct seq_file *s, void *v)
>>>> +{
>>>> +       struct rdt_resource *r = rdt_kn_parent_priv(of->kn);
>>>> +
>>>> +       mutex_lock(&rdtgroup_mutex);
>>>> +       rdt_last_cmd_clear();
>>>> +
>>>> +       seq_printf(s, "%u\n", r->mon.mbm_assign_on_mkdir);
>>>> +
>>>> +       mutex_unlock(&rdtgroup_mutex);
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>> +static ssize_t resctrl_mbm_assign_on_mkdir_write(struct kernfs_open_file *of,
>>>> +                                                char *buf, size_t nbytes, loff_t off)
>>>> +{
>>>> +       struct rdt_resource *r = rdt_kn_parent_priv(of->kn);
>>>> +       bool value;
>>>> +       int ret;
>>>> +
>>>> +       ret = kstrtobool(buf, &value);
>>>> +       if (ret)
>>>> +               return ret;
>>>> +
>>>> +       mutex_lock(&rdtgroup_mutex);
>>>> +       rdt_last_cmd_clear();
>>>> +
>>>> +       r->mon.mbm_assign_on_mkdir = value;
>>>> +
>>>> +       mutex_unlock(&rdtgroup_mutex);
>>>> +
>>>> +       return 0;
>>>
>>> I noticed that "echo 0 > mbm_assign_on_mkdir" never completes. This
>>> should return nbytes.
>>>
>>
>> Agree. Thanks for pointing it out.
> 
> Not the first time:
> https://lore.kernel.org/lkml/7fcdb142-b188-4f8d-ab26-391e0b9cec8e@intel.com/

How did I miss that? My bad. Will take care of it next revision for sure.
-- 
Thanks
Babu Moger