[SRU][jammy][PULL] Kernel livepatch support for s390x

Stefan Bader stefan.bader at canonical.com
Wed Mar 8 08:59:04 UTC 2023


On 07.03.23 23:48, John Cabaj wrote:
> On 3/6/23 4:32 AM, Stefan Bader wrote:
>> On 04.03.23 00:06, John Cabaj wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1639924 (Kernel livepatch support for s390x)
>>>
>>> [Impact]
>>> * Enables Livepatch support for Jammy s390x generic kernel
>>>
>>> [Fix]
>>> b56340e565fd ("UBUNTU: [Config] s390x: Removing UBSAN from configuration")
>>> f0bd174678a9 ("UBUNTU: [Config] Enable EXPOLINE_EXTERN on s390x")
>>> 145dd7c7bfc6 ("s390/nospec: build expoline.o for modules_prepare target")
>>> ad24b243a0be ("bug: Use normal relative pointers in 'struct bug_entry'")
>>> 9c018ab00f18 ("s390/nospec: align and size extern thunks")
>>> 110252959a61 ("s390/nospec: add an option to use thunk-extern")
>>> 9183918aabab ("s390/nospec: generate single register thunks if possible")
>>> f08372d421dd ("s390: remove unused expoline to BC instructions")
>>> d1de041f86ad ("s390/entry: remove unused expoline thunk")
>>> 376e067eb888 ("sched: Improve wake_up_all_idle_cpus() take #2")
>>> 16fa7242e5b9 ("sched,livepatch: Use wake_up_if_idle()")
>>> 3a133e568eea ("sched: Simplify wake_up_*idle*()")
>>> 360b90e726cd ("sched,livepatch: Use task_call_func()")
>>> 3c01d492a326 ("sched,rcu: Rework try_invoke_on_locked_down_task()")
>>> 2a989b8ace35 ("sched: Improve try_invoke_on_locked_down_task()")
>>>
>>> [Test Case]
>>> * Boot test
>>> * Tested a Livepatch (patch to /proc/meminfo module)
>>>
>>> [Other Info]
>>> * Requires patch to GCC-11.3, removing gcc-ibmz-plt-revert.diff from debian/patches and debian/rules.patch (LP#2002429)
>>> * UBSAN (Undefined Behavior Santizer) configuration was removed from s390x as kpatch-build could not find symbols
>>> * kpatch update being considered for UBSAN issue (https://github.com/dynup/kpatch/issues/1328)
>>>
>>> [Where things could go wrong]
>>> * Functionality already exists upstream, kernel was boot and Livepatch tested - should have no regressions
>>> * s390x UBSAN functionality no longer included, may not catch previously caught undefined behavior
>>>
>>> -----
>>>
>>> The following changes since commit ed81d4519af756cfde8c3c02b495c6006b6ee0e9:
>>>
>>>     UBUNTU: Start new release (2023-03-02 21:29:10 -0600)
>>>
>>> are available in the git repository at:
>>>
>>>     git+ssh://user@kathleen/home/john-cabaj/for-review/jammy-s390x-livepatch s390x-livepatch
>>>
>>> for you to fetch changes up to b56340e565fdae9b6fc53b573e01194435a65e3e:
>>>
>>>     UBUNTU: [Config] s390x: Removing UBSAN from configuration (2023-03-02 21:36:26 -0600)
>>>
>>> ----------------------------------------------------------------
>>> Dimitri John Ledkov (1):
>>>         UBUNTU: [Config] Enable EXPOLINE_EXTERN on s390x
>>>
>>> John Cabaj (1):
>>>         UBUNTU: [Config] s390x: Removing UBSAN from configuration
>>>
>>> Josh Poimboeuf (1):
>>>         bug: Use normal relative pointers in 'struct bug_entry'
>>>
>>> Peter Zijlstra (6):
>>>         sched: Improve try_invoke_on_locked_down_task()
>>>         sched,rcu: Rework try_invoke_on_locked_down_task()
>>>         sched,livepatch: Use task_call_func()
>>>         sched: Simplify wake_up_*idle*()
>>>         sched,livepatch: Use wake_up_if_idle()
>>>         sched: Improve wake_up_all_idle_cpus() take #2
>>
>> I find this whole block of generic scheduler changes concerning. I have not looked into the actual changes but the delta but changing something related to RCU sounds potentially dangerous for any architecture. Even those not doing livepatch. Some more explanation about why this is needed and roughly what the purpose of those changes is would be much better. So not everyone checking has to go through the same research.
> Working backwards from "sched: Improve wake_up_all_idle_cpus() take #2":
> 
> * "sched: Improve wake_up_all_idle_cpus() take #2": Improvements from "sched: Simplify wake_up_*idle*()"
> * "sched,livepatch: Use wake_up_if_idle()": Uses wake_up_if_idle(cpu) function modified in "sched: Simplify wake_up_*idle*()" to have idle CPUs update patch state
> * "sched: Simplify wake_up_*idle*()": Doesn't disable kernel preemption to wake up all idle CPUs, rather locks CPU read state
> * "sched,livepatch: Use task_call_func()": Uses task_call_func() introduced in "sched,rcu: Rework try_invoke_on_locked_down_task()"
> * "sched,rcu: Rework try_invoke_on_locked_down_task()": Provides integer return value instead of "bool" to better handle specific errors
> * "sched: Improve try_invoke_on_locked_down_task()": Avoids rq->lock when task is blocked
> 
> So the patches indicating "livepatch" largely depend on either enhancements from previous patches for waking idle CPUs, or updated function signatures from the task scheduler
> 
> All patches are already in Kinetic 5.19, and Jammy HWE-5.19 as well

Those are part of 5.19 then with all the other changes between 5.15 and 
5.19. The group of changes might be sufficient for livepatch. But one 
cannot completely rule out that having those will require additional 
changes in other places or cause problems even if only in corner cases.

I guess Ideally we would have an artificial workload test which puts 
some stress on the scheduler. Maybe even checking timings. That we could 
run across arches to gain more confidence there is no regression hidden 
within.

Call it old grumpy or experience, but changing things in the scheduler 
and even worse having those changes include RCU make a lot of alarm 
bells ring in my head.

> 
>>>
>>> Vasily Gorbik (6):
>>>         s390/entry: remove unused expoline thunk
>>>         s390: remove unused expoline to BC instructions
>>>         s390/nospec: generate single register thunks if possible
>>>         s390/nospec: add an option to use thunk-extern
>>>         s390/nospec: align and size extern thunks
>>>         s390/nospec: build expoline.o for modules_prepare target
>>>
>>>    arch/arm64/include/asm/asm-bug.h          |  4 ++--
>>>    arch/powerpc/include/asm/bug.h            | 14 ++++++++------
>>>    arch/riscv/include/asm/bug.h              |  4 ++--
>>>    arch/s390/Kconfig                         | 15 +++++++++++++++
>>>    arch/s390/Makefile                        | 20 +++++++++++++++-----
>>>    arch/s390/include/asm/bug.h               |  5 +++--
>>>    arch/s390/include/asm/nospec-insn.h       | 91 +++++++++++++++++++++++++++++++++++--------------------------------------------------------
>>>    arch/s390/kernel/entry.S                  |  1 -
>>>    arch/s390/kernel/nospec-branch.c          | 25 +++++--------------------
>>>    arch/s390/lib/Makefile                    |  2 ++
>>>    arch/s390/lib/expoline/Makefile           |  3 +++
>>>    arch/s390/lib/expoline/expoline.S         | 12 ++++++++++++
>>>    arch/s390/tools/gcc-thunk-extern.sh       | 24 ++++++++++++++++++++++++
>>>    arch/x86/include/asm/bug.h                |  2 +-
>>>    debian.master/config/annotations          |  5 +++--
>>>    debian.master/config/config.common.ubuntu |  2 +-
>>>    debian/rules.d/2-binary-arch.mk           |  4 ++++
>>>    include/linux/sched/idle.h                |  4 ++++
>>>    include/linux/wait.h                      |  3 ++-
>>>    kernel/livepatch/transition.c             | 95 ++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------
>>>    kernel/rcu/tasks.h                        | 12 ++++++------
>>>    kernel/rcu/tree_stall.h                   |  8 ++++----
>>>    kernel/sched/core.c                       | 83 +++++++++++++++++++++++++++++++++++++++++++++++------------------------------------
>>>    kernel/smp.c                              | 12 +++++-------
>>>    lib/bug.c                                 | 15 +++++++--------
>>>    scripts/mod/modpost.c                     |  5 +++++
>>>    26 files changed, 263 insertions(+), 207 deletions(-)
>>>    create mode 100644 arch/s390/lib/expoline/Makefile
>>>    create mode 100644 arch/s390/lib/expoline/expoline.S
>>>    create mode 100755 arch/s390/tools/gcc-thunk-extern.sh
>>>
>>
>> - Stefan
>>
> 

- Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 44613 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230308/69cf22ca/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230308/69cf22ca/attachment-0001.sig>


More information about the kernel-team mailing list