[PATCH 0/4] s390: Consistent usage of text_mutex

[PATCH 0/4] s390: Consistent usage of text_mutex

[Heiko Carstens]

Other architectures serialize kernel text patching with text_mutex. s390
does not: __s390_kernel_write() relies on a private spinlock, and callers
partially depend on implicit exclusion (e.g. no kprobe can be set on a
jump label).

Make s390 consistent with other architectures:

- Patches 1-3 add text_mutex to the jump_label, ftrace and bpf text
  patching paths

- Patch 4 removes the redundant spinlock from __s390_kernel_write().
  CPU serialization, previously done implicitly, is now explicit via
  sync_core()

Thanks,
Heiko

Heiko Carstens (4):
  s390/jump_label: Serialize text patching with text_mutex
  s390/ftrace: Serialize text patching with text_mutex
  s390/bpf: Serialize text patching with text_mutex
  s390/maccess: Remove spinlock from __s390_kernel_write()

 arch/s390/kernel/ftrace.c     |  9 +++++++++
 arch/s390/kernel/jump_label.c |  3 +++
 arch/s390/mm/maccess.c        |  9 ++++-----
 arch/s390/net/bpf_jit_comp.c  | 21 +++++++++++++++++----
 4 files changed, 33 insertions(+), 9 deletions(-)

-- 
2.53.0

[PATCH 1/4] s390/jump_label: Serialize text patching with text_mutex

[Heiko Carstens]

Acquire text_mutex around the verify-then-patch sequence in
jump_label_transform(). This way s390 uses text_mutex like other
architectures and does not need to rely on other implicit semantics
(e.g. no kprobe can be set on a jump label).

This is a prereq for removing the spinlock in __s390_kernel_write().

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/jump_label.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index e808bb8bc0da..682069673f48 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -5,6 +5,7 @@
  * Copyright IBM Corp. 2011
  * Author(s): Jan Glauber <jang@linux.vnet.ibm.com>
  */
+#include <linux/memory.h>
 #include <linux/uaccess.h>
 #include <linux/jump_label.h>
 #include <linux/module.h>
@@ -57,9 +58,11 @@ static void jump_label_transform(struct jump_entry *entry,
 		jump_label_make_branch(entry, &old);
 		jump_label_make_nop(entry, &new);
 	}
+	mutex_lock(&text_mutex);
 	if (memcmp(code, &old, sizeof(old)))
 		jump_label_bug(entry, &old, &new);
 	s390_kernel_write(code, &new, sizeof(new));
+	mutex_unlock(&text_mutex);
 }
 
 void arch_jump_label_transform(struct jump_entry *entry,
-- 
2.53.0

[PATCH 2/4] s390/ftrace: Serialize text patching with text_mutex

[Heiko Carstens]

Add ftrace_arch_code_modify_prepare() and extend
ftrace_arch_code_modify_post_process() to acquire and release text_mutex
around the ftrace code modification pass. This way s390 uses text_mutex
like e.g. x86 and riscv.

This is a prereq for removing the spinlock in __s390_kernel_write().

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/ftrace.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index e94bb98f5231..772dbdcc51f2 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/hardirq.h>
+#include <linux/memory.h>
 #include <linux/uaccess.h>
 #include <linux/ftrace.h>
 #include <linux/kernel.h>
@@ -247,13 +248,21 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 	return 0;
 }
 
+void ftrace_arch_code_modify_prepare(void)
+	__acquires(&text_mutex)
+{
+	mutex_lock(&text_mutex);
+}
+
 void arch_ftrace_update_code(int command)
 {
 	ftrace_modify_all_code(command);
 }
 
 void ftrace_arch_code_modify_post_process(void)
