Message-ID: <0665a64b-a83f-99d9-02a3-17931b6f0f4d@amd.com>
Date:   Fri, 17 Feb 2023 11:29:39 -0600
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Reply-To: babu.moger@amd.com
Subject: Re: [RFC v2 PATCH 5/7] x86/resctrl: Introduce -o debug mount option
Content-Language: en-US
To:     Fenghua Yu <fenghua.yu@intel.com>, 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>
 <167537444491.647488.11815488030989406143.stgit@bmoger-ubuntu>
 <04a598d8-eeee-9512-7db9-cfeae33999a7@intel.com>
From:   "Moger, Babu" <bmoger@amd.com>
In-Reply-To: <04a598d8-eeee-9512-7db9-cfeae33999a7@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d2J3YmlrQUZwTnVsWXAvZ0tXalN0blJVMlZuaGV6Z3hHY08rajUzejFKZWtM?=
 =?utf-8?B?bnN0ei9BaGYvOVVrb0t5VHdnckZyOGRwVC9GZEhRNEx5dllqQm52TmlTMU9U?=
 =?utf-8?B?QUNWaXkvcmxvSXNUOEtWYnlhc2l3dWw2NU9YNjR0QkprVmVQYXZoSG1UQ2VC?=
 =?utf-8?B?MGd4V0N6MVFIMzQ3Umo5Qi9QY3I1SW1RaUZJdDVub0RMdVQ3eUFiWHREZEhR?=
 =?utf-8?B?d2ZWdkZZVURWeXhaMGpnQVl2NDB0S0F2UFVXMkJibzZyZjZmWmVDb2JxeEpQ?=
 =?utf-8?B?TmhIUGlud1AzVVZ3VDJ2L0RlUUY1MmNWTnVKcVZyQnlMT2JtTjNFMzR4aWNs?=
 =?utf-8?B?VEtZME45RmdtYi9rRGJ5eUhxYkJhRzYzc05ickhHOTJkdGpLZE51N2IrNzBX?=
 =?utf-8?B?NlF3a2tpL3BweHhiYnBWQ2Q2d0pPNHNNNnFJUUtGbGx6MHhFRTV3bkpscFRj?=
 =?utf-8?B?MlMwSkpVaFlPZkhHNUI5KzVPV1NXbWdZR2xJN29ZYUNUY0owUjlCYU9uQ3Qr?=
 =?utf-8?B?bVdOc3JibC93NjJQK1hwVkY0cW9KVUxuRVA3bDd6ZlYyT3JtdjJDOVJEUURt?=
 =?utf-8?B?cUF0UDFtanUwNVhHZzlLR2JqdFk4RGQxTE8vZ1pvZ2lYcmxoVVZ0N1pGQTB4?=
 =?utf-8?B?QlhkUXYvRTZaZlY2bEVjSEx5ZzdPaGtlNU1CQk8raGpVMzZJcXJxalR4OXlE?=
 =?utf-8?B?QWZTcHIzUmduV3FmdmVOVFl2WE5mUGJEa1ltK0hRMFphK1lwZGdFVjJ4M3p3?=
 =?utf-8?B?MUpyRWdkTjdGSjE2NThDZVc0cklVQ3h1M0YyVGw1L3ZPK2YySmVZdExBZkRQ?=
 =?utf-8?B?T0o4TEI2anRJT1c5RmJGWGhobUVQQXlHT1ZDN3hVT2VYNGk0UmFUSTErMm5P?=
 =?utf-8?B?MS9nL29WLzM4OHNSaEc2QWNlVkRMNlJtbHdCMXJCUjZaUjRYb3ZEVE15VGwr?=
 =?utf-8?B?ZTJjVkp2Z0plaFpaZVVNR3pINVd5SlgrWTdsT1gzanlKc3d6WUtLWitja1dI?=
 =?utf-8?B?SE9kSG01VVNYNmdSMXRaSjVHMndyTlpHeTFWYU1vRDBmeHI4Y2x3MUxvODRz?=
 =?utf-8?B?cG1CcndwTGNIRlBsUEF1WlBPWFcwRmhMZU0zNVJFRDh2aTZIdWhKeTUvYzJv?=
 =?utf-8?B?VlB1Z0xXb012MW5UOUFqZjd4V3lEbHBsU3kyS3VrOHY5ZTBUZXBvWklFSXlC?=
 =?utf-8?B?MXJCWkprSlF6am1IaW92R1dKSWFXT0g4aTBkL2tCWEViSzIvbmlHcUwwbGxY?=
 =?utf-8?B?RXhBcWlsUTRGSm5rYnkxS0tnQ2d4bEk3TUlTTHBuM1JTUEZibWxkc20vQjhv?=
 =?utf-8?B?Z2V4bHQvNk92Q2hGTWEyYXErdy9kWWZTZzJoN21maGZ4V3JTZzBtVmo1ODRQ?=
 =?utf-8?B?dUNobkJ0Z0dwNUZsWkprdlFWRG1FYkpHRCtGdVk5VFJ4NnRIZDN2T2s5NFlH?=
 =?utf-8?B?ZStRNWx1UEtjb2FLZnJqMTlRNWNHbFVxaURmRkcvTXBnREdjQ05LZnUvNTdY?=
 =?utf-8?B?MytLS2x3QkJLWEgvdFJIdGlWR0Y3ZVhyU2oxanBUVW1WNi93U0FBZW5Jd1dO?=
 =?utf-8?B?SEl4bE1sVXZFME9jRE1TZzJ5N3FIaUh6SXBFMko3S29rUnZoSzJJNkxjUHU3?=
 =?utf-8?B?UVhVUUgzLzNMd2p3Z3R4eUZGd0MxN3VlR2RxUGtIMzhjU3VXT0N5cEVDSU5X?=
 =?utf-8?B?dTRLSzh1UVA3K0swUVhicHgvaWJrdi8rMXNKcHlOL2xBNThPT01tRDdYNzBI?=
 =?utf-8?B?MWowL21ONXl0Q2x4S0tYQkpHdVM0bDh3bmhTZW5xSXBtSnpKSWpuK1E4Z1BJ?=
 =?utf-8?B?TW90Wm4yQUEyRWdBWWtIcS8vWlc1QzdtZXdBWnZqcEtBd0l4RGI2TWc1Y1Ur?=
 =?utf-8?B?Yzl0N1FwRURnTkFreEY4TmZlZGNGMHJOUENpQy9wQ1E5a3J2K3RncW9DdE85?=
 =?utf-8?B?ZEVYdzBqL2RSeDBhc09lTUd1NTk3bE9SYXI0Sks0a1NqRjRtVk81dlcyVHpv?=
 =?utf-8?B?ZkpuRnNyNDdiem5QOU03MmNqaEZWdURUN2dEeXlrRm45cGd0Wjl2RFIvdU9K?=
 =?utf-8?B?Q2FjMDJ5S3grUy8xV3NpRHJwUDVLNmE3R3hUL05DU09STnM5RFFMVmhyWG12?=
 =?utf-8?Q?T5X+NkBp0qdACIfcvsSCgtQww?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 91e9a6aa-b6b5-4dde-c5fc-08db110c8e99
