RE: [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS

[Michael Kelley]

From: Frank Chen <frankchen158@xxxxxxx> Sent: Tuesday, August 4, 2026 7:30 PM
> 
> From: chenhuguanshen <chenhgs@xxxxxxxxxxxxxxx>
> 
> Under heavy concurrent DMA traffic on CoCo VMs, inc_used_and_hiwater()
> performs an atomic_long_add_return() plus a CAS loop on the global
> used_hiwater, and dec_used() performs an atomic_long_sub() on total_used.
> All CPUs contend on the same cacheline, causing measurable throughput
> degradation at scale.
> 
> Historically these counters were only compiled in under CONFIG_DEBUG_FS,
> which means production kernels without debugfs paid the atomic overhead

"with debugfs"?  "without debugfs" doesn't make sense to me.

> unconditionally once the symbols were present. Make the tracking

I'm not clear on the meaning of "once the symbols were present". Could
you clarify?

> boot-time opt-in instead so that it is disabled by default with near-zero
> overhead via static_call, and can be enabled via "swiotlb=track_hiwater"
> parameter on demand for debugging.

There's another functional implication here as well: With
CONFIG_DEBUG_FS and when high water tracking is disabled, the value
reported for "io_tlb_used" is approximate instead of instantaneously
exact.

> 
> Changes:
> 
> - Introduction of static_call infrastructure: Under CONFIG_DEBUG_FS, real
>   tracking functions and no-op stubs are defined, with DEFINE_STATIC_CALL
>   defaulting to the stubs, making overhead near-zero when disabled.
> 
> - Parsing the track_hiwater boot parameter: setup_io_tlb_npages() is updated
>   to recognize track_hiwater, which triggers static_call_update() to dynamically
>   replace the stubs with the real tracking implementations.
> 
> - Removal of old compile-time-gated definitions: The old #ifdef/#else block
>   defining inc_used_and_hiwater()/dec_used() is deleted, as its logic is now
>   replaced by the static_call approach.
> 
> - Unification of mem_used() implementation: The two separate debugfs/non-debugfs
>   versions are merged into one function that checks track_hiwater_enabled at
>   runtime to decide whether to return an accurate or approximate value.
> 
> - Update of struct field comments: Comments on total_used and used_hiwater are
>   changed from "used only in debugfs" to "enabled via swiotlb=track_hiwater boot
>   parameter and exposed via debugfs".
> 
> - Documentation of the new boot parameter: The kernel-parameters documentation
>   adds track_hiwater as a valid option for swiotlb= with its description.

To me, the code changes are straightforward and don't need a detailed
recapitulation in the commit message. The commit message should focus on
the "why" (which you've done) and perhaps a top-level summary of the
approach. You've mentioned using static calls to make the overhead near zero
when disabled, and that seems sufficient.

> 
> Suggested-by: Fan Du <fan.du@xxxxxxxxx>
> Signed-off-by: Jun Miao <jun.miao@xxxxxxxxx>
> Co-developed-by: Fan Du <fan.du@xxxxxxxxx>
> Signed-off-by: Fan Du <fan.du@xxxxxxxxx>
> Tested-by: chenhuguanshen <chenhgs@xxxxxxxxxxxxxxx>
> Signed-off-by: chenhuguanshen <chenhgs@xxxxxxxxxxxxxxx>
> 
> ---
> v1 -> v2:
>  - Change the patch title.
>  - Doing the exact hiwater calculation is dynamic and defaults to "off",
>    dynamic config would replace being under #ifdef CONFIG_DEBUG_FS
>  - The mechanism used for dynamic config needs to be one that is selectable
>    on the kernel boot line so that the exact hiwater mark during boot is
>    easily available.
> 
> v2 -> v3:
>  - When track_hiwater is enabled, we keep the original precise hiwater
>    calculation backed by the global total_used atomic counter. If disabled,
>    we switch to the approximate approach that sums per-area counters to
>    derive the total used slot count.
> ---
>  .../admin-guide/kernel-parameters.txt         |   3 +-
>  include/linux/swiotlb.h                       |   8 +-
>  kernel/dma/swiotlb.c                          | 153 +++++++++++-------
>  3 files changed, 100 insertions(+), 64 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt
> b/Documentation/admin-guide/kernel-parameters.txt
> index b5493a7f8f22..2caa9c0b3d7a 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7477,7 +7477,7 @@ Kernel parameters
>  			Execution Facility on pSeries.
> 
>  	swiotlb=	[ARM,PPC,MIPS,X86,S390,EARLY]
> -			Format: { <int> [,<int>] | force | noforce }
> +			Format: { <int> [,<int>] | force | noforce | track_hiwater}
>  			<int> -- Number of I/O TLB slabs
>  			<int> -- Second integer after comma. Number of swiotlb
>  				 areas with their own lock. Will be rounded up
> @@ -7485,6 +7485,7 @@ Kernel parameters
>  			force -- force using of bounce buffers even if they
>  			         wouldn't be automatically used by the kernel
>  			noforce -- Never use bounce buffers (for debugging)
> +			track_hiwater -- Track high watermark of swiotlb buffers

Add: "Applies only when CONFIG_DEBUG_FS is set" ?  I see several
other entries in kernel-parameters.txt that document dependencies
on CONFIG_* values.

> 
>  	switches=	[HW,M68k,EARLY]
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 3dae0f592063..008f169e4006 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -98,10 +98,10 @@ struct io_tlb_pool {
>   * @pools:	List of IO TLB memory pool descriptors (if dynamic).
>   * @dyn_alloc:	Dynamic IO TLB pool allocation work.
>   * @total_used:	The total number of slots in the pool that are currently used
> - *		across all areas. Used only for calculating used_hiwater in
> - *		debugfs.
> - * @used_hiwater: The high water mark for total_used.  Used only for reporting
> - *		in debugfs.
> + *		across all areas. Used only for calculating used_hiwater via boot
> + *		parameter swiotlb=track_hiwater and exposed via debugfs.
> + * @used_hiwater: The high water mark for total_used.  Can be enabled at boot
> + *		time via swiotlb=track_hiwater and exposed via debugfs.
>   * @transient_nslabs: The total number of slots in all transient pools that
>   *		are currently used across all areas.
>   */
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 1abd3e6146f4..799eee3bf9cf 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -180,6 +180,74 @@ static unsigned int limit_nareas(unsigned int nareas, unsigned long nslots)
>  	return nareas;
>  }
> 
> +#ifdef CONFIG_DEBUG_FS
> +/*
> + * Track the total used slots with a global atomic value in order to have
> + * correct information to determine the high water mark.
> + */
> +static void inc_used_and_hiwater_real(struct io_tlb_mem *mem,
> +				      unsigned int nslots)
> +{
> +	unsigned long old_hiwater, new_used;
> +
> +	new_used = atomic_long_add_return(nslots, &mem->total_used);
> +	old_hiwater = atomic_long_read(&mem->used_hiwater);
> +	do {
> +		if (new_used <= old_hiwater)
> +			break;
> +	} while (!atomic_long_try_cmpxchg(&mem->used_hiwater,
> +					  &old_hiwater, new_used));
> +}
> +
> +static void dec_used_real(struct io_tlb_mem *mem, unsigned int nslots)
> +{
> +	atomic_long_sub(nslots, &mem->total_used);
> +}
> +
> +static void inc_used_and_hiwater_nop(struct io_tlb_mem *mem,
> +				     unsigned int nslots)
> +{
> +}
> +static void dec_used_nop(struct io_tlb_mem *mem, unsigned int nslots)
> +{
> +}
> +
> +DEFINE_STATIC_CALL(swiotlb_inc_used, inc_used_and_hiwater_nop);
> +DEFINE_STATIC_CALL(swiotlb_dec_used, dec_used_nop);
> +
> +static __always_inline void inc_used_and_hiwater(struct io_tlb_mem *mem,
> +						  unsigned int nslots)
> +{
> +	static_call(swiotlb_inc_used)(mem, nslots);
> +}
> +
> +static __always_inline void dec_used(struct io_tlb_mem *mem,
> +				     unsigned int nslots)
> +{
> +	static_call(swiotlb_dec_used)(mem, nslots);
> +}
> +
> +static bool track_hiwater_enabled __read_mostly;
> +
> +#else
> +
> +static __always_inline void inc_used_and_hiwater(struct io_tlb_mem *mem,
> +						  unsigned int nslots)
> +{
> +}
> +
> +static __always_inline void dec_used(struct io_tlb_mem *mem,
> +				     unsigned int nslots)
> +{
> +}
> +#endif
> +
> +/*
> + * The tracking of used slots high watermark can be enabled
> + * by appending "track_hiwater" to the swiotlb= boot parameter.
> + * When disabled the tracking functions are no-ops with near-zero
> + * overhead via static_call.
> + */
>  static int __init
>  setup_io_tlb_npages(char *str)
>  {
> @@ -194,10 +262,24 @@ setup_io_tlb_npages(char *str)
>  		swiotlb_adjust_nareas(simple_strtoul(str, &str, 0));
>  	if (*str == ',')
>  		++str;
> -	if (!strcmp(str, "force"))
> +	if (!strncmp(str, "force", 5)) {
>  		swiotlb_force_bounce = true;
> -	else if (!strcmp(str, "noforce"))
> +		str += 5;
> +	} else if (!strncmp(str, "noforce", 7)) {
>  		swiotlb_force_disable = true;
> +		str += 7;
> +	}
> +
> +#ifdef CONFIG_DEBUG_FS
> +	if (*str == ',')
> +		++str;
> +	if (!strncmp(str, "track_hiwater", 13)) {
> +		track_hiwater_enabled = true;
> +		static_call_update(swiotlb_inc_used,
> +					inc_used_and_hiwater_real);
> +		static_call_update(swiotlb_dec_used, dec_used_real);
> +	}
> +#endif
> 
>  	return 0;
>  }
> @@ -959,40 +1041,6 @@ static unsigned int wrap_area_index(struct io_tlb_pool *mem, unsigned int index)
>  	return index;
>  }
> 
> -/*
> - * Track the total used slots with a global atomic value in order to have
> - * correct information to determine the high water mark. The mem_used()
> - * function gives imprecise results because there's no locking across
> - * multiple areas.
> - */
> -#ifdef CONFIG_DEBUG_FS
> -static void inc_used_and_hiwater(struct io_tlb_mem *mem, unsigned int nslots)
> -{
> -	unsigned long old_hiwater, new_used;
> -
> -	new_used = atomic_long_add_return(nslots, &mem->total_used);
> -	old_hiwater = atomic_long_read(&mem->used_hiwater);
> -	do {
> -		if (new_used <= old_hiwater)
> -			break;
> -	} while (!atomic_long_try_cmpxchg(&mem->used_hiwater,
> -					  &old_hiwater, new_used));
> -}
> -
> -static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
> -{
> -	atomic_long_sub(nslots, &mem->total_used);
> -}
> -
> -#else /* !CONFIG_DEBUG_FS */
> -static void inc_used_and_hiwater(struct io_tlb_mem *mem, unsigned int nslots)
> -{
> -}
> -static void dec_used(struct io_tlb_mem *mem, unsigned int nslots)
> -{
> -}
> -#endif /* CONFIG_DEBUG_FS */
> -
>  #ifdef CONFIG_SWIOTLB_DYNAMIC
>  #ifdef CONFIG_DEBUG_FS
>  static void inc_transient_used(struct io_tlb_mem *mem, unsigned int nslots)
> @@ -1295,24 +1343,6 @@ static int swiotlb_find_slots(struct device *dev,
> phys_addr_t orig_addr,
> 
>  #endif /* CONFIG_SWIOTLB_DYNAMIC */
> 
> -#ifdef CONFIG_DEBUG_FS
> -
> -/**
> - * mem_used() - get number of used slots in an allocator
> - * @mem:	Software IO TLB allocator.
> - *
> - * The result is accurate in this version of the function, because an atomic
> - * counter is available if CONFIG_DEBUG_FS is set.
> - *
> - * Return: Number of used slots.
> - */
> -static unsigned long mem_used(struct io_tlb_mem *mem)
> -{
> -	return atomic_long_read(&mem->total_used);
> -}
> -
> -#else /* !CONFIG_DEBUG_FS */
> -
>  /**
>   * mem_pool_used() - get number of used slots in a memory pool
>   * @pool:	Software IO TLB memory pool.
> @@ -1335,13 +1365,20 @@ static unsigned long mem_pool_used(struct io_tlb_pool *pool)
>   * mem_used() - get number of used slots in an allocator
>   * @mem:	Software IO TLB allocator.
>   *
> - * The result is not accurate, because there is no locking of individual
> - * areas.
> + * When trace_hiwater and CONFIG_DEBUG_FS is enabled, the result is accurate
> + * because the total number of used slots is tracked in mem->total_used.
> + * Otherwise, the result is not accurate, because there is no locking of

Rather than saying "the result is not accurate", I'd suggest saying "the
result is an approximation".  "Not accurate" sounds like it is of dubious
usefulness, while an "approximation" can be useful. :-)

> + * individual areas.
>   *
> - * Return: Approximate number of used slots.
> + * Return: Number of used slots.
>   */
>  static unsigned long mem_used(struct io_tlb_mem *mem)
>  {
> +#ifdef CONFIG_DEBUG_FS
> +    if (track_hiwater_enabled)
> +	    return atomic_long_read(&mem->total_used);
> +#endif
> +
>  #ifdef CONFIG_SWIOTLB_DYNAMIC
>  	struct io_tlb_pool *pool;
>  	unsigned long used = 0;
> @@ -1357,8 +1394,6 @@ static unsigned long mem_used(struct io_tlb_mem *mem)
>  #endif
>  }
> 
> -#endif /* CONFIG_DEBUG_FS */
> -
>  /**
>   * swiotlb_tbl_map_single() - bounce buffer map a single contiguous physical area
>   * @dev:		Device which maps the buffer.
> --
> 2.53.0
> 

Modulo the few nits about the commit message, documentation, and comments,
this looks good to me. I've tested on x86/x64 against linux-next20260726 with
and without CONFIG_DEBUG_FS, and with and without the swiotlb=track_hiwater.
All seems good.

Reviewed-by: Michael Kelley <mhklinux@xxxxxxxxxxx>
Tested-by: Michael Kelley <mhklinux@xxxxxxxxxxx>