Received-SPF: pass (google.com: domain of linux-kernel+bounces-145891-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <4b1a4cc5-e057-4944-be69-d25f28645256@wanadoo.fr>
Date: Mon, 15 Apr 2024 22:47:59 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] seq_file: Optimize seq_puts()
To: Alexander Viro <viro@zeniv.linux.org.uk>,
 Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
 linux-fsdevel@vger.kernel.org
References: <5c4f7ad7b88f5026940efa9c8be36a58755ec1b3.1704374916.git.christophe.jaillet@wanadoo.fr>
Content-Language: en-MW
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
In-Reply-To: <5c4f7ad7b88f5026940efa9c8be36a58755ec1b3.1704374916.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Le 04/01/2024 à 14:29, Christophe JAILLET a écrit :
> Most of seq_puts() usages are done with a string literal. In such cases,
> the length of the string car be computed at compile time in order to save
> a strlen() call at run-time. seq_write() can then be used instead.
> 
> This saves a few cycles.
> 
> To have an estimation of how often this optimization triggers:
>     $ git grep seq_puts.*\" | wc -l
>     3391
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Hi,

any feed-back on this small optimisation of seq_puts()?

Most of its usage would be optimized and a strlen() would be saved in 
all the corresponding cases.


$ git grep seq_puts.*\" | wc -l
3436

$ git grep seq_puts | wc -l
3644

CJ

> ---
> Checked by comparing the output of a few .s files.
> Here is one of these outputs:
> 
> $ diff -u drivers/clk/clk.s.old drivers/clk/clk.s | grep -C6 seq_w
> 
>   	call	clk_prepare_unlock	#
>   # drivers/clk/clk.c:3320: 	seq_puts(s, "}\n");
>   	movq	%r12, %rdi	# s,
> +	movl	$2, %edx	#,
>   	movq	$.LC66, %rsi	#,
> -	call	seq_puts	#
> +	call	seq_write	#
>   	call	__tsan_func_exit	#
>   # drivers/clk/clk.c:3322: }
>   	xorl	%eax, %eax	#
> @@ -34520,6 +34521,7 @@
>   	popq	%rbp	#
>   	popq	%r12	#
> --
>   # drivers/clk/clk.c:3205: 		seq_puts(s, "-----");
>   	call	__sanitizer_cov_trace_pc	#
> +	movl	$5, %edx	#,
>   	movq	$.LC72, %rsi	#,
>   	movq	%r13, %rdi	# s,
> -	call	seq_puts	#
> +	call	seq_write	#
>   	jmp	.L2134	#
>   .L2144:
>   # drivers/clk/clk.c:1793: 	return clk_core_get_accuracy_no_lock(core);
> @@ -35225,20 +35228,23 @@
>   	leaq	240(%r12), %rdi	#, tmp95
>   	call	__tsan_read8	#
> --
>   	movq	%r12, %rdi	# s,
> +	movq	$.LC77, %rsi	#,
>   # drivers/clk/clk.c:3244: 	struct hlist_head **lists = s->private;
>   	movq	240(%r12), %rbp	# s_9(D)->private, lists
>   # drivers/clk/clk.c:3246: 	seq_puts(s, "                                 enable  prepare  protect                                duty  hardware                            connection\n");
> -	call	seq_puts	#
> +	call	seq_write	#
>   # drivers/clk/clk.c:3247: 	seq_puts(s, "   clock                          count    count    count        rate   accuracy phase  cycle    enable   consumer                         id\n");
> +	movl	$142, %edx	#,
>   	movq	$.LC78, %rsi	#,
>   	movq	%r12, %rdi	# s,
> -	call	seq_puts	#
> +	call	seq_write	#
>   # drivers/clk/clk.c:3248: 	seq_puts(s, "---------------------------------------------------------------------------------------------------------------------------------------------\n");
> +	movl	$142, %edx	#,
>   	movq	$.LC79, %rsi	#,
>   	movq	%r12, %rdi	# s,
> -	call	seq_puts	#
> +	call	seq_write	#
>   # drivers/clk/clk.c:3251: 	clk_prepare_lock();
>   	call	clk_prepare_lock	#
>   .L2207:
> @@ -37511,7 +37517,7 @@
>   	subq	$16, %rsp	#,
>   # drivers/clk/clk.c:3082: {
> ---
>   fs/seq_file.c            |  4 ++--
>   include/linux/seq_file.h | 10 +++++++++-
>   2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index f5fdaf3b1572..8ef0a07033ca 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -669,7 +669,7 @@ void seq_putc(struct seq_file *m, char c)
>   }
>   EXPORT_SYMBOL(seq_putc);
>   
> -void seq_puts(struct seq_file *m, const char *s)
> +void __seq_puts(struct seq_file *m, const char *s)
>   {
>   	int len = strlen(s);
>   
> @@ -680,7 +680,7 @@ void seq_puts(struct seq_file *m, const char *s)
>   	memcpy(m->buf + m->count, s, len);
>   	m->count += len;
>   }
> -EXPORT_SYMBOL(seq_puts);
> +EXPORT_SYMBOL(__seq_puts);
>   
>   /**
>    * seq_put_decimal_ull_width - A helper routine for putting decimal numbers
> diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
> index 234bcdb1fba4..15abf45d62c5 100644
> --- a/include/linux/seq_file.h
> +++ b/include/linux/seq_file.h
> @@ -118,7 +118,15 @@ void seq_vprintf(struct seq_file *m, const char *fmt, va_list args);
>   __printf(2, 3)
>   void seq_printf(struct seq_file *m, const char *fmt, ...);
>   void seq_putc(struct seq_file *m, char c);
> -void seq_puts(struct seq_file *m, const char *s);
> +void __seq_puts(struct seq_file *m, const char *s);
> +#define seq_puts(m, s)						\
> +do {								\
> +	if (__builtin_constant_p(s))				\
> +		seq_write(m, s, __builtin_strlen(s));		\
> +	else							\
> +		__seq_puts(m, s);				\
> +} while (0)
> +
>   void seq_put_decimal_ull_width(struct seq_file *m, const char *delimiter,
>   			       unsigned long long num, unsigned int width);
>   void seq_put_decimal_ull(struct seq_file *m, const char *delimiter,