X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4553.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Feb 2023 17:29:43.4653
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: yCTZ3ptNE5uVYwXg0n24oTWrSTNaICKixA8PEHcl2LSGTbDZS038jR0FU+4Cf0y8
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR12MB4540
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Fenghua,

On 2/16/2023 7:42 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 2/2/23 13:47, Babu Moger wrote:
>> Add -o debug option to mount resctrl filesystem in debug mode. Debug
>> option adds the files for debug purposes.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   Documentation/x86/resctrl.rst          |    2 ++
>>   arch/x86/kernel/cpu/resctrl/internal.h |    1 +
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c |    7 +++++++
>>   3 files changed, 10 insertions(+)
>>
>> diff --git a/Documentation/x86/resctrl.rst 
>> b/Documentation/x86/resctrl.rst
>> index 58b76fc75cb7..2c013c5d45fd 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -46,6 +46,8 @@ mount options are:
>>   "mba_MBps":
>>       Enable the MBA Software Controller(mba_sc) to specify MBA
>>       bandwidth in MBps
>> +"debug":
>> +    Lists the debug files in resctrl interface
>>     L2 and L3 CDP are controlled separately.
>>   diff --git a/arch/x86/kernel/cpu/resctrl/internal.h 
>> b/arch/x86/kernel/cpu/resctrl/internal.h
>> index 6767c85b9699..35a9ee343fe0 100644
>> --- a/arch/x86/kernel/cpu/resctrl/internal.h
>> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
>> @@ -59,6 +59,7 @@ struct rdt_fs_context {
>>       bool                enable_cdpl2;
>>       bool                enable_cdpl3;
>>       bool                enable_mba_mbps;
>> +    bool                debug;
>>   };
>>     static inline struct rdt_fs_context *rdt_fc2context(struct 
>> fs_context *fc)
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c 
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 18d458a3cba6..9b7813aa6baf 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -2555,6 +2555,7 @@ enum rdt_param {
>>       Opt_cdp,
>>       Opt_cdpl2,
>>       Opt_mba_mbps,
>> +    Opt_debug,
>>       nr__rdt_params
>>   };
>>   @@ -2562,6 +2563,7 @@ static const struct fs_parameter_spec 
>> rdt_fs_parameters[] = {
>>       fsparam_flag("cdp",        Opt_cdp),
>>       fsparam_flag("cdpl2",        Opt_cdpl2),
>>       fsparam_flag("mba_MBps",    Opt_mba_mbps),
>> +    fsparam_flag("debug",        Opt_debug),
>>       {}
>>   };
>>   @@ -2587,6 +2589,9 @@ static int rdt_parse_param(struct fs_context 
>> *fc, struct fs_parameter *param)
>>               return -EINVAL;
>>           ctx->enable_mba_mbps = true;
>>           return 0;
>> +    case Opt_debug:
>> +        ctx->debug = true;
>> +        return 0;
>>       }
>>         return -EINVAL;
>> @@ -3525,6 +3530,8 @@ static int rdtgroup_show_options(struct 
>> seq_file *seq, struct kernfs_root *kf)
>>       if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA].r_resctrl))
>>           seq_puts(seq, ",mba_MBps");
>>   +    seq_puts(seq, ",debug");
>
> Need to add a check here otherwise ",debug" will be always shown 
> regardless "-o debug" is given or not:
> +    if (ctx->debug)
> +        seq_puts(seq, ",debug");
>
> But I don't know a good way to get ctx->debug in this function yet. I 
> think somehow ctx can be retrieved from kf but not sure.

Yes. Make sense. May be i will have to save it in rdt_hw_resource then I 
can add that check.

Thanks

Babu


>
>> +
>>       return 0;
>>   }
>>
>>
> Thanks.
>
> -Fenghua