Re: [PATCH v3] io_uring: annotate remote tasks for kcoverage

On Tue, May 26, 2026 at 6:49 PM Robert Femmer <[email protected]> wrote:
> Fuzzers use coverage information to guide generation of test cases
> towards new or interesting code paths. Syzkaller, specifically, makes
> use kcoverage (CONFIG_KCOV). Coverage information is not collected for
> kernel tasks unless annotated by kcov_remote_start and kcov_remote_stop.
> This patch annotates io-uring's work queue and sqpoll tasks.

I think this is a useful change overall.

@maintainers: For context, this should have no impact on normal builds
- "struct kcov_common_handle_id" is zero-sized in normal builds, and
all the helpers used here are empty inline functions.

> Depends-on: 20260430-kcov-refactor-common-handle-v1-1-23a0c7a0ba38@google.com
> Signed-off-by: Robert Femmer <[email protected]>
> ---
>  include/linux/io_uring_types.h | 2 ++
>  io_uring/io-wq.c               | 4 ++++
>  io_uring/io_uring.c            | 1 +
>  io_uring/io_uring.h            | 2 ++
>  io_uring/sqpoll.c              | 4 ++++
>  5 files changed, 13 insertions(+)
>
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index 244392026c6d..b6590b2b350c 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -504,6 +504,8 @@ struct io_ring_ctx {
>         struct io_mapped_region         ring_region;
>         /* used for optimised request parameter and wait argument passing  */
>         struct io_mapped_region         param_region;
> +
> +       struct kcov_common_handle_id    kcov_handle;
>  };
>
>  /*
> diff --git a/io_uring/io-wq.c b/io_uring/io-wq.c
> index 8cc7b47d3089..9ade4c4f4983 100644
> --- a/io_uring/io-wq.c
> +++ b/io_uring/io-wq.c
> @@ -639,6 +639,7 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
>                 /* handle a whole dependent link */
>                 do {
>                         struct io_wq_work *next_hashed, *linked;
> +                       struct io_kiocb *req;
>                         unsigned int work_flags = atomic_read(&work->flags);
>                         unsigned int hash = __io_wq_is_hashed(work_flags)
>                                 ? __io_get_work_hash(work_flags)
> @@ -649,7 +650,10 @@ static void io_worker_handle_work(struct io_wq_acct *acct,
>                         if (do_kill &&
>                             (work_flags & IO_WQ_WORK_UNBOUND))
>                                 atomic_or(IO_WQ_WORK_CANCEL, &work->flags);
> +                       req = container_of(work, struct io_kiocb, work);
> +                       kcov_remote_start_common(req->ctx->kcov_handle);
>                         io_wq_submit_work(work);
> +                       kcov_remote_stop();
>                         io_assign_current_work(worker, NULL);
>
>                         linked = io_wq_free_work(work);
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 103b6c88f252..89cb649944d9 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -293,6 +293,7 @@ static __cold struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p)
>         INIT_HLIST_HEAD(&ctx->cancelable_uring_cmd);
>         io_napi_init(ctx);
>         mutex_init(&ctx->mmap_lock);
> +       ctx->kcov_handle = kcov_common_handle();
>
>         return ctx;
>
> diff --git a/io_uring/io_uring.h b/io_uring/io_uring.h
> index e612a66ee80e..7226fbbbf9f0 100644
> --- a/io_uring/io_uring.h
> +++ b/io_uring/io_uring.h
> @@ -7,6 +7,7 @@
>  #include <linux/resume_user_mode.h>
>  #include <linux/poll.h>
>  #include <linux/io_uring_types.h>
> +#include <linux/kcov.h>

I think instead of this, normal kernel coding style is to use includes
directly in the files where they are needed.
https://docs.kernel.org/process/submit-checklist.html says:
"If you use a facility then #include the file that defines/declares
that facility. Don’t depend on other header files pulling in ones that
you use."

>  #include <uapi/linux/eventpoll.h>
>  #include "alloc_cache.h"
>  #include "io-wq.h"
> @@ -581,4 +582,5 @@ static inline bool io_has_work(struct io_ring_ctx *ctx)
>         return test_bit(IO_CHECK_CQ_OVERFLOW_BIT, &ctx->check_cq) ||
>                io_local_work_pending(ctx);
>  }
> +
>  #endif

This looks like an accidental whitespace change.

> diff --git a/io_uring/sqpoll.c b/io_uring/sqpoll.c
> index 46c12afec73e..c7b78ea98587 100644
> --- a/io_uring/sqpoll.c
> +++ b/io_uring/sqpoll.c
> @@ -342,19 +342,23 @@ static int io_sq_thread(void *data)
>
>                 cap_entries = !list_is_singular(&sqd->ctx_list);
>                 list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) {
> +                       kcov_remote_start_common(ctx->kcov_handle);
>                         int ret = __io_sq_thread(ctx, sqd, cap_entries, &ist);
>
>                         if (!sqt_spin && (ret > 0 || !list_empty(&ctx->iopoll_list)))
>                                 sqt_spin = true;
> +                       kcov_remote_stop();
>                 }
>                 if (io_sq_tw(&retry_list, IORING_TW_CAP_ENTRIES_VALUE))
>                         sqt_spin = true;
>
>                 list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) {
> +                       kcov_remote_start_common(ctx->kcov_handle);
>                         if (io_napi(ctx)) {
>                                 io_sq_start_worktime(&ist);
>                                 io_napi_sqpoll_busy_poll(ctx);
>                         }
> +                       kcov_remote_stop();

Someone who knows more about networking than me might know this area
better, but I think we probably don't want to have KCOV coverage
around the call to io_napi_sqpoll_busy_poll() for two reasons:

1. This is NAPI busypolling code, designed to busy-loop until network
packets arrive - meaning the limited KCOV coverage buffer will quickly
fill up even if no data is actually being processed.
2. As far as I know, io_napi_sqpoll_busy_poll() doesn't really process
data related to the uring instance - it (more or less) merely
busy-polls network interfaces specified by the user. Received packets
are not necessarily actually related to this uring instance.

>                 }
>
>                 io_sq_update_worktime(sqd, &ist);
> --
> 2.54.0
>