Re: [PATCH v3 2/7] x86/resctrl: Remove few unnecessary rftype flags

From: Moger, Babu
Date: Thu Mar 16 2023 - 17:11:27 EST


Hi Reinette,

On 3/16/23 15:41, Reinette Chatre wrote:
> Hi Babu,
>
> On 3/16/2023 1:31 PM, Moger, Babu wrote:
>> On 3/15/23 13:33, Reinette Chatre wrote:
>>> On 3/2/2023 12:24 PM, Babu Moger wrote:
>>>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> index 15ea5b550fe9..3c86506e54c1 100644
>>>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>>> @@ -3163,7 +3163,7 @@ static int mkdir_rdt_prepare(struct kernfs_node *parent_kn,
>>>> {
>>>> struct rdtgroup *prdtgrp, *rdtgrp;
>>>> struct kernfs_node *kn;
>>>> - uint files = 0;
>>>> + uint fflags = 0;
>>>
>>> Hoe does changing the variable name from "files" to "fflags" simplify
>>> the code?
>>
>> I should have said readability of the code. Its actually fflags we are
>> passing to rdtgroup_add_files. While changing flags below, I changed the
>> variable name to fflags.
>
> You are correct in that it is the actual fflags parameter but what it
> reflects is which files will be created. I do not find that using "files"
> makes the code unreadable.

Everything helps. I changed it because I was already changing few things
in this function. That is not the only change in this function. Here is
the main change in this function.

- files = RFTYPE_BASE | BIT(RF_CTRLSHIFT + rtype);
- ret = rdtgroup_add_files(kn, files);
+ if (rtype == RDTCTRL_GROUP)
+ fflags = RFTYPE_BASE | RFTYPE_CTRL;
+ else
+ fflags = RFTYPE_BASE | RFTYPE_MON;
+
+ ret = rdtgroup_add_files(kn, fflags);

In my opinion this is more clearer. Also I can delete some of these
un-necessary definitions below.

#define RFTYPE_INFO BIT(0)
#define RFTYPE_BASE BIT(1)
-#define RF_CTRLSHIFT 4
-#define RF_MONSHIFT 5
-#define RF_TOPSHIFT 6
-#define RFTYPE_CTRL BIT(RF_CTRLSHIFT)
-#define RFTYPE_MON BIT(RF_MONSHIFT)
-#define RFTYPE_TOP BIT(RF_TOPSHIFT)
+#define RFTYPE_CTRL BIT(4)
+#define RFTYPE_MON BIT(5)
+#define RFTYPE_TOP BIT(6)

Thanks
Babu