+	__releases(&text_mutex)
 {
+	mutex_unlock(&text_mutex);
 	/*
 	 * Flush any pre-fetched instructions on all
 	 * CPUs to make the new code visible.
-- 
2.53.0

[PATCH 3/4] s390/bpf: Serialize text patching with text_mutex

[Heiko Carstens]

Acquire text_mutex in bpf_arch_text_poke(). This way s390 uses text_mutex
like most other architectures.

This is a prereq for removing the spinlock in __s390_kernel_write().

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/net/bpf_jit_comp.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 51db92c3329d..b8ff1ca2f598 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -22,6 +22,7 @@
 #include <linux/init.h>
 #include <linux/bpf.h>
 #include <linux/cfi.h>
+#include <linux/memory.h>
 #include <linux/mm.h>
 #include <linux/kernel.h>
 #include <asm/cacheflush.h>
@@ -2446,9 +2447,9 @@ bool bpf_jit_supports_far_kfunc_call(void)
 	return true;
 }
 
-int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
-		       enum bpf_text_poke_type new_t, void *old_addr,
-		       void *new_addr)
+static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+				enum bpf_text_poke_type new_t, void *old_addr,
+				void *new_addr)
 {
 	struct bpf_plt expected_plt, current_plt, new_plt, *plt;
 	struct {
@@ -2458,6 +2459,8 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
 	char *ret;
 	int err;
 
+	guard(mutex)(&text_mutex);
+
 	/* Verify the branch to be patched. */
 	err = copy_from_kernel_nofault(&insn, ip, sizeof(insn));
 	if (err < 0)
@@ -2491,10 +2494,20 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
 	/* Adjust the mask of the branch. */
 	insn.opc = 0xc004 | (new_addr ? 0xf0 : 0);
 	s390_kernel_write((char *)ip + 1, (char *)&insn.opc + 1, 1);
+	return 0;
+}
 
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
+		       enum bpf_text_poke_type new_t, void *old_addr,
+		       void *new_addr)
+{
+	int err;
+
+	err = __bpf_arch_text_poke(ip, old_t, new_t, old_addr, new_addr);
+	if (err)
+		return err;
 	/* Make the new code visible to the other CPUs. */
 	text_poke_sync_lock();
-
 	return 0;
 }
 
-- 
2.53.0

[PATCH 4/4] s390/maccess: Remove spinlock from __s390_kernel_write()

[Heiko Carstens]

The spinlock in __s390_kernel_write() was introduced to serialize the
read-modify-write sequence against concurrent callers. Now that all
relevant runtime callers of s390_kernel_write() hold the text_mutex, the
spinlock is not required anymore.

Replace it with preempt_disable() / preempt_enable() and sync_core().
Previously, CPU serialization was implicitly done by other
instructions. Make this explicit with sync_core() instead of
relying on that.

Preemption is disabled so that sync_core() issues the CPU serialization
instruction on the same CPU that executed the stores. This completes all
previous stores and makes them visible to other CPUs.

Other CPUs need to perform CPU serialization, e.g. with text_poke_sync(),
to discard already prefetched instructions.

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/mm/maccess.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/s390/mm/maccess.c b/arch/s390/mm/maccess.c
index f39968dd8063..cda3f81dfbc2 100644
--- a/arch/s390/mm/maccess.c
+++ b/arch/s390/mm/maccess.c
@@ -6,6 +6,7 @@
  *
  */
 
+#include <linux/text-patching.h>
 #include <linux/uaccess.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
