Message-ID: <6713e2e9-827d-8f3d-0923-a03b6b072383@intel.com>
Date:   Thu, 16 Feb 2023 17:50:17 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.11.0
Subject: Re: [RFC v2 PATCH 7/7] x86/resctrl: Add debug files when mounted with
 debug option
Content-Language: en-US
To:     Babu Moger <babu.moger@amd.com>, <reinette.chatre@intel.com>
CC:     <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>,
        <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
        <corbet@lwn.net>, <linux-kernel@vger.kernel.org>,
        <linux-doc@vger.kernel.org>, <eranian@google.com>,
        <peternewman@google.com>
References: <167537433143.647488.9641864719195184123.stgit@bmoger-ubuntu>
 <167537445777.647488.12333202522131465679.stgit@bmoger-ubuntu>
From:   Fenghua Yu <fenghua.yu@intel.com>
In-Reply-To: <167537445777.647488.12333202522131465679.stgit@bmoger-ubuntu>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?KzJGdENpYk5PZXhWbmNMWDdpcUd3RHUvb250REJ2dEhNem56NXVSQkpLbi9Z?=
 =?utf-8?B?SG1rVkxoTVc2WkVFeDd6Q3NRQ3hlTjM5OWVVVlhmT0grWkZFM1RVRUo0N0Zj?=
 =?utf-8?B?bHpwTWJubnBtM0ZwS1dXdy9QR09kd2ZXaXlBNUJVdTlycHZYZWlOK0oyM1hW?=
 =?utf-8?B?TGpVcXFaUGgvUStFdE9tMFZ4a3VEMEY0TmRUVXNOWVFpVTV6SjQweWsxWmhx?=
 =?utf-8?B?d0thREdDMHorRVJGSmxYeDlvZEN5aXdoY0hhMjVVN2h1RXZTY2RTUU5MUkFX?=
 =?utf-8?B?em5mZklJVm03UDA5Tm9nK0JGUEQvamJueEFsWG1PeFI1TWh3SUw1L01lTzJx?=
 =?utf-8?B?cFBqT2tzN2pDYW5jUnZIZnFyTHJ0UE5wdzNkTDN0SC82TDdYVTdvUFRQdS9r?=
 =?utf-8?B?UkFhYXB0aE9UMUJueitjc1ROWFhzVWZVVGRMVUhlUzlWTjlZaTVPWFh4OFBB?=
 =?utf-8?B?cFdvYnFEclJPZUphWXdDcUw0bGFTZmhUVDR2UWR6YlZWZVZJUmZHWFJOdFpE?=
 =?utf-8?B?cXJ2S0Q4M1ora3BFZ2l0Q1NzTGZIdHdIS3h5dFQvMkFOVFJOc2s4cVI1bEI3?=
 =?utf-8?B?aHpWMVg0bUd6MlJoeVNaYkl1U2JubjN0MlljbEdCMEVGallTS1NLQ3FLRFBN?=
 =?utf-8?B?TnRFQStvRWhNd0V3S0I3YkUvdHc5bXAvNkVLMXVQczMzYTlrL3dJWktDSnpm?=
 =?utf-8?B?WXJheVVNNUhVYXEzcXl5S3dYNHZNZ1g1UDZxQ0lLLy9vUFVtRzZTWWcrWUw4?=
 =?utf-8?B?ait4SVY2SzhIUFR4NUFZRithT0JmSE5zeVc5MHNUTnpTNWhjRDF6UCtnZktW?=
 =?utf-8?B?Q09ZcjR3SVZoZ1FlK1QvODR4RjM5MnhXdXZhUm5lVUtCNE1nZzByTzU2L0ND?=
 =?utf-8?B?R1pKdzVmQVl4VWhRZ29wS0RZQXJZT1JmbGpXY1lPdXNuV0IxbUlXcTNOZVp0?=
 =?utf-8?B?N3ZKWjZjL2VBc1VVaERteGQyYnBmRlZxbXNxU3duQW1pWnZXOG9LSG1BQUZw?=
 =?utf-8?B?MS9PSmsyWnM5Ry81bmgzTk9pTEVDYlpZc2todVQxdzlYejh3WlNWTDlEOTBT?=
 =?utf-8?B?Ly9wTkprQ0ZuQWpBYW82SGFLTGZ3YXlrRzVROTc2cFpLT3Uyb1I3RDI1T3pM?=
 =?utf-8?B?cGRSNzVicURFUmJNNkJVczMreHUzbTlJUitnOUFmOW5wazhqeXF5NnA1Uk1j?=
 =?utf-8?B?QnJVMXpXc2luT0lZcy9jSTdraWxrV21TK1ZPc1AxZDVlZXJRUDdQTVBsa0x5?=
 =?utf-8?B?b044Yjl3RUZsakdmdnVibVMwTTBSMFhjV0FPdnR6WG5lTE1RaDhTYVVFWkM1?=
 =?utf-8?B?MXVmQjFObkhmeDFZblFQY1ZEc0pSWnZCcEYveEV2cmZadjM2Sm55U1gxMHZV?=
 =?utf-8?B?Zk1BZ3dZTmFMd0s1dmgxSkxtMFBlbThyYVY0N0gxcXIwTCs1U1VQNGROS3RH?=
 =?utf-8?B?L3lGd1F4SWExWTM0Y3N5di9MMUpnazFuOWVHaHBvbXRlazNEdzdDVk05cmx4?=
 =?utf-8?B?ZlJxK0RHYzdFSkZEOTJzVW9UTlpoWEpobWFzVHg5WC8yaDFrQmQ1SmJLSG9G?=
 =?utf-8?B?bHhCZ01hU3BYUFRlamtKQmxVV3VCb0MwdUhkVjFDQ1hGR2ZuM0t3OGs1YTdS?=
 =?utf-8?B?aURMdjZzejlza1FXZU9SN3phaEJSSzVuT21BNzhPWVVMVUh6bFcwNUkySWpq?=
 =?utf-8?B?a2dnZGZRdEdTamdVZ2diU2h4S2N4ZkNJOWJ5bkhZeVF4bVNYNnh3ZDF2clIw?=
 =?utf-8?B?MXhmcjc4MGlEbDhjQk5wYkJNYW9zbi9lVGc4ckUyS2hnQzlPVWVSOWl0bXZD?=
 =?utf-8?B?eEVydGQ1MWhsMjB5c2VnNkw2cUtoeld0VUhPeUd3QklTKzhVNkllSVMzVWZx?=
 =?utf-8?B?eGVJazZxTDFDeUZDeG1SaWh6VXV2UXBVTFcvZ0syRk9uc3Uxdy9qRjFEcllX?=
 =?utf-8?B?UlpBRlk3TnRYMTBlZjVtWHo2elFOWFgzTzUzZVloUGZJTlZkUUZ4dWlqMTJC?=
 =?utf-8?B?ckJTM29BbnNucDlnaUdkZ3V4YjJxMDEyb1IzNnFMdmYwMWduK3JkRGhNMUtH?=
 =?utf-8?B?UU4wT0s0N2s1SUtRYW9QOXNHMFZaRW1ybW5Nb2VqMXFwQ2J3RVl6cjNNTkp3?=
 =?utf-8?Q?m3q8jwSe2jRwVwJv2BoOfDmW6?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 088f9f70-e2ac-408f-dbb9-08db1089526b
