Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <8e6fec2d-2262-1f62-8c4e-cedfa8459434@intel.com>
Date:   Wed, 27 Sep 2023 15:02:42 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.15.1
Subject: Re: [PATCH v10 09/10] x86/resctrl: Add support for the files for MON
 groups only
Content-Language: en-US
To:     <babu.moger@amd.com>, <corbet@lwn.net>, <tglx@linutronix.de>,
        <mingo@redhat.com>, <bp@alien8.de>
CC:     <fenghua.yu@intel.com>, <dave.hansen@linux.intel.com>,
        <x86@kernel.org>, <hpa@zytor.com>, <paulmck@kernel.org>,
        <akpm@linux-foundation.org>, <quic_neeraju@quicinc.com>,
        <rdunlap@infradead.org>, <damien.lemoal@opensource.wdc.com>,
        <songmuchun@bytedance.com>, <peterz@infradead.org>,
        <jpoimboe@kernel.org>, <pbonzini@redhat.com>,
        <chang.seok.bae@intel.com>, <pawan.kumar.gupta@linux.intel.com>,
        <jmattson@google.com>, <daniel.sneddon@linux.intel.com>,
        <sandipan.das@amd.com>, <tony.luck@intel.com>,
        <james.morse@arm.com>, <linux-doc@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <bagasdotme@gmail.com>,
        <eranian@google.com>, <christophe.leroy@csgroup.eu>,
        <jarkko@kernel.org>, <adrian.hunter@intel.com>,
        <quic_jiles@quicinc.com>, <peternewman@google.com>
