Hi Reinette, Forgot to respond to this. On 9/27/23 17:02, Reinette Chatre wrote: > Hi Babu, > > On 9/27/2023 2:34 PM, Moger, Babu wrote: >> Hi Reinette, >> >> On 9/27/2023 1:34 PM, Reinette Chatre wrote: >>> Hi Babu, >>> >>> On 9/15/2023 3:42 PM, Babu Moger wrote: >>>> There are 3 types resctrl root files. >>> Considering patch #4, should this be "base"? >> Sure. >>> >>>> 1. RFTYPE_BASE : Files common for both MON and CTRL groups >>>> 2. RFTYPE_BASE | RFTYPE_CTRL : Files only for CTRL groups >>> If (1) is accurate then I cannot see how (2) can be accurate. >> >> How about ? >> >> 2. RFTYPE_BASE | RFTYPE_CTRL : Files for the CTRL groups > > Yes, this is accurate. Even so, this snippet does not seem > necessary considering the changelog rewrite below. Sure. Removed this whole snippet. > >> >>> >>>> 3. RFTYPE_BASE | RFTYPE_MON : Files only for MON groups >>> Same here. >> >> How bout? >> >> 3. RFTYPE_BASE | RFTYPE_MON : Files for the MON groups > > Same comment. > >> >>> >>>> Files only for the MON groups are not supported now. Add the >>>> support to create these files. >>> This is not accurate. Files "only for the MON groups" are >>> actually the only monitor files that *are* supported. The >>> problem being fixed here is that monitor files are >>> not supported for CTRL_MON groups. >>> >>> I made an attempt at rewriting this commit message but I am >>> doing it quite quickly so please do improve it: >>> >>> Files unique to monitoring groups have the >>> RFTYPE_MON flag. When a new monitoring group is created >>> the resctrl files with flags RFTYPE_BASE (files common >>> to all resource groups) and RFTYPE_MON (files unique >>> to monitoring groups) are created to support interacting with >>> the new monitoring group. >>> >>> A resource group can support both monitoring and control, >>> also termed a CTRL_MON resource group. CTRL_MON groups should >>> get both monitoring and control resctrl files but that >>> is not the case. Only the RFTYPE_BASE and RFTYPE_CTRL files >>> are created for CTRL_MON groups. This is not a problem >>> because there are no monitoring specific files with the >>> RFTYPE_MON flag associated with resource groups. >>> >>> A later patch introduces the first RFTYPE_MON file for >>> resource groups so fix resctrl file creation for CTRL_MON >>> groups to ensure that monitoring files, those with the >>> RFTYPE_MON flag, are also created. >> >> Looks good with slight change(only last para. Rest looks good). Just removed the "fix". >> >> A later patch introduces the first RFTYPE_MON file for >> resource groups. Add the changes to create the files for CTRL_MON >> groups to ensure that monitoring files, those with the RFTYPE_MON flag, >> are also created. >> > > "Add the changes to" is not necessary. With this and removing the "fix" it > can be summarized to: > A later patch introduces the first monitoring specific (RFTYPE_MON) > file for resource groups. Ensure that files with the RFTYPE_MON > flag are created for CTRL_MON groups. > > What do you think? > Yes. Looks good. -- Thanks Babu Moger