RE: [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in

[Michael Kelley]

From: Frank Chen <frankchen158@xxxxxxx> Sent: Monday, July 13, 2026 8:25 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 with debugfs paid the atomic overhead
> unconditionally once the symbols were present. Make the tracking
> 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.
> 
> Changes:
> 
> - The previous inc_used_and_hiwater() maintained a separate atomic
>   total_used counter for accurate hiwater updates. This is replaced with
>   mem_used() which sums across areas. While less precise (no cross-area
>   locking), the result is adequate for high watermark statistics and
>   avoids the overhead of maintaining total_used on every alloc/free.

I'm not sold on giving up an accurate hiwater calculation. If it's
optional, let's have the option be to maintain total_used on every
alloc/free so the hiwater calculation is accurate.

Maintaining hiwater based on mem_used() is already pretty low
overhead in that it only does a global atomic operation if the hiwater is
raised. After a startup transient that establishes a typical steady-state
hiwater, most ongoing swiotlb alloc/free operations won't raise
the hiwater and so won't do any atomic operations to maintain it.

To recap, there are three levels of hiwater maintenance:

1) None
2) Based on the less precise mem_used()
3) Based on an atomically update global total_used value

The perf difference between #1 and #2 seems minimal outside of
the initial ramp to a steady-state hiwater value. The perf impact
of #3 can indeed be more significant, especially at high CPU counts.

Having the kernel boot line offer all three is arguably overkill.
I'd suggest offering #1 and #3 (with #1 as the default), or #2
and #3 (with #2 as the default and !CONFIG_DEBUG_FS providing
#1).

There's another aspect here as well. Robin Murphy had 
suggested a sysctl option, so that the accurate hiwater maintenance
could be turned on/off at runtime, not just a boot time. My earlier
comment about boot time was probably unclear. I was thinking in
terms of a runtime option that could *also* be set at boot time, which
sysctl does. But I'll admit that enabling accurate hiwater calculation
at runtime is tricky because some swiotlb buffers may be allocated
at the time of the enablement, and total_used could go negative
when those are freed. For runtime sysctl enablement, would
probably have to do stop_machine() to freeze everything, and set
total_used to the value of mem_used() so that everything is
correct. I haven't worked through all the details.

All that said, I don't want to make too big of an issue out of this,
and end up over-engineering with complexity that exceeds the
value. I could be persuaded to have boot time enablement but
not runtime enablement. I'm less happy with losing entirely
the accurate calculation (my #3 above) that's in the current
upstream code.

Michael

> 
> - Refactor high watermark tracking to use static_call, allowing it to be
>   enabled at boot via the swiotlb=track_hiwater parameter when
>   CONFIG_DEBUG_FS is set. When disabled (the default), the static_call
>   resolves to a no-op function, ensuring near-zero overhead.
> 
> - Remove the total_used atomic counter from io_tlb_mem since it is no
>   longer needed. The debugfs "io_tlb_used" file already uses mem_used()
>   to report current usage.
> 
> 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.
> 
> ---
>  .../admin-guide/kernel-parameters.txt         |   3 +-
>  include/linux/swiotlb.h                       |  10 +-
>  kernel/dma/swiotlb.c                          | 125 +++++++++---------
>  3 files changed, 68 insertions(+), 70 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt
> b/Documentation/admin-guide/kernel-parameters.txt
> index b2d7d3540ded..bb60c55383a0 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7485,7 +7485,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
> @@ -7493,6 +7493,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
> 
>  	switches=	[HW,M68k,EARLY]
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 3dae0f592063..e308792243cc 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -97,11 +97,8 @@ struct io_tlb_pool {
>   * @lock:	Lock to synchronize changes to the list.
>   * @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.
> + * @used_hiwater: The high watermark of used slots. Can be enabled at
> + *              boot time via swiotlb=track_hiwater.
>   * @transient_nslabs: The total number of slots in all transient pools that
>   *		are currently used across all areas.
>   */
> @@ -119,9 +116,8 @@ struct io_tlb_mem {
>  	struct work_struct dyn_alloc;
>  #endif
>  #ifdef CONFIG_DEBUG_FS
> -	atomic_long_t total_used;
> -	atomic_long_t used_hiwater;
>  	atomic_long_t transient_nslabs;
> +	atomic_long_t used_hiwater;
>  #endif
>  };
> 
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 1abd3e6146f4..5828163c15f7 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -180,8 +180,55 @@ static unsigned int limit_nareas(unsigned int nareas, unsigned
> long nslots)
>  	return nareas;
>  }
> 
> -static int __init
> -setup_io_tlb_npages(char *str)
> +#ifdef CONFIG_DEBUG_FS
> +static unsigned long mem_used(struct io_tlb_mem *mem);
> +
> +/*
> + * Track the total used slots to determine the high watermark.
> + * Unlike the previous total_used atomic counter which provided
> + * exact values, mem_used() iterates areas without cross-area
> + * locking and may return imprecise results under concurrent
> + * alloc/free. This is acceptable for high watermark statistics
> + * and avoids the overhead of maintaining a dedicated atomic
> + * counter on every allocation and free.
> + */
> +static void account_hiwater_real(struct io_tlb_mem *mem)
> +{
> +	unsigned long old_hiwater, new_used;
> +
> +	new_used = mem_used(mem);
> +	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 account_hiwater_nop(struct io_tlb_mem *mem)
> +{
> +}
> +
> +DEFINE_STATIC_CALL(swiotlb_account_hiwater, account_hiwater_nop);
> +
> +static __always_inline void account_hiwater(struct io_tlb_mem *mem)
> +{
> +	static_call(swiotlb_account_hiwater)(mem);
> +}
> +
> +#else
> +static __always_inline void account_hiwater(struct io_tlb_mem *mem)
> +{
> +}
> +#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)
>  {
>  	if (isdigit(*str)) {
>  		/* avoid tail segment of size < IO_TLB_SEGSIZE */
> @@ -194,10 +241,21 @@ 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)) {
> +		static_call_update(swiotlb_account_hiwater,
> +				   account_hiwater_real);
> +	}
> +#endif
> 
>  	return 0;
>  }
> @@ -959,40 +1017,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)
> @@ -1134,7 +1158,7 @@ static int swiotlb_search_pool_area(struct device *dev,
> struct io_tlb_pool *pool
>  	area->used += nslots;
>  	spin_unlock_irqrestore(&area->lock, flags);
> 
> -	inc_used_and_hiwater(dev->dma_io_tlb_mem, nslots);
> +	account_hiwater(dev->dma_io_tlb_mem);
>  	return slot_index;
>  }
> 
> @@ -1295,24 +1319,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.
> @@ -1357,8 +1363,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.
> @@ -1508,8 +1512,6 @@ static void swiotlb_release_slots(struct device *dev,
> phys_addr_t tlb_addr,
>  		mem->slots[i].list = ++count;
>  	area->used -= nslots;
>  	spin_unlock_irqrestore(&area->lock, flags);
> -
> -	dec_used(dev->dma_io_tlb_mem, nslots);
>  }
> 
>  #ifdef CONFIG_SWIOTLB_DYNAMIC
> @@ -1531,7 +1533,6 @@ static bool swiotlb_del_transient(struct device *dev,
> phys_addr_t tlb_addr,
>  	if (!pool->transient)
>  		return false;
> 
> -	dec_used(dev->dma_io_tlb_mem, pool->nslabs);
>  	swiotlb_del_pool(dev, pool);
>  	dec_transient_used(dev->dma_io_tlb_mem, pool->nslabs);
>  	return true;
> --
> 2.34.1
>