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
