Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <51277249-2351-4933-98d5-2cb74a14c7dd@intel.com>
Date:   Mon, 9 Oct 2023 10:59:27 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v11 05/10] x86/resctrl: Unwind the errors inside
 rdt_enable_ctx()
Content-Language: en-US
To:     Borislav Petkov <bp@alien8.de>, Babu Moger <babu.moger@amd.com>
CC:     <corbet@lwn.net>, <tglx@linutronix.de>, <mingo@redhat.com>,
        <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: <20231003235430.1231238-1-babu.moger@amd.com>
 <20231003235430.1231238-6-babu.moger@amd.com>
 <20231009172517.GRZSQ3fT05LGgpcW35@fat_crate.local>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231009172517.GRZSQ3fT05LGgpcW35@fat_crate.local>
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?U0xnK0JvRzA4YytOM0NTaERJakdncWw5WkhVUnJCNGpwMTNVVFNReGw3Rjdq?=
 =?utf-8?B?MU1HdTFXdHNkNFd6bUtPaTBobXB6UUxVdjN0Q2RaVWs4UDB1UVU3WFJoYlA0?=
 =?utf-8?B?TmZtam0rUXpadU9heXQ1RHpiWEpmRU1ydExUbFo3amFTS0VEdCtQMVBtRE96?=
 =?utf-8?B?Qy9pUFpmVXc0TlVpTGtyb3AwMUxVV3daaDBSUDA3VXQwRlEvZXpKSi82Nk5C?=
 =?utf-8?B?d29TeXlpMWpQR2NlMFBraWNBNzhYWEt5THNLQ2dNRm1XRFcxUkZMaXBraGtz?=
 =?utf-8?B?a2Q4eHlUdjhrN0xNY2l6emlOYmFYUXl5ZElTQmw4MjJYb1pSY05UWUtrcFdq?=
 =?utf-8?B?dzFuRWFSQWdGaGhsS0RCTUIzbXJzQWlwZVpwdGhWRTUxVzM0MFNDNVF0V1lF?=
 =?utf-8?B?QWVPWlBQSS9Id3FERGY1OTZ6VzUxSTlwdzAwUTFwSGlXVS9lcjFVVnVib1B4?=
 =?utf-8?B?czE2YWVwZG55UWRjRklOY0ZWNVdwRm02Nit6WmJYd2YySENZa2xwWVFkdjhx?=
 =?utf-8?B?bW9mK2pSaDlrT09ZVVU1dXQvRHlGYVV6SjZpdktWN1hYZW9IclVIc05qaGg2?=
 =?utf-8?B?c29CY21Wemk1ZU9UTHY1T1ZWWnQ4WHhPbGF2YlhwQ2l3eDNxaFpkZDZDRExI?=
 =?utf-8?B?U3NNbmJ6dFhObkg1Z0p2aTdVZEtGMW5yTEE2Ny81aWtDeXllcm5XZnVrcE9D?=
 =?utf-8?B?SWRyRENDWUk3SFo1cVBhZzhJK3BDTHh5WTFYbG9VdUpYN1YvZkU4Rlc5bXFD?=
 =?utf-8?B?Rm9hSGpMSHBMR1lITG1iN2RSOXRBOGdjbzB1ekFwYW90MUxsYUVyNTRzQ0lB?=
 =?utf-8?B?ZHlzeFVDWCtHdXhhY1BVUUl6SW1XNy92bHJQakR6SjRmaUpDSElwUmd0R0E1?=
 =?utf-8?B?VjZFbnhwZ1lKTFNoNHJESFhrNkFULzB3MFMzdzJ1RWNIM25PQnZxUm95QlZz?=
 =?utf-8?B?MFRFWStsbFM2SDlyWEVqNE1COXZ1UDBzR0kzZlVqbmdINktTS2RiblBtM2tJ?=
 =?utf-8?B?LzFodGlGQm45MncvRlJHMUI3T2h6UURBY0lUYUwrc1NjNUFNcnVGSjlKRVJS?=
 =?utf-8?B?RldlbmV3cHlwa0xjOGFDUWp6QkpQT1huVEdXVTBxTkdwTHFsL2o5ZmdDNVhJ?=
 =?utf-8?B?a2dlNzlNSnI5SVFzZitlb2NHeGhQbDVpWGYwUlc0dEw1dTVXeUtXS1ZFMFZu?=
 =?utf-8?B?dzNPZnJvZGhFVzZIM1orSnliOWF2aXJzbWliVVhQVGlzQVJwWlNlK3BnTXZi?=
 =?utf-8?B?N0o3eERWRlRoSEpTbVFyeFAyZjNsOUl0NTQ3cEJVWVFHdXdSNjVpd1hOa2lz?=
 =?utf-8?B?eTYzZHdEZjhjYWJlN3p1MitxZ2JHRkxBaGZjZWlNMVhVUGtHM3pCUktPbm5o?=
 =?utf-8?B?SmgzTzBoQUkzcHZ6VWtCNkxJTFQrL1VwL1dyTzVBRHg3b2lZdU9zVldXd1V1?=
 =?utf-8?B?V1BlTk9ZczRVSkIvVnlPSWhtemt6YnRrdWk5R09OZGhGakJBckRkb3JmUzlL?=
 =?utf-8?B?SGQzelJNb0VJeDZYMlQvMm82YzFwOU5KeUdTYXhjZ2prb2dxdXhOcmxGOFlp?=
 =?utf-8?B?bGJteFJyVXFPOUdUVUtUWDdhWkU4UVh3dlE4RW1YQktkaHo4dVgzQlYrRGcr?=
 =?utf-8?B?NGRLMzNYTlZqbmxxbkZGNWRWbnZTZnU1a1RJOUFZb0tJNmRYeGhESldPakRZ?=
 =?utf-8?B?QXBLdmpCRG1tTjUrNDhzWDB6K3hOUS9Mem16WGRJc1FnTVF4VGFpZ1NyWGhO?=
 =?utf-8?B?WVZlNkhlc3JnRGh0NGxWcjJtMWhmYUFQTkV2RkJqdWpMVm5UOEJ1VXBxSksz?=
 =?utf-8?B?RmlvQ2NTWUNRY3ZCWjd4dkdobHQzclY3VjFBNGJBcUYyWlhlNVV2UU4xOFBP?=
 =?utf-8?B?YWZiVHo0b05sMUs4dXNHa2xXVjFpOHRBQXRxcU5ScFZjalN2Z0ovTXp2OWx6?=
 =?utf-8?B?RzRiY2RGY2taUTBlOHpEQ0NrMmljNVNkV3IyemQ5SkFBcUJtKy93cmlCZzA3?=
 =?utf-8?B?UHAxL3NJZGxLd1hxdTUzRGlCTjQzUndKRkZKbUY4eWhkRVRkNjlNZDRzeW1W?=
 =?utf-8?B?Vk9vdjZIS0pEV01kRTR1Qk4rTjBMNXdwTkFuWlAyRDh0bDBLWmlERlhHZWZ1?=
 =?utf-8?B?MWFwZWJUYXpLZUs2MVdpOU1vN1l0WHFzN2tjSDZZNGcyY21GV0FCZGsrWnZq?=
 =?utf-8?B?eGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8e84da31-24d7-4ede-61a9-08dbc8f17cb6
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Oct 2023 17:59:31.3121
 (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: sWeJE2lvBrMAIWY2Q1lNhM+ERitt6Hrt5X4uMIKrLQjjdCPjQAdWaLuo7Wt5zbQn5FYXMsTr7gUTd0AdcLOnOh1NyNjUdj4/qtJDZRrL7vs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5090
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=2.7 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.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 (morse.vger.email [0.0.0.0]); Mon, 09 Oct 2023 10:59:55 -0700 (PDT)
X-Spam-Level: **

Hi Boris,

On 10/9/2023 10:25 AM, Borislav Petkov wrote:
> On Tue, Oct 03, 2023 at 06:54:25PM -0500, 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.
> 
> Do not talk about *what* the patch is doing in the commit message - that
> should be obvious from the diff itself. Rather, concentrate on the *why*
> it needs to be done.

I worked with Babu on this commit message and would appreciate guidance to
get this (and others) right. The second paragraph intends to explain the
current problem with rdt_enable_ctx() with the third paragraph providing an
overview of how the problem will be fixed and why (future code needs to touch
this area).

Is it the fourth paragraph (mentioning cdp_disable_all()) that is annoying? I
can see that it is redundant. Would it be more palatable if the fourth paragraph
is just dropped?

> 
> Check your whole series.
> 

Reinette