Re: [PATCH 1/4] arm64: ptdump: use seq_puts() in pt_dump_seq_puts() macro

[Will Deacon <will@kernel.org>]

On Sat, Oct 18, 2025 at 07:04:16PM +0200, Josephine Pfeiffer wrote:
> The pt_dump_seq_puts() macro incorrectly uses seq_printf() instead of
> seq_puts(). This is both a performance issue and conceptually wrong,
> as the macro name suggests plain string output (puts) but the
> implementation uses formatted output (printf).

What's conceptually wrong with using printf() to print an unformatted
string? There are loads of printk() calls that do that and I think it's
fine.

>  arch/arm64/mm/ptdump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/mm/ptdump.c b/arch/arm64/mm/ptdump.c
> index ab9899ca1e5f..a35fcd62bf75 100644
> --- a/arch/arm64/mm/ptdump.c
> +++ b/arch/arm64/mm/ptdump.c
> @@ -35,7 +35,7 @@
>  #define pt_dump_seq_puts(m, fmt)	\
>  ({					\
>  	if (m)				\
> -		seq_printf(m, fmt);	\
> +		seq_puts(m, fmt);	\
>  })

Given that this macro has exactly one caller and it isn't a fast path,
wouldn't it be better to go the other way around and remove this helper
in favour of using pt_dump_seq_printf() everywhere?

i.e. something like the diff below

Will

--->8

diff --git a/arch/arm64/mm/ptdump.c b/arch/arm64/mm/ptdump.c
index ab9899ca1e5f..8a03b2c9f88b 100644
--- a/arch/arm64/mm/ptdump.c
+++ b/arch/arm64/mm/ptdump.c
@@ -32,12 +32,6 @@
                seq_printf(m, fmt, ##args);     \
 })
 
-#define pt_dump_seq_puts(m, fmt)       \
-({                                     \
-       if (m)                          \
-               seq_printf(m, fmt);     \
-})
-
 static const struct ptdump_prot_bits pte_bits[] = {
        {
                .mask   = PTE_VALID,
@@ -232,7 +226,7 @@ void note_page(struct ptdump_state *pt_st, unsigned long addr, int level,
                if (st->current_prot && pg_level[st->level].bits)
                        dump_prot(st, pg_level[st->level].bits,
                                  pg_level[st->level].num);
-               pt_dump_seq_puts(st->seq, "\n");
+               pt_dump_seq_printf(st->seq, "\n");
 
                if (addr >= st->marker[1].start_address) {
                        st->marker++;