Re: [PATCH v2 2/6] kcov: Add a kcov_pause guard

[Alexander Potapenko]

On Tue, Aug 11, 2026 at 5:41 PM Karl Mehltretter <kmehltretter@xxxxxxxxx> wrote:
>
> Interrupt-return work can run after HARDIRQ_OFFSET is dropped, when
> in_task() is true. KCOV then attributes instrumented callees to the
> interrupted task.
>
> Add a KCOV_PAUSED bit next to KCOV_IN_CTXSW and mask both in
> kcov_mode_enabled(). The coverage callbacks need no new check because
> check_kcov_mode()'s exact comparison rejects modes with KCOV_PAUSED set.
>
> The context switch suppression keeps its own bit: kcov_prepare_switch()
> runs on the previous task and kcov_finish_switch() on the one switched
> in, so its lifetime is not a pause section.
>
> Provide a kcov_pause guard backed by internal helpers that operate on
> current. The guard saves the previous pause state and restores it at
> scope exit, so sections nest. When KCOV is enabled for current, remote
> softirq sections save and restore the complete mode, preserving the pause
> state.
>
> The helpers are __always_inline, and guard users must be uninstrumented:
> inlining does not remove the caller's own coverage callbacks.
>
> Assisted-by: Claude:claude-fable-5
> Signed-off-by: Karl Mehltretter <kmehltretter@xxxxxxxxx>
> ---
> Notes:
>     v2:
>      - add guard(kcov_pause)() backed by private current-only helpers
>
>  include/linux/kcov.h | 40 +++++++++++++++++++++++++++++++++++++++-
>  kernel/kcov.c        |  2 +-
>  2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/kcov.h b/include/linux/kcov.h
> index 895b761b2db15..1b4806dab62fb 100644
> --- a/include/linux/kcov.h
> +++ b/include/linux/kcov.h
> @@ -2,6 +2,8 @@
>  #ifndef _LINUX_KCOV_H
>  #define _LINUX_KCOV_H
>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
>  #include <linux/sched.h>
>  #include <uapi/linux/kcov.h>
>
> @@ -23,7 +25,8 @@ enum kcov_mode {
>         KCOV_MODE_TRACE_CMP = 3,
>  };
>
> -#define KCOV_IN_CTXSW  (1 << 30)
> +#define KCOV_IN_CTXSW  BIT(30)
> +#define KCOV_PAUSED    BIT(29)
>
>  void kcov_task_init(struct task_struct *t);
>  void kcov_task_exit(struct task_struct *t);
> @@ -38,6 +41,25 @@ do {                                         \
>         (t)->kcov_mode &= ~KCOV_IN_CTXSW;       \
>  } while (0)
>
> +/*
> + * Pause coverage for current. Callers must be uninstrumented.
> + * Pass the returned state to __kcov_resume().
> + */
> +static __always_inline unsigned int __kcov_pause(void)
> +{
> +       unsigned int paused;
> +
> +       paused = current->kcov_mode & KCOV_PAUSED;
> +       current->kcov_mode |= KCOV_PAUSED;
> +       return paused;

I think these two function can be consolidated with
kcov_prepare_switch()/kcov_finish_switch():

static __always_inline unsigned int __kcov_set_flag(struct task_struct
*t, unsigned int flag)
{
    unsigned int mode = READ_ONCE(t->kcov_mode);
    unsigned int prev_flag = mode & flag;
    if (!prev_flag && kcov_mode_enabled(mode)) {
        WRITE_ONCE(t->kcov_mode, mode | flag);
        barrier();
    }
    return prev_flag;
}

#define kcov_prepare_switch(t) __kcov_set_flag((t), KCOV_IN_CTXSW)

static __always_inline unsigned int __kcov_pause(void)
{
    return __kcov_set_flag(current, KCOV_PAUSED);
}

(note the READ_ONCE()/WRITE_ONCE() around kcov_mode, plus the compiler barrier).