@@ -60,22 +61,20 @@ static notrace long s390_kernel_write_odd(void *dst, const void *src, size_t siz
  * bytes from destination at an eight byte boundary, modifies the bytes
  * requested and writes the result back in a loop.
  */
-static DEFINE_SPINLOCK(s390_kernel_write_lock);
-
 notrace void *__s390_kernel_write(void *dst, const void *src, size_t size)
 {
 	void *tmp = dst;
-	unsigned long flags;
 	long copied;
 
-	spin_lock_irqsave(&s390_kernel_write_lock, flags);
+	preempt_disable();
 	while (size) {
 		copied = s390_kernel_write_odd(tmp, src, size);
 		tmp += copied;
 		src += copied;
 		size -= copied;
 	}
-	spin_unlock_irqrestore(&s390_kernel_write_lock, flags);
+	sync_core();
+	preempt_enable();
 
 	return dst;
 }
-- 
2.53.0

Re: [PATCH 4/4] s390/maccess: Remove spinlock from __s390_kernel_write()

[Ilya Leoshkevich]

On 10/26/26 13:03, Heiko Carstens wrote:
> The spinlock in __s390_kernel_write() was introduced to serialize the
> read-modify-write sequence against concurrent callers. Now that all
> relevant runtime callers of s390_kernel_write() hold the text_mutex, the
> spinlock is not required anymore.

Do we want to enforce this with __must_hold?

> Replace it with preempt_disable() / preempt_enable() and sync_core().
> Previously, CPU serialization was implicitly done by other
> instructions. Make this explicit with sync_core() instead of
> relying on that.
> 
> Preemption is disabled so that sync_core() issues the CPU serialization
> instruction on the same CPU that executed the stores. This completes all
> previous stores and makes them visible to other CPUs.
> 
> Other CPUs need to perform CPU serialization, e.g. with text_poke_sync(),
> to discard already prefetched instructions.
> 
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> ---
>   arch/s390/mm/maccess.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/s390/mm/maccess.c b/arch/s390/mm/maccess.c
> index f39968dd8063..cda3f81dfbc2 100644
> --- a/arch/s390/mm/maccess.c
> +++ b/arch/s390/mm/maccess.c
> @@ -6,6 +6,7 @@
>    *
>    */
>   
> +#include <linux/text-patching.h>
>   #include <linux/uaccess.h>
>   #include <linux/kernel.h>
>   #include <linux/types.h>
> @@ -60,22 +61,20 @@ static notrace long s390_kernel_write_odd(void *dst, const void *src, size_t siz
>    * bytes from destination at an eight byte boundary, modifies the bytes
>    * requested and writes the result back in a loop.
>    */
> -static DEFINE_SPINLOCK(s390_kernel_write_lock);
> -
>   notrace void *__s390_kernel_write(void *dst, const void *src, size_t size)
>   {
>   	void *tmp = dst;
> -	unsigned long flags;
>   	long copied;
>   
> -	spin_lock_irqsave(&s390_kernel_write_lock, flags);
> +	preempt_disable();
>   	while (size) {
>   		copied = s390_kernel_write_odd(tmp, src, size);
>   		tmp += copied;
>   		src += copied;
>   		size -= copied;
>   	}
> -	spin_unlock_irqrestore(&s390_kernel_write_lock, flags);
> +	sync_core();
> +	preempt_enable();
>   
>   	return dst;
>   }

I've been looking at sync_core() and started thinking about the safety
of this interface w.r.t. CPUs concurrently executing the code being
patched, since locking protects only against concurrent readers and
writers, but not executors.

If there is only one active CPU, for example, during ftrace
initialization, then everything is fine.

If there are other active CPUs and we make modifications within a single
aligned u64, then sync_core() is required, because even though
text_poke_sync() nominally takes care of this, it may sync our core
last. However, I wonder, what would it take to make it always sync the
current core first?

Finally, and most importantly, if there are other active CPUs and we
patch multiple u64s, then we are broken, since the other CPUs may see
partial instructions. I'm not sure if it's worth bothering with, but
maybe consider adding a WARN_ONCE() to guard against this?

Re: [PATCH 4/4] s390/maccess: Remove spinlock from __s390_kernel_write()

[Heiko Carstens]

On Mon, Oct 26, 2026 at 02:04:02PM +0100, Ilya Leoshkevich wrote:
> On 10/26/26 13:03, Heiko Carstens wrote:
> > The spinlock in __s390_kernel_write() was introduced to serialize the
> > read-modify-write sequence against concurrent callers. Now that all
> > relevant runtime callers of s390_kernel_write() hold the text_mutex, the
> > spinlock is not required anymore.
> 
> Do we want to enforce this with __must_hold?

Initially I thought that wouldn't be necessary, since "I know what I'm doing"
:) But Sashiko already complained that this might not be safe. So I'm going to
had the mutex to all callers of s390_kernel_write(), and add __must_hold too,
like you propose.

> > -	spin_unlock_irqrestore(&s390_kernel_write_lock, flags);
> > +	sync_core();
> > +	preempt_enable();
> >   	return dst;
> >   }
> 
> I've been looking at sync_core() and started thinking about the safety
> of this interface w.r.t. CPUs concurrently executing the code being
> patched, since locking protects only against concurrent readers and
> writers, but not executors.
> 
> If there is only one active CPU, for example, during ftrace
> initialization, then everything is fine.
> 
> If there are other active CPUs and we make modifications within a single
> aligned u64, then sync_core() is required, because even though
> text_poke_sync() nominally takes care of this, it may sync our core
> last. However, I wonder, what would it take to make it always sync the
> current core first?

Maybe I don't understand your point, but I added the preempt_disable() /
preempt_enable() pair exactly for this reason: make sure that the writer syncs
first - only calling text_poke_sync() afterward does not guarantee anything
due to preemption (well, that's not true since there are many implicit
synchronizations before the next CPU runs, but the point of this was to make
everything explicit, and not "hope" that things work).

> Finally, and most importantly, if there are other active CPUs and we
> patch multiple u64s, then we are broken, since the other CPUs may see
> partial instructions. I'm not sure if it's worth bothering with, but
> maybe consider adding a WARN_ONCE() to guard against this?

Initial alternatives patching might indeed patch multiple instructions in a
row, and that's a valid use case, since the code is guaranteed to be not
executed anywhere...
So a WARN_ONCE() would at least find one false positive :)

I don't think we should go this route (when is it valid to patch more than one
instruction, when not). As of now no code is doing that.