Re: [PATCH v4 2/2] x86/resctrl: Read supported bandwidth sources using CPUID command

[Moger, Babu]

Hi  Reinette,

On 1/12/2024 1:02 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 1/11/2024 1:36 PM, Babu Moger wrote:
>
> > @@ -1686,6 +1681,13 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
> >   		return -EINVAL;
> >   	}
> >   
> > +	/* mon_config cannot be more than the supported set of events */
> copy&paste error? There is no mon_config in this function.
Yea. it should be mbm_cfg_mask.  Will fix it.
> (copy&paste difficulties reminds me of [1])
>
> > +	if ((val & hw_res->mbm_cfg_mask) != val) {
> > +		rdt_last_cmd_printf("Invalid event configuration: The maximum valid "
> > +				    "bitmask is 0x%02x\n", hw_res->mbm_cfg_mask);
> checkpatch.pl should have warned about this split of text across two lines.
> Logging functions and single strings are allowed to exceed the max line length.
> If you just merge the two lines then checkpatch.pl may still warn for resctrl strings
> but that is because it does not recognize rdt_last_cmd_printf() as a logging function.
>
> You can also just shorten the string so this patch passes the checkpatch.pl check.
> For example,
> "Invalid event configuration: maximum valid mask is 0x%02x\n"
> or
> "Invalid event configuration: maximum is 0x%02x\n"
> or ?

Yes. Checkpatch reported error when I split the text.

How about this?. Checkpatch is happy.

rdt_last_cmd_printf("Invalid event configuration: max valid mask is 
0x%02x\n",
+                                   hw_res->mbm_cfg_mask);

> Reinette
>
> [1] https://lore.kernel.org/lkml/cc273d98-d73c-49bd-8799-b119966e226c@xxxxxxx/

We spent quite a bit of time on this earlier. Yea. It did not go the way 
I wanted it. Now, I needed to get back to higher priority tasks.  Will 
pick it up once, I am done with current priorities.

Thanks

Babu