x86/resctrl: Use seq_putc() in two functions

[PATCH] x86/resctrl: Use seq_putc() in two functions

Posted by Markus Elfring 1 year, 6 months ago

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 13 Jul 2024 10:43:14 +0200

Single characters should be put into a sequence.
Thus use the corresponding function “seq_putc”.

This issue was transformed by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 4 ++--
 arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 50fa1fe9a073..dce68f19c29d 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -452,7 +452,7 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
 	seq_printf(s, "%*s:", max_name_width, schema->name);
 	list_for_each_entry(dom, &r->ctrl_domains, hdr.list) {
 		if (sep)
-			seq_puts(s, ";");
+			seq_putc(s, ';');

 		if (is_mba_sc(r))
 			ctrl_val = dom->mbps_val[closid];
@@ -464,7 +464,7 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
 			   ctrl_val);
 		sep = true;
 	}
-	seq_puts(s, "\n");
+	seq_putc(s, '\n');
 }

 int rdtgroup_schemata_show(struct kernfs_open_file *of,
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index d7163b764c62..d4e216073e7a 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1605,7 +1605,7 @@ static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid

 	list_for_each_entry(dom, &r->mon_domains, hdr.list) {
 		if (sep)
-			seq_puts(s, ";");
+			seq_putc(s, ';');

 		memset(&mon_info, 0, sizeof(struct mon_config_info));
 		mon_info.evtid = evtid;
@@ -1614,7 +1614,7 @@ static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid
 		seq_printf(s, "%d=0x%02x", dom->hdr.id, mon_info.mon_config);
 		sep = true;
 	}
-	seq_puts(s, "\n");
+	seq_putc(s, '\n');

 	mutex_unlock(&rdtgroup_mutex);
 	cpus_read_unlock();
--
2.45.2

Re: [PATCH] x86/resctrl: Use seq_putc() in two functions

Posted by Reinette Chatre 1 year, 6 months ago

Hi Markus,

On 7/13/24 2:00 AM, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 13 Jul 2024 10:43:14 +0200
> 
> Single characters should be put into a sequence.
> Thus use the corresponding function “seq_putc”.
> 
> This issue was transformed by using the Coccinelle software.

Could you please point me to how you accomplished this? I tried
on the latest upstream as well as the latest resctrl staged
changes in tip's x86/cache but the needed coccinelle scripts do
not seem to be available. I tried by running the commands below. It appeared
to run many coccinelle scripts but there was not one related to this change.
Is this a new coccinelle script outside of the kernel or still
on its way upstream?

make C=2 CHECK=scripts/coccicheck MODE=report arch/x86/kernel/cpu/resctrl/rdtgroup.o
make C=2 CHECK=scripts/coccicheck MODE=report arch/x86/kernel/cpu/resctrl/ctrlmondata.o

> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 4 ++--
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c    | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> index 50fa1fe9a073..dce68f19c29d 100644
> --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
> @@ -452,7 +452,7 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
>   	seq_printf(s, "%*s:", max_name_width, schema->name);
>   	list_for_each_entry(dom, &r->ctrl_domains, hdr.list) {
>   		if (sep)
> -			seq_puts(s, ";");
> +			seq_putc(s, ';');
> 
>   		if (is_mba_sc(r))
>   			ctrl_val = dom->mbps_val[closid];
> @@ -464,7 +464,7 @@ static void show_doms(struct seq_file *s, struct resctrl_schema *schema, int clo
>   			   ctrl_val);
>   		sep = true;
>   	}
> -	seq_puts(s, "\n");
> +	seq_putc(s, '\n');
>   }
> 
>   int rdtgroup_schemata_show(struct kernfs_open_file *of,
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index d7163b764c62..d4e216073e7a 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1605,7 +1605,7 @@ static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid
> 
>   	list_for_each_entry(dom, &r->mon_domains, hdr.list) {
>   		if (sep)
> -			seq_puts(s, ";");
> +			seq_putc(s, ';');
> 
>   		memset(&mon_info, 0, sizeof(struct mon_config_info));
>   		mon_info.evtid = evtid;
> @@ -1614,7 +1614,7 @@ static int mbm_config_show(struct seq_file *s, struct rdt_resource *r, u32 evtid
>   		seq_printf(s, "%d=0x%02x", dom->hdr.id, mon_info.mon_config);
>   		sep = true;
>   	}
> -	seq_puts(s, "\n");
> +	seq_putc(s, '\n');
> 
>   	mutex_unlock(&rdtgroup_mutex);
>   	cpus_read_unlock();
> --
> 2.45.2
> 


Could you please highlight the benefit of this change? Looking at seq_puts() implementation, thanks
to [1], it seems to me these seq_puts() calls will result in seq_putc() anyway?

Reinette

[1] https://lore.kernel.org/all/a8589bffe4830dafcb9111e22acf06603fea7132.1713781332.git.christophe.jaillet@wanadoo.fr/

Re: x86/resctrl: Use seq_putc() in two functions

Posted by Markus Elfring 1 year, 6 months ago

>> Single characters should be put into a sequence.
>> Thus use the corresponding function “seq_putc”.
>>
>> This issue was transformed by using the Coccinelle software.
>
> Could you please point me to how you accomplished this?
…

Maybe.


> Is this a new coccinelle script outside of the kernel

I constructed small scripts for the semantic patch language according to
the presented code refactoring a while ago.


> or still on its way upstream?

I am curious if such a transformation approach can eventually be integrated
for the coccicheck tool.


> Could you please highlight the benefit of this change? Looking at seq_puts() implementation, thanks
> to [1], it seems to me these seq_puts() calls will result in seq_putc() anyway?

If the appropriate function call would be directly used, extra optimisation efforts can be avoided
by the compiler.

Regards,
Markus

Re: [PATCH] x86/resctrl: Use seq_putc() in two functions

Posted by Christophe JAILLET 1 year, 6 months ago

Le 16/07/2024 à 23:43, Reinette Chatre a écrit :
> Hi Markus,
> 
> On 7/13/24 2:00 AM, Markus Elfring wrote:
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Sat, 13 Jul 2024 10:43:14 +0200
>>
>> Single characters should be put into a sequence.
>> Thus use the corresponding function “seq_putc”.
>>

...

> Could you please highlight the benefit of this change? Looking at 
> seq_puts() implementation, thanks
> to [1], it seems to me these seq_puts() calls will result in seq_putc() 
> anyway?

Hi,

in this case, there is no benefit and the generated code should be 
exactly the same.

CJ

> 
> Reinette
> 
> [1] 
> https://lore.kernel.org/all/a8589bffe4830dafcb9111e22acf06603fea7132.1713781332.git.christophe.jaillet@wanadoo.fr/
> 
>

Re: [PATCH] x86/resctrl: Use seq_putc() in two functions

Posted by Christian Heusel 1 year, 6 months ago

On 24/07/16 02:43PM, Reinette Chatre wrote:
> Hi Markus,
> 
> On 7/13/24 2:00 AM, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sat, 13 Jul 2024 10:43:14 +0200
> > 
> > Single characters should be put into a sequence.
> > Thus use the corresponding function “seq_putc”.
> > 
> > This issue was transformed by using the Coccinelle software.
> 
> Could you please point me to how you accomplished this? I tried
> on the latest upstream as well as the latest resctrl staged
> changes in tip's x86/cache but the needed coccinelle scripts do
> not seem to be available. I tried by running the commands below. It appeared
> to run many coccinelle scripts but there was not one related to this change.
> Is this a new coccinelle script outside of the kernel or still
> on its way upstream?
> 
> make C=2 CHECK=scripts/coccicheck MODE=report arch/x86/kernel/cpu/resctrl/rdtgroup.o
> make C=2 CHECK=scripts/coccicheck MODE=report arch/x86/kernel/cpu/resctrl/ctrlmondata.o

Just to give some context to this: Markus usually uses extra
(out-of-tree) coccinelle scripts to change code to his liking, sometimes
these are good changes, sometimes they are meaningless.

See for example this:

https://lore.kernel.org/all/50978c86-cf4a-991d-54ab-50d8d977f946@inria.fr/

Some well known kernel contributors also have auto-replies (although in
a different context) specially for this contributor:

https://lore.kernel.org/all/2024071537-schematic-envoy-4272@gregkh/

Cheers,
Chris

Re: x86/resctrl: Use seq_putc() in two functions

Posted by Markus Elfring 1 year, 6 months ago

> Just to give some context to this: Markus usually uses extra
> (out-of-tree) coccinelle scripts to change code to his liking,

I occasionally dare to present special change possibilities.


> sometimes these are good changes,

These can be nice.


> sometimes they are meaningless.

Further development ideas can trigger corresponding communication challenges.

Regards,
Markus