Re: [RFC PATCH v2 6/6] kcov: add recursion guard and documentation for kcov-dataflow

Peter Zijlstra <[email protected]> Thu, 4 Jun 2026 10:52:43 +0200
Newsgroups org.kernel.vger.workflows,dev.linux.lists.llvm,org.kernel.vger.linux-doc,org.kernel.vger.linux-kbuild,org.kernel.vger.linux-kernel,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Wed, Jun 03, 2026 at 07:43:33PM +0200, Yunseong Kim wrote:
> Add a per-task recursion guard to kcov_df_write() using the high bit of
> kcov_dataflow_seq. This prevents infinite recursion when
> CONFIG_KCOV_DATAFLOW_INSTRUMENT_ALL is enabled: functions called by the
> callback itself (copy_from_kernel_nofault, xadd helpers) are also
> instrumented and would re-enter kcov_df_write() without this guard.
> 
> The guard uses the sequence counter's bit 31 as a re-entrancy flag.
> The low 24 bits (used for TLV record sequence numbers) are unaffected.
> 
> Also:
> - Exclude kcov.o, extable.o, softirq.o from dataflow instrumentation
>   (same pattern as KCOV_INSTRUMENT exclusions)
> - Add Documentation/dev-tools/kcov-dataflow.rst with:
>   - Prerequisites and Kconfig options
>   - Per-module instrumentation instructions
>   - Complete C example for data collection
>   - Ring buffer format specification
>   - Ioctl interface reference
>   - Fork interception example for child process tracing
>   - Rust module support via post-compilation pipeline
> 
> Signed-off-by: Yunseong Kim <[email protected]>

Another really weird patch. Adding Documentation is okay I suppose,
although I still utterly detest this rst crap. I still fail to see
what's wrong with plain text :-(

But then you do these two other random things in the same patch. Which
make no sense.

> diff --git a/kernel/Makefile b/kernel/Makefile
> index 1e1a31673577..9c56421c5390 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -37,6 +37,7 @@ KCOV_INSTRUMENT_extable.o := n
>  KCOV_INSTRUMENT_stacktrace.o := n
>  # Don't self-instrument.
>  KCOV_INSTRUMENT_kcov.o := n
> +KCOV_DATAFLOW_kcov.o := n
>  # If sanitizers detect any issues in kcov, it may lead to recursion
>  # via printk, etc.
>  KASAN_SANITIZE_kcov.o := n
> @@ -207,3 +208,5 @@ $(obj)/kheaders.md5: $(obj)/kheaders-srclist FORCE
>  	$(call filechk,kheaders_md5sum)
>  
>  clean-files := kheaders.md5 kheaders-srclist kheaders-objlist
> +KCOV_DATAFLOW_extable.o := n
> +KCOV_DATAFLOW_softirq.o := n

Why?!?! You Changelog also does not elucidate.

> diff --git a/kernel/kcov.c b/kernel/kcov.c
> index 373b8034ca5c..8d9d5e33549f 100644
> --- a/kernel/kcov.c
> +++ b/kernel/kcov.c
> @@ -413,6 +413,16 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
>  	if (!in_task())
>  		return;
>  
> +	/*
> +	 * Prevent recursion: functions called by this callback
> +	 * (copy_from_kernel_nofault, xadd helpers) may be instrumented
> +	 * with INSTRUMENT_ALL. Use a per-task guard via the sequence
> +	 * counter's high bit.
> +	 */
> +	if (t->kcov_dataflow_seq & (1U << 31))
> +		return;
> +	t->kcov_dataflow_seq |= (1U << 31);
> +
>  	area = (u64 *)t->kcov_df_area;
>  	if (!area)
>  		return;
> @@ -449,7 +459,7 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
>  		if (KCOV_DF_IS_ERR(ptr)) {
>  			for (i = 0; i < num_fields; i++)
>  				area[pos + 3 + i] = KCOV_DF_MAGIC_BAD;
> -			return;
> +			goto out;
>  		}
>  		for (i = 0; i < num_fields; i++) {
>  			u64 off, sz, val = KCOV_DF_MAGIC_BAD;
> @@ -469,6 +479,8 @@ kcov_df_write(u64 type_marker, u64 pc, u64 meta, void *ptr,
>  			area[pos + 3 + i] = val;
>  		}
>  	}
> +out:
> +	t->kcov_dataflow_seq &= ~(1U << 31);
>  }
>  
>  #ifdef CONFIG_KCOV_DATAFLOW_ARGS

This needs barrier()s to be functional. And should be in a separate
patch.