Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2a48c57a-ff8f-b657-8138-a5da6c7806db@intel.com>
Date:   Fri, 4 Aug 2023 13:42:10 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.13.0
Subject: Re: [PATCH v6 6/8] x86/resctrl: Move default control group creation
 during mount
Content-Language: en-US
To:     Babu Moger <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: <168980872063.1619861.420806535295905172.stgit@bmoger-ubuntu>
 <168980893158.1619861.8945141367681744527.stgit@bmoger-ubuntu>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <168980893158.1619861.8945141367681744527.stgit@bmoger-ubuntu>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OHZFcXd5ODNNMzhkcXFUNEV1cFkzVWc4b0ZHRkREdkFDOXdHYiszaklYcWtS?=
 =?utf-8?B?T3Q2Z3JFUzZvckVKaER0Sk1QNFVRWHhxTkhkRHI4SUdGaHN5UDVsdWFWWjRM?=
 =?utf-8?B?WmJMUzVJK1ZLL2tGeTMvSE8rdXlHR09LOUNleWE4ZlM3RUlsYXdzYlU2MGpY?=
 =?utf-8?B?bCs5RkVsNlNEckhRaUdBY3NPVHpsYm5ISmZJcFR0Ujg1KzBoc1VTMVE1L0tY?=
 =?utf-8?B?ZTdvaWdHUWF5cnNPdzl0eSs2RW5aMmhEaVBCVkRFWFYwTFBjVmtPc1p0OVY0?=
 =?utf-8?B?cThodmFYVUtGZTJTUXZGUjZXYmtqc3RtdzQxejhTWUtrUlBOMU16VTFjTEFN?=
 =?utf-8?B?NnFzRXZZTGluMm9JaDE0aTVvWmI1ZW9yVVROSXAyUWFQcjhqNDRnTVlsR0JX?=
 =?utf-8?B?OENtRStEN29FSHJyWTdYclNMVmVLSVkyNnVIUzkyMTRQejhUQWhydzdGemIw?=
 =?utf-8?B?NFhpYnZuT0RWQWtnakxFT1Nkcld3OENFWnM0RWZSc1ZFZERQK1B5K1A4NHRQ?=
 =?utf-8?B?bzVYaVgzMDBBWWVsakNzOHFqNEtRaWNjS3NqTkwzTGZnQ01FUXJNV0I1M1pP?=
 =?utf-8?B?OE9tS0NpV21FTUFDSEZRbEFGQ0s4NWhMQkQrVCtqZk00Nm80cWMrNGlJUm10?=
 =?utf-8?B?RG84Z2JjWngvTmRhSjhhNlprczVlOG4vUVJCdHhCMzBCc2Y1VkF4RUZvaXVR?=
 =?utf-8?B?eEZML1d5cHloaGxhSHl6SjNXamtEMUw2aTFreE5nZUhEK0hPTTFGMzRRWDVm?=
 =?utf-8?B?OUk3RndkQTVYUXFHMUZvamNqRkVVbEZ5V1M3M011cHFzNG5Ca3lYS3hvMnBH?=
 =?utf-8?B?MnRMMFY4WHlLZmpxWGlzS1g2N0dWVkdWTnFIendpUm5UVlIvWVFaS0lvUTZh?=
 =?utf-8?B?ZCttc3U5UlVMRnFBZklRT1V3NG94RVV2R09sYXlNWEl3OEFiZFpFOWJXZWhp?=
 =?utf-8?B?WWl3MExYL2RGTjFxUHM1VGgxR0g2Wm9sOEtFdXdOTitwWUw0WmxDREQ1aTlm?=
 =?utf-8?B?WEhuT1JwWklySTFaRWRKdE83S2IxbUM5VmxsVjBDb1FUZndVVUt5MU9sL1FW?=
 =?utf-8?B?bC92UzJZc3Jub1d6V3FkamxJQ0VtWWFabC8xQ1ErZnlCZHUwNjdRQS9yWllz?=
 =?utf-8?B?OGVYWnJnMTJWWjkxRk9OR2tzeWF5M04vS3dLazRVQ3d4ZmtMbEoyTUxSY0to?=
 =?utf-8?B?Nk52OEppa2dFMWdHWjhJRTAxNytJQ0ZFWUFuU3luZ0JydER6U2dPblJKMEZR?=
 =?utf-8?B?Z1pxTkcxWm82U25VdCtmaUNVUUU5clBBN2VuVkpvSUtUSnI0eXhrU0FQVjMr?=
 =?utf-8?B?cGRDeDVjK2hqcEJxWnpEdGdLZUErdWVmRHpXMDgwOS9DdExZSFZrV2NGbFhu?=
 =?utf-8?B?REVvV05RM2NqalFNc0d2NGVuKzgzY3ZSN2VmbDVodlVqVktsNXBrZW4zTXJ0?=
 =?utf-8?B?UVRsdXdzNC83NDVNUjM3L1dFdGdSTHFralBRUVNRbTRXeVlpSTdKZVJ4U1VC?=
 =?utf-8?B?ZGp0dWxTZk9JRzh2Z3FiSUJUU21pV3NxM3Q4WGplb0pPS1Q2bUhJdUk1azhE?=
 =?utf-8?B?SGgvOG9GK1AyR2h0MXpLWTF0Q055VGg2QmZid0F4eUpNbWwvUlhiUXgxNnpW?=
 =?utf-8?B?UnVDQnYzUGVrc2hxM3p1N1NIUHY2cXJqSVNjOGFpYjlIOTMrV2RaODN6SEtT?=
 =?utf-8?B?VllXV28vMEdSZFRRbkQybVdhY0pCNEc3MmxxMlVHdEVsL0lEWERzeXRvQloy?=
 =?utf-8?B?T3UvME9OMmtvOW1zUWk5NmUrRzVNcFVDY0NiNURnZjBBekZKRURxNHpQc09E?=
 =?utf-8?B?V2pSWGdKWkRsMlpHUEpPQkFra0lleHFFdXM4bGVSdE1yYzVoSGFqbmVqUlZN?=
 =?utf-8?B?SGU0NkRpeTlKY1EyVkJLWFVndHZaUjBXYnVRUjY1S2tTZmZXclJLZ21UZVF3?=
 =?utf-8?B?UmxoaElFeFBNY2NlZzNEa3FiSjhFZWRkYlVCUnVZMWJXNkVodGZVYjZlTi9C?=
 =?utf-8?B?Q2JnaUxuMXY0dUNpSG1namxCSzBtdy9tOU9Obzd3ZEtUZkFqZG5zMjhaNFdD?=
 =?utf-8?B?SWVvRTM2ZnpDZVMza1ZldDcrWGlNeU81K2pvdmhtaXpRV21UL2NiRzdQUTJN?=
 =?utf-8?B?WDY5d1U4b2dzcXBRSkVBZVJOdVR0NVJpMlc4V2lYTXNrWFpPVTRZa2NWR3FE?=
 =?utf-8?B?dVE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 05fc7870-288a-4c02-20d1-08db952b4892
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Aug 2023 20:42:13.8051
 (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: lDccbEV9KCtjRYl6FnCzSoJiUYP6vbqnCwADQpDIS3GrLuKoY9EovNBj2G9lc0/aLV/JYvXebFH9eaUG2ZQVPCh54BOQVvlXmiWr58v17l4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5077
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Babu,

On 7/19/2023 4:22 PM, Babu Moger wrote:
> The resctrl default control group is created during kernel init time. If
> the new files are to be added to the default group during the mount based
> on the mount option, then each file needs to be created separately and
> call kernfs_activate.
> 
> This can avoided if all the files are created during the mount and
> destroyed during the umount. Move the root and default group creation
> in rdt_get_tree and removal in rdt_kill_sb.

Please use () to indicate function names.

> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

...

> @@ -3704,10 +3720,8 @@ static struct kernfs_syscall_ops rdtgroup_kf_syscall_ops = {
>  	.show_options	= rdtgroup_show_options,
>  };
>  
> -static int __init rdtgroup_setup_root(void)
> +int rdtgroup_setup_root(void)
>  {
> -	int ret;
> -
>  	rdt_root = kernfs_create_root(&rdtgroup_kf_syscall_ops,
>  				      KERNFS_ROOT_CREATE_DEACTIVATED |
>  				      KERNFS_ROOT_EXTRA_OPEN_PERM_CHECK,
> @@ -3715,28 +3729,17 @@ static int __init rdtgroup_setup_root(void)
>  	if (IS_ERR(rdt_root))
>  		return PTR_ERR(rdt_root);
>  
> -	mutex_lock(&rdtgroup_mutex);
> -
>  	rdtgroup_default.closid = 0;
>  	rdtgroup_default.mon.rmid = 0;
>  	rdtgroup_default.type = RDTCTRL_GROUP;
> +	rdtgroup_default.mode = RDT_MODE_SHAREABLE;
>  	INIT_LIST_HEAD(&rdtgroup_default.mon.crdtgrp_list);
>  
>  	list_add(&rdtgroup_default.rdtgroup_list, &rdt_all_groups);
>  
> -	ret = rdtgroup_add_files(kernfs_root_to_node(rdt_root), RFTYPE_CTRL_BASE);
> -	if (ret) {
> -		kernfs_destroy_root(rdt_root);
> -		goto out;
> -	}
> -
>  	rdtgroup_default.kn = kernfs_root_to_node(rdt_root);
> -	kernfs_activate(rdtgroup_default.kn);
>  
> -out:
> -	mutex_unlock(&rdtgroup_mutex);
> -
> -	return ret;
> +	return 0;
>  }

I am missing something here. Why is it now needed to re-initialize
and add default resource group on every mount of resctrl? I expected
only the kernfs related changes to move.

Reinette