References: <20230915224227.1336967-1-babu.moger@amd.com>
 <20230915224227.1336967-10-babu.moger@amd.com>
 <031ef099-a7d2-37ce-eb97-c4f7d76a12c4@intel.com>
 <7c6dcc06-0b6c-a346-c7ff-60da3fe59fbc@amd.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <7c6dcc06-0b6c-a346-c7ff-60da3fe59fbc@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Tkp2NjZkM2dTS2ZqcysvRUVrOVBGaStEalViT3Ric3FMT0g5TlhhVXFNM2U3?=
 =?utf-8?B?aU94UURqOTNyQ3FpeTJYVElKYmg5R1RIRDdhWG51YXVlQ0pQMG5qNTFUY3Ro?=
 =?utf-8?B?NndRNjNQQWJrSmxPcDU2RC9iNW1oYVlDQlE1Y3RGYk9xNWxzZFFmdTdrblZz?=
 =?utf-8?B?YjdYSngxU1E0dmt0TFR0MTlHUzF5TlNlWnNlVHc1emIzbmdoZ1hLSUdnSmZV?=
 =?utf-8?B?NS9Oem1MTWE1d2dBRWxQY0JmSGE3OHpjVnpSaFJVczlzQ3JHYUJldW8xSmsy?=
 =?utf-8?B?OEJOTS9wcXFMWGNxdHVZVzF5ZkFjNy81RExpNzR5MjBtQ0dab1AwZDNLSlZW?=
 =?utf-8?B?ZFJ4R0tDU2ljY055K1RFUHFJdjRZSmt4MjVjcUJpM1NSeWNWNDNDMmFuWmE4?=
 =?utf-8?B?UUlDaXdqank0K00zVzFvV21zRkhNZ2JlbGMwbjVLUkVSSkRrSHduVjN3V1Qz?=
 =?utf-8?B?YUNsNTlSUktwN1Y3SHRoaG4waGo2bFRSUUhuWGFRUFp2akorZWVsWVNiN2U1?=
 =?utf-8?B?alB3MHFjOTFPWUFuaTN1L3RZTWJKTm9IczBjelVVRnZEUXF4NUlxdW9xeU83?=
 =?utf-8?B?TG1XeCtZSklKQlFSZm42VnB0NERvUEVQTEZTWVFkUisvYnRvUkp4Rkw5RHZn?=
 =?utf-8?B?eVpJM0ZYWnIyWlFXYTlqRVptUUlwUUhZY0RJY215bUFFek54aUFYQUZua0VW?=
 =?utf-8?B?c251MzluU1JoSHlRb1Vic1dTaHhKOHZlTnkzVVZJOXppd3NXZXhwS1FkaU9L?=
 =?utf-8?B?OUQxeE5aU3JtUDl4aFFGNlBYNlVGZTVBeWVhM3QvakFreWdabzRueUllcktD?=
 =?utf-8?B?Z29Nc3BrZzAzUHptazV6eVhKWHpRMTNXYk9Gd1Bic29iUUM2c01pRjM0Y3lk?=
 =?utf-8?B?eFJFZWxXRDQ5TkF6YzdTSEN2N2lVMHp5Y1VVV2JZV2hXdktERnF4Y0RVK01L?=
 =?utf-8?B?cXBkRGhrM2dPN3RUV3pZV3pyN0Q5WEV1MzJPcit6OTFORzFyUWdKM2lOK0tN?=
 =?utf-8?B?WDBTTFZ6ZjdBQzVLQ0xwY2lkYkYwR1RRclVXS1dqR1UyWXJ1MHYwa1F4c0hy?=
 =?utf-8?B?ZnE1cHU5aSt0Rkc1ZFpKdjJ1OWJQTlAxVWJGNlZ3SGgwZHM3M0diSzdsL2Zw?=
 =?utf-8?B?SWNiQzlvZ0JONHFSV2FrZ2dETjdoSVc4ZHJEWW9lei9YOFRhNTdKNnhyWnJX?=
 =?utf-8?B?TExIa3FmMTlLeHA0OFlsRFplc1R4KzJKb3pwM1VBQzFuSlBpTnRZL293UGd2?=
 =?utf-8?B?VkdZUVJRenhsK2QrL0RCUE1VUktrQmsvUHM3OXVIaSsxYjA4NlA3T1Vsd09l?=
 =?utf-8?B?ZjlVc1ZTSHVLTFp1MmY2c2hTbUhyd1ZmRzYwR20zQlhyWUNXNjFGRlhQdGRy?=
 =?utf-8?B?Sm5DT0RRYkpLcHduVFQ2Wm5WUk0wRnMwWXkzVlNQS1NLemJmWUJvNHNoczd2?=
 =?utf-8?B?UWo0dmo1Z0cxL1dkd0xlSXlIM2ttZkZROXJmWFpyZ0w2akc5ejZnUDZkVjRa?=
 =?utf-8?B?K0RLajFKZjdQVFVWR2dHU2lnNlNMV21NREM5WVdyS0xramczTmUyS0Yvb0Rz?=
 =?utf-8?B?SXA2dkRDWGk4b2J2Qmphcnp3c2dkeGxjS2h3WmpWcVBML0ZuRHhMck1IVity?=
 =?utf-8?B?THhJaElqMHpwYUYwY1NFQWEwbElOSm9pM3ROejJxdlUwUFp2cHNHQVJxRnZ4?=
 =?utf-8?B?amZDS1pMN3pFZGZWcXdVOXQwSGdMQms1bTBlWFRQNk5NdEliOTFiUDBwUHNZ?=
 =?utf-8?B?MmdPcGVHcDBMVmxKWFczQnIwK3h4V0I2TlpSWURyS01WcXRscHpQNlZucWxE?=
 =?utf-8?B?WWtoK3JpZUJuVXBaeURESEZZNmZCejBhL1llaDRYeEFOTlkvWHB4d3dSRjd2?=
 =?utf-8?B?MG14N0RhUFZ6cTFBRVhIaDVxWlc5V1pWMSsycmlzQzFBWXd1amdHWTdYM2VQ?=
 =?utf-8?B?SlhpV0EwV3RFcmpPR2pJcU9nbjI3dHNQckY4YThUZHRGaHcyMW4rTXNjZlhQ?=
 =?utf-8?B?aWpXWU1uanphQnljT01LSW9GWXFwMmMzbndVQ1lHbjJNdDIyNlZwckhuS2hm?=
 =?utf-8?B?c3hCUlBVSmpKL2IyLzlzZUtkYktsLzhiT0JOeUpSTUN5T1Q1NWp5SEZmNVJY?=
 =?utf-8?B?cTZuNC9aSmQ1VGV5YWQrazhSaXpqNXJKQU1OdjhXSjVqVkRsMWNHSEJkL3pH?=
 =?utf-8?B?alE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8edc0dde-fe38-4479-3985-08dbbfa57b9c
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Sep 2023 22:02:46.8888
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CkmROXm/IbqMMSFcasw3hZYxLOjh7wjE+vDtwFBlg3QCAU5coEf5jPg8w7OhIk7iQEutC4umpV/kXAunGgK2YGfeFYXSh07mT77ckFkuHX8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB7915
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Wed, 27 Sep 2023 15:03:59 -0700 (PDT)

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.

> 
>>
>>> 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?

Reinette