X-MS-Exchange-CrossTenant-AuthSource: IA1PR11MB6097.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Feb 2023 01:50:18.4608
 (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: BeA8tpgKlmofbXt0kgZIb1yeIrVAzWb7V40Aw//vs2yYqv82XQl0s/B3yZ67zsxeBmdPynYR7nst+2VzJXC/kA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5244
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi, Babu,

On 2/2/23 13:47, Babu Moger wrote:
> Add the debug files to the resctrl hierarchy.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |   27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index c35d91b04de6..b7c72b011264 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2398,6 +2398,31 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>   			     struct rdtgroup *prgrp,
>   			     struct kernfs_node **mon_data_kn);
>   
> +void resctrl_add_debug_file(struct kernfs_node *parent_kn,
> +			    const char *config, unsigned long fflags,
> +			    bool debug)
> +{
> +	struct rftype *rft;
> +
> +	rft = rdtgroup_get_rftype_by_name(config);
> +	if (debug && rft) {
> +		rft->fflags |= fflags;
> +		rdtgroup_add_file(parent_kn, rft);
> +	} else if (rft) {
> +		rft->fflags &= ~fflags;
> +		kernfs_remove_by_name(parent_kn, config);
> +	}
> +}
> +
> +static void resctrl_add_debug_files(bool debug)
> +{
> +	resctrl_add_debug_file(rdtgroup_default.kn, "rmid",
> +			       RFTYPE_BASE, debug);
> +	resctrl_add_debug_file(rdtgroup_default.kn, "closid",
> +			       RFTYPE_BASE_CTRL, debug);
> +	kernfs_activate(rdtgroup_default.kn);
> +}
> +
>   static int rdt_enable_ctx(struct rdt_fs_context *ctx)
>   {
>   	int ret = 0;
> @@ -2411,6 +2436,8 @@ static int rdt_enable_ctx(struct rdt_fs_context *ctx)
>   	if (!ret && ctx->enable_mba_mbps)
>   		ret = set_mba_sc(true);
>    > +	resctrl_add_debug_files(ctx->debug);

It's better to change to:
+	if (ctx->debug)
+		resctrl_add_debug_files();

Then the functions in the call chain can remove 'debug' parameter and 
can be simpler.
> +
>   	return ret;
>   }
>   
> 
> 
Thanks.

-Fenghua