Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <b22da35c-ceeb-7116-b33d-4e596980bf80@intel.com>
Date:   Mon, 11 Sep 2023 15:58:06 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.15.0
Subject: Re: [PATCH v9 5/9] x86/resctrl: Unwind the errors inside
 rdt_enable_ctx()
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: <20230907235128.19120-1-babu.moger@amd.com>
 <20230907235128.19120-6-babu.moger@amd.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20230907235128.19120-6-babu.moger@amd.com>
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?aWI4OUphSms2dGFuNGl4WTFLMWZFczA5Qnl3RHJxQUx0eVJHMloyOEFianc2?=
 =?utf-8?B?Y0JIVDMvVHdJWEpOK0dBdXRxMy9oY1ppaG5vQk50aERBNVFRSTNiUjcwSmpq?=
 =?utf-8?B?Q1JQdDR6S3dtVDZCSitMYk9SWVZxdlp6SFNoUXh5K05aMTk2ZG8zUjdMTmdz?=
 =?utf-8?B?eXBuTjRYOGRoSUw5cDVzRmMyQ3VNaEp3cC9RdTgzcjBkM0c0ZTl5enlQV2Jz?=
 =?utf-8?B?YlZURnZHZ0dncXQyTGZuMDZHMmxXSWh0VmtRNldrRnI0WE85aTA1aG9RQjht?=
 =?utf-8?B?My84SWRLT2s3d1FxUDcvbCtCYUpmckNpUTNHZmtzTVhCVHFkR2VGUHFqek4z?=
 =?utf-8?B?aVcvSlB2VkM3MDNDN1pKQUloamJETC9ydlNMNmNEdDNyWFpCbnR3bmhteHkz?=
 =?utf-8?B?M09aaCt3bHY1a0poWWdJeE55U3VDUkl5b0tXNVo0eU83U1J0VlU0RFNXa0V3?=
 =?utf-8?B?S3V0RytWdy9FeVNKL1ZnYWc5QjZzWEU0VzVFSkNSWW5Nd2dzY3VjNjFON2M2?=
 =?utf-8?B?SWtubWhyd0Z0YXlGb3Y3ZDIvemM1WWx6QWtvUW9hSkJ3QlFnMUp1MkhYV2pF?=
 =?utf-8?B?T2htYTNralQ1bzVZckM1R2tuYVlNLzdKK2xMN3FKVkE2R3N1NWVzK3BIdXNJ?=
 =?utf-8?B?NUYxcG9hVlBLR01Ya2VuQS9Uejl1dnpaMFNETVNiclFxYUgwQjBVODFxUkpT?=
 =?utf-8?B?c2grWlF2K1phcGtrZFNVSkx6ZVBpS3ptUVE0VnI5d1U3NlNjZm9OWmhjUGVD?=
 =?utf-8?B?RWZwK3ByUEc1NGgwYTNjMEtUcDVFNmNScUJidzB1WHp2YUpqRG1oMitoREhz?=
 =?utf-8?B?UHpQMGNNMUZGc0JJL3Vqb2F0b0hoV0I0MjlRVmQ4Z1FVWllQZVVFZ2wwNE50?=
 =?utf-8?B?ZXJXLy9GZ1YrZWRzWXNCTGc2ZEl2YWpRTUZNVHk0QlIrWE1nL2ZpZFNOakRN?=
 =?utf-8?B?cEJKSlJ6MkpnSXJ2UjRnSk5FSHNRS0FxY0ZiYW5RQVRDWXNpeDQvdnhWbUpi?=
 =?utf-8?B?S2o2WTZUQmRRL24zY0N0UHZUNEhEVXpZZER4TE04TkZHSnFtWTcxTGpyMWpF?=
 =?utf-8?B?ZDVLZTAzcVFBcGhyNE5aT2ZadVJIOEt6N2ZsWm5yZDhCRk1SQXNFbFJkd2Ex?=
 =?utf-8?B?ZkhWWnNsQlRHM0FQQ0lVYkcwM28vdlhMMUNLS2p6ZWdMZVFhcXROUlZKRlM2?=
 =?utf-8?B?dGJiL1orQXdES2VCM3lHdE13M3VmZlNBeWgwTGs2dGt0QnZKc0l5cHBmUjlU?=
 =?utf-8?B?VXNIU29mQUt5UGEvcnhJc0tHOGJhNFZWRjF6M0c3Q1dFckFvcHdPZHkxTXov?=
 =?utf-8?B?UndBaUNOWHcrN2wrL21mb0drVFlEbU1tWlpDRkhES2k0WmszcXMzT1g5clk0?=
 =?utf-8?B?UW9VQmJHbWlkcmFRWjB0YThRNDhoN0hxYzRJOVRMSGZuVHVHQXBiWmhoZkpi?=
 =?utf-8?B?Z2N2b3QwWWxSZFNSTjhwWXFOYWx1dElhK0laV211T3VYWGE3YklscXpnSmhy?=
 =?utf-8?B?WjV3RW01YlNrTmtsNzIyVDMwYnJuUk1FSkw5enlsVEk5ajdyT1c2NGtxLy9S?=
 =?utf-8?B?REVKZzFNZUdkcWRsKy9HY1RUaDJkL1FmTmMxdmpCa1FRWUk5TGpyN0Q2YVN3?=
 =?utf-8?B?U3h6dVQ4TWUrRlg2ZTRQV0h3TG9NMVpiZFM4K1loaGxzbGE2NzdiemlBZGNZ?=
 =?utf-8?B?RnoxQkF5ekwvV0FkQmd4T0diMTFrM3JpWWtRelVnRjlnUktVbGFvYVprYVkx?=
 =?utf-8?B?eVNRL0ZQWVNmZXBVRmU5eHcvZHByM0JPRkM3cXJINnA4OEZhWFJJWUZjT2hD?=
 =?utf-8?B?L3YreENyaEgrS2poc3pCbkorbU9HMkhhUldBR1dzNy9Nby9PTFJiVjY2bitY?=
 =?utf-8?B?enFFT2VZQUhwRDBYNWI5NktEMmwxQm5ZaXNPeTd1ZGdOV3BoTnIyeDVPVDBh?=
 =?utf-8?B?dDNtTWwwSThSMlphYU80b0kxaWd4NTI1bCtRVE5uZnJOMm5CTjRza3dGcDBh?=
 =?utf-8?B?aWhSYlR6bWZWaFAwVHF3MnY1SFNlY2NhRVp6MFpSeUVERDhhUE55TDVQdm92?=
 =?utf-8?B?aFJZV2JoY0MxakVXb1VCNkY5SWtOL2VpL2dXODFNb2RUeUwyRnYzTlE0Z0s5?=
 =?utf-8?B?VlRJNHVLVTkzbGQvTW00MlZ1VDJOS2h2Z2hIM016ZzdQSzhPdVVMY3VwYUEz?=
 =?utf-8?B?dlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e00dc60a-5bde-45af-ec86-08dbb31a91f0
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Sep 2023 22:58:10.3678
 (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: hIkN9z+PgIqcY/KG0HHC+ueTYBnvYhwdenfcV+PFN4WzDnnAobcsrHmxZQO/Y4ZPHosOn2+FD4liCTF3E49nrY6aK7VQm0sCU747EJCXZcQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB6963
X-OriginatorOrg: intel.com
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 (snail.vger.email [0.0.0.0]); Mon, 11 Sep 2023 21:15:29 -0700 (PDT)

Hi Babu,

On 9/7/2023 4:51 PM, Babu Moger wrote:
> rdt_enable_ctx() enables the features provided during resctrl mount.
> 
> Additions to rdt_enable_ctx() are required to also modify error paths
> of rdt_enable_ctx() callers to ensure correct unwinding if errors
> are encountered after calling rdt_enable_ctx(). This is error prone.
> 
> Introduce rdt_disable_ctx() to refactor the error unwinding of
> rdt_enable_ctx() to simplify future additions. This also simplifies
> cleanup in rdt_kill_sb().
> 
> Remove cdp_disable_all() as it is not used anymore after the refactor.
> 
> Suggested-by: Reinette Chatre <reinette.chatre@intel.com>
> Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

This is one of the patches that received a change that was not
mentioned in cover letter nor in a patch specific list of changes.

Having a list of changes within each patch in this area is
helpful in reviews.

>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 55 ++++++++++++++++----------
>  1 file changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 35945b4bf196..34cb512be1de 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -2290,14 +2290,6 @@ int resctrl_arch_set_cdp_enabled(enum resctrl_res_level l, bool enable)
>  	return 0;
>  }
>  
> -static void cdp_disable_all(void)
> -{
> -	if (resctrl_arch_get_cdp_enabled(RDT_RESOURCE_L3))
> -		resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L3, false);
> -	if (resctrl_arch_get_cdp_enabled(RDT_RESOURCE_L2))
> -		resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L2, false);
> -}
> -
>  /*
>   * We don't allow rdtgroup directories to be created anywhere
>   * except the root directory. Thus when looking for the rdtgroup
> @@ -2377,19 +2369,44 @@ static int mkdir_mondata_all(struct kernfs_node *parent_kn,
>  			     struct rdtgroup *prgrp,
>  			     struct kernfs_node **mon_data_kn);
>  
> +static void rdt_disable_ctx(void)
> +{
> +	resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L3, false);
> +
> +	resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L2, false);
> +
> +	set_mba_sc(false);
> +}

Could you please remove the empty line following each call?

With the above change you can add:

Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette