Hi Babu, On 8/21/2023 4:30 PM, Babu Moger wrote: > static int rdt_enable_ctx(struct rdt_fs_context *ctx) > { > int ret = 0; > > - if (ctx->enable_cdpl2) > + if (ctx->enable_cdpl2) { > ret = resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L2, true); > + if (ret) > + goto out_done; > + } > > - if (!ret && ctx->enable_cdpl3) > + if (ctx->enable_cdpl3) { > ret = resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L3, true); > + if (ret) > + goto out_cdpl2; > + } > > - if (!ret && ctx->enable_mba_mbps) > + if (ctx->enable_mba_mbps) { > ret = set_mba_sc(true); > + if (ret) > + goto out_cdpl3; An error may be encountered here without CDP ever enabled or just enabled for L2 or L3. I think that the error unwinding should take care to not unwind an action that was not done. Considering the information available I think checking either ctx->enable_... or the checks used in rdt_disable_ctx() would be ok but for consistency the resctrl_arch_get_cdp_enabled() checks may be most appropriate. > + } > + > + return 0; > > +out_cdpl3: So here I think there should be a check. if (resctrl_arch_get_cdp_enabled(RDT_RESOURCE_L3)) > + resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L3, false); > +out_cdpl2: ... and here a check: if (resctrl_arch_get_cdp_enabled(RDT_RESOURCE_L2)) > + resctrl_arch_set_cdp_enabled(RDT_RESOURCE_L2, false); > +out_done: > return ret; > } > Reinette