Hi Babu, On 10/14/24 9:35 AM, Moger, Babu wrote: > On 12/31/69 18:00, Luck, Tony wrote: >> >> It is still the case that callers don't care about the return value. > > That is correct. > Are you planning to change this? I think Tony has a good point that since assignment failures do not matter it unnecessarily complicates the code to have rdtgroup_assign_cntrs() return failure. I also think the internals of rdtgroup_assign_cntrs() deserve a closer look. I assume that error handling within rdtgroup_assign_cntrs() was created with ABMC in mind. When only considering ABMC then the only reason why rdtgroup_assign_cntr_event() could fail is if the system ran out of counters and then indeed it makes no sense to attempt another call to rdtgroup_assign_cntr_event(). Now that the resctrl fs/arch split is clear the implementation does indeed expose another opportunity for failure ... if the arch callback, resctrl_arch_config_cntr() fails. It could thus be possible for the first rdtgroup_assign_cntr_event() to fail while the second succeeds. Earlier [1], Tony suggested to, within rdtgroup_assign_cntrs(), remove the local ret variable and have it return void. This sounds good to me. When doing so a function comment explaining the usage will be helpful. I also think that rdtgroup_unassign_cntrs() deserves similar scrutiny. Even more so since I do not think that the second rdtgroup_unassign_cntr_event() should be prevented from running if the first rdtgroup_unassign_cntr_event() fails. Reinette [1] https://lore.kernel.org/all/ZwldvDBjEA3TSw2k@xxxxxxxxxxxxxxxxxxxxxxxxx/
