All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Ryan Roberts <ryan.roberts@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	vladimir.murzin@arm.com, peterz@infradead.org,
	catalin.marinas@arm.com, ruanjinjie@huawei.com,
	stable@vger.kernel.org, james.morse@arm.com,
	yang@os.amperecomputing.com, cl@gentwo.org, maz@kernel.org,
	david@kernel.org, ljs@kernel.org, will@kernel.org,
	ardb@kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 14/20] arm64: percpu: Implement preemptible read/write ops
Date: Wed, 5 Aug 2026 13:08:48 +0100	[thread overview]
Message-ID: <20260805130848.6d533293@pumpkin> (raw)
In-Reply-To: <00a85d9a-8727-4b89-b33b-ce2d3358355c@arm.com>

On Wed, 5 Aug 2026 10:24:04 +0100
Ryan Roberts <ryan.roberts@arm.com> wrote:

> On 04/08/2026 18:04, Mark Rutland wrote:
> > Use the PCPU GPR infrastructure to implement preemptible this_cpu_read()
> > and this_cpu_write().
> > 
> > This change means that this_cpu_read() will always use a plain LDR, even
> > in LTO configurations where READ_ONCE() will use LDA[P]R. Using plain
> > LDR is preferable, given that the rationale for using LDA[P]R in
> > READ_ONCE() was to retain address dependencies against values written by
> > other CPUs, which isn't expected usage for this_cpu_read(). Using plain
> > LDR will enforce fewer ordering constraints.
> > 
> > Test case:
> > 
> > | void outline_this_cpu_write_u64(u64 __percpu *p, u64 v)
> > | {
> > | 	this_cpu_write(*p, v);
> > | }
> > 
> > Generated code before this patch (v7.2-rc4):
> > 
> > | <outline_this_cpu_write_u64>:
> > |        paciasp
> > |        stp     x29, x30, [sp, #-16]!
> > |        mrs     x2, sp_el0
> > |        mov     x29, sp
> > |        ldr     w3, [x2, #8]
> > |        add     w3, w3, #0x1
> > |        str     w3, [x2, #8]
> > |        mrs     x3, tpidr_el1
> > |        str     x1, [x0, x3]
> > |        ldr     x0, [x2, #8]
> > |        sub     x0, x0, #0x1
> > |        str     w0, [x2, #8]
> > |        cbz     x0, 1f
> > |        ldr     x0, [x2, #8]
> > |        cbnz    x0, 2f
> > | 1:     bl      preempt_schedule_notrace
> > | 2:     ldp     x29, x30, [sp], #16
> > |        autiasp
> > |        ret
> > 
> > Generated code after this patch:
> > 
> > | <outline_this_cpu_write_u64>:
> > |        mrs     x2, sp_el0
> > |        mov     w3, #0x7c60
> > |        strh    w3, [x2, #20]
> > |        mrs     x3, tpidr_el1
> > |        str     x1, [x0, x3]
> > |        strh    wzr, [x2, #20]
> > |        ret
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Jinjie Ruan <ruanjinjie@huawei.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Yang Shi <yang@os.amperecomputing.com>
> > ---  
> FYI I'm seeing build warnings caused by this patch (with ftrace enabled - based 
> on the warnings, I'm guessing that's the key bit), using:
> 
> aarch64-linux-gnu-gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
> 
> I haven't investigated the cause.

It'll be the earlier patch that changed the casts.
The code is doing:
		this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
			       FTRACE_PID_IGNORE);
where the constant is -1.
I suspect the warning messages are coming from the switch case that are
optimised away because the size if wrong (ftrace_ignore_pid seems to
be a pid number to ignore and is 32 bits.

That does seem a long-winded way to access per-cpu data.
To make any sense the code must be running with preemption disabled and
be caching info in per-cpu memory.
In which case it can just access it directly.
OTOH it could save the 'global' address of the per-cpu data and then access
it using the pointer so that access would be a normal one.

	David

> 
> Thanks,
> Ryan
> 
> ---8<---
> In file included from linux/arch/arm64/include/asm/spectre.h:17,                                                                                                                                                                       
>                  from linux/arch/arm64/include/asm/processor.h:47,
>                  from linux/include/linux/sched.h:13,
>                  from linux/include/linux/ratelimit.h:6,
>                  from linux/include/linux/dev_printk.h:16,
>                  from linux/include/linux/device.h:15,
>                  from linux/include/linux/node.h:18,
>                  from linux/include/linux/cpu.h:17,
>                  from linux/include/linux/stop_machine.h:5,
>                  from linux/kernel/trace/ftrace.c:17:
> linux/kernel/trace/ftrace.c: In function 'ftrace_filter_pid_sched_switch_probe':
> linux/arch/arm64/include/asm/percpu.h:295:42: warning: conversion from 'long unsigned int' to 'u8' {aka 'unsigned char'} changes value from '18446744073709551615' to '255' [-Woverflow]                                               
>   295 |         _pcp_wrap(__percpu_write_8, pcp, (unsigned long)(val))
>       |                                          ^~~~~~~~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:277:20: note: in definition of macro '_pcp_wrap'
>   277 |         op(&(pcp), __VA_ARGS__);                                        \
>       |                    ^~~~~~~~~~~
> linux/include/linux/percpu-defs.h:369:25: note: in expansion of macro 'this_cpu_write_1'
>   369 |                 case 1: stem##1(variable, __VA_ARGS__);break;           \
>       |                         ^~~~
> linux/include/linux/percpu-defs.h:500:41: note: in expansion of macro '__pcpu_size_call'
>   500 | #define this_cpu_write(pcp, val)        __pcpu_size_call(this_cpu_write_, pcp, val)
>       |                                         ^~~~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c:8628:17: note: in expansion of macro 'this_cpu_write'
>  8628 |                 this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
>       |                 ^~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:297:43: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '18446744073709551615' to '65535' [-Woverflow]
>   297 |         _pcp_wrap(__percpu_write_16, pcp, (unsigned long)(val))
>       |                                           ^~~~~~~~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:277:20: note: in definition of macro '_pcp_wrap'
>   277 |         op(&(pcp), __VA_ARGS__);                                        \
>       |                    ^~~~~~~~~~~
> linux/include/linux/percpu-defs.h:370:25: note: in expansion of macro 'this_cpu_write_2'
>   370 |                 case 2: stem##2(variable, __VA_ARGS__);break;           \
>       |                         ^~~~
> linux/include/linux/percpu-defs.h:500:41: note: in expansion of macro '__pcpu_size_call'
>   500 | #define this_cpu_write(pcp, val)        __pcpu_size_call(this_cpu_write_, pcp, val)
>       |                                         ^~~~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c:8628:17: note: in expansion of macro 'this_cpu_write'
>  8628 |                 this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
>       |                 ^~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:299:43: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551615' to '4294967295' [-Woverflow]
>   299 |         _pcp_wrap(__percpu_write_32, pcp, (unsigned long)(val))
>       |                                           ^~~~~~~~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:277:20: note: in definition of macro '_pcp_wrap'
>   277 |         op(&(pcp), __VA_ARGS__);                                        \
>       |                    ^~~~~~~~~~~
> linux/include/linux/percpu-defs.h:371:25: note: in expansion of macro 'this_cpu_write_4'
>   371 |                 case 4: stem##4(variable, __VA_ARGS__);break;           \
>       |                         ^~~~
> linux/include/linux/percpu-defs.h:500:41: note: in expansion of macro '__pcpu_size_call'
>   500 | #define this_cpu_write(pcp, val)        __pcpu_size_call(this_cpu_write_, pcp, val)
>       |                                         ^~~~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c:8628:17: note: in expansion of macro 'this_cpu_write'
>  8628 |                 this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
>       |                 ^~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c: In function 'ignore_task_cpu':
> linux/arch/arm64/include/asm/percpu.h:295:42: warning: conversion from 'long unsigned int' to 'u8' {aka 'unsigned char'} changes value from '18446744073709551615' to '255' [-Woverflow]
>   295 |         _pcp_wrap(__percpu_write_8, pcp, (unsigned long)(val))
>       |                                          ^~~~~~~~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:277:20: note: in definition of macro '_pcp_wrap'
>   277 |         op(&(pcp), __VA_ARGS__);                                        \
>       |                    ^~~~~~~~~~~
> linux/include/linux/percpu-defs.h:369:25: note: in expansion of macro 'this_cpu_write_1'
>   369 |                 case 1: stem##1(variable, __VA_ARGS__);break;           \
>       |                         ^~~~
> linux/include/linux/percpu-defs.h:500:41: note: in expansion of macro '__pcpu_size_call'
>   500 | #define this_cpu_write(pcp, val)        __pcpu_size_call(this_cpu_write_, pcp, val)
>       |                                         ^~~~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c:8898:17: note: in expansion of macro 'this_cpu_write'
>  8898 |                 this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
>       |                 ^~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:297:43: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '18446744073709551615' to '65535' [-Woverflow]
>   297 |         _pcp_wrap(__percpu_write_16, pcp, (unsigned long)(val))
>       |                                           ^~~~~~~~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:277:20: note: in definition of macro '_pcp_wrap'
>   277 |         op(&(pcp), __VA_ARGS__);                                        \
>       |                    ^~~~~~~~~~~
> linux/include/linux/percpu-defs.h:370:25: note: in expansion of macro 'this_cpu_write_2'
>   370 |                 case 2: stem##2(variable, __VA_ARGS__);break;           \
>       |                         ^~~~
> linux/include/linux/percpu-defs.h:500:41: note: in expansion of macro '__pcpu_size_call'
>   500 | #define this_cpu_write(pcp, val)        __pcpu_size_call(this_cpu_write_, pcp, val)
>       |                                         ^~~~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c:8898:17: note: in expansion of macro 'this_cpu_write'
>  8898 |                 this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
>       |                 ^~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:299:43: warning: conversion from 'long unsigned int' to 'u32' {aka 'unsigned int'} changes value from '18446744073709551615' to '4294967295' [-Woverflow]
>   299 |         _pcp_wrap(__percpu_write_32, pcp, (unsigned long)(val))
>       |                                           ^~~~~~~~~~~~~~~~~~~~
> linux/arch/arm64/include/asm/percpu.h:277:20: note: in definition of macro '_pcp_wrap'
>   277 |         op(&(pcp), __VA_ARGS__);                                        \
>       |                    ^~~~~~~~~~~
> linux/include/linux/percpu-defs.h:371:25: note: in expansion of macro 'this_cpu_write_4'
>   371 |                 case 4: stem##4(variable, __VA_ARGS__);break;           \
>       |                         ^~~~
> linux/include/linux/percpu-defs.h:500:41: note: in expansion of macro '__pcpu_size_call'
>   500 | #define this_cpu_write(pcp, val)        __pcpu_size_call(this_cpu_write_, pcp, val)
>       |                                         ^~~~~~~~~~~~~~~~
> linux/kernel/trace/ftrace.c:8898:17: note: in expansion of macro 'this_cpu_write'
>  8898 |                 this_cpu_write(tr->array_buffer.data->ftrace_ignore_pid,
>       |                 ^~~~~~~~~~~~~~
> ---8<---
> 
> 



  reply	other threads:[~2026-08-05 12:09 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 17:04 [PATCH v2 00/20] arm64: Preemptible this_cpu_*() operations Mark Rutland
2026-08-04 17:04 ` [PATCH v2 01/20] arm64: percpu: Fix this_cpu_write() casting Mark Rutland
2026-08-05  8:37   ` David Laight
2026-08-07  4:00   ` Jinjie Ruan
2026-08-04 17:04 ` [PATCH v2 02/20] arm64: percpu: Fix this_cpu_and() mask generation Mark Rutland
2026-08-05  9:14   ` David Laight
2026-08-05 13:02     ` Mark Rutland
2026-08-06  8:28       ` David Laight
2026-08-06 10:23         ` Mark Rutland
2026-08-07  9:52   ` Jinjie Ruan
2026-08-04 17:04 ` [PATCH v2 03/20] arm64: cmpxchg: LL/SC: Avoid redundant extension Mark Rutland
2026-08-04 17:04 ` [PATCH v2 04/20] arm64: cmpxchg128: LSE: Remove redundant operands Mark Rutland
2026-08-04 17:04 ` [PATCH v2 05/20] arm64: preempt: Simplify and optimize __preempt_count_dec_and_test() Mark Rutland
2026-08-04 17:04 ` [PATCH v2 06/20] arm64: preempt: Treat should_resched() as unlikely Mark Rutland
2026-08-04 17:04 ` [PATCH v2 07/20] arm64: ptrace: Always inline pt_regs_[read,write}_reg() Mark Rutland
2026-08-04 17:04 ` [PATCH v2 08/20] arm64: percpu: Factor out percpu offset asm Mark Rutland
2026-08-04 17:04 ` [PATCH v2 09/20] arm64: gpr-num: Add wxN aliases for wN registers Mark Rutland
2026-08-04 17:04 ` [PATCH v2 10/20] arm64: gpr-num: add __GPR_NUM() helper Mark Rutland
2026-08-04 17:04 ` [PATCH v2 11/20] arm64: entry: sdei: Restore all clobberable GPRs Mark Rutland
2026-08-07 11:35   ` Mark Rutland
2026-08-04 17:04 ` [PATCH v2 12/20] arm64: entry: sdei: Make 'tsk' available Mark Rutland
2026-08-04 17:04 ` [PATCH v2 13/20] arm64: percpu: Add infrastructure for preemptible this_cpu_*() ops Mark Rutland
2026-08-04 22:45   ` Pedro Falcato
2026-08-05 10:27     ` David Laight
2026-08-05 12:50       ` Pedro Falcato
2026-08-05  6:45   ` David Hildenbrand (Arm)
2026-08-05  6:47     ` David Hildenbrand (Arm)
2026-08-06 11:21       ` Mark Rutland
2026-08-06 11:32         ` David Hildenbrand (Arm)
2026-08-06 12:02           ` Mark Rutland
2026-08-06 13:25           ` David Laight
2026-08-06 13:30             ` David Hildenbrand (Arm)
2026-08-04 17:04 ` [PATCH v2 14/20] arm64: percpu: Implement preemptible read/write ops Mark Rutland
2026-08-05  9:24   ` Ryan Roberts
2026-08-05 12:08     ` David Laight [this message]
2026-08-05 13:34     ` Mark Rutland
2026-08-04 17:04 ` [PATCH v2 15/20] arm64: percpu: Implement preemptible void RMW ops Mark Rutland
2026-08-04 17:04 ` [PATCH v2 16/20] arm64: percpu: Implement preemptible return " Mark Rutland
2026-08-04 17:05 ` [PATCH v2 17/20] arm64: percpu: Implement preemptible XCHG ops Mark Rutland
2026-08-04 17:05 ` [PATCH v2 18/20] arm64: percpu: Implement preemptible CMPXCHG ops Mark Rutland
2026-08-04 17:05 ` [PATCH v2 19/20] arm64: percpu: Implement preemptible CMPXCHG128 ops Mark Rutland
2026-08-04 17:05 ` [PATCH v2 20/20] arm64: percpu: Remove _pcp_protect*() wrappers Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260805130848.6d533293@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=ardb@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=cl@gentwo.org \
    --cc=david@kernel.org \
    --cc=james.morse@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=ljs@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=peterz@infradead.org \
    --cc=ruanjinjie@huawei.com \
    --cc=ryan.roberts@arm.com \
    --cc=stable@vger.kernel.org \
    --cc=vladimir.murzin@arm.com \
    --cc=will@kernel.org \
    --cc=yang@os.amperecomputing.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.