mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
       [not found] <20260622122114.2563254-1-jun.miao@intel.com>
@ 2026-06-23  2:35 ` Michael Kelley
  2026-06-23 12:22   ` Robin Murphy
  2026-06-25  7:30   ` Du, Fan
  2026-07-14  3:25 ` [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in Frank Chen
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 19+ messages in thread
From: Michael Kelley @ 2026-06-23  2:35 UTC (permalink / raw)
  To: Jun Miao, m.szyprowski, robin.murphy; +Cc: iommu, chenhgs, fan.du, LKML


From: Jun Miao <jun.miao@intel.com> Sent: Monday, June 22, 2026 5:21 AM
>

[Adding LKML to the Cc: list]

> Under heavy concurrent DMA traffic, 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.

This statement seems overly broad. Normally the swiotlb is rarely used, so
heavy concurrent DMA traffic doesn't generate this problem. The bad case
would be in a CoCo VM where all DMA traffic goes through the swiotlb. If
your statement is based on the CoCo VM case, please call that out.

> All CPUs contend
> on the same cacheline, causing measurable throughput degradation at scale.
> Both counters are debug-only (CONFIG_DEBUG_FS).
> 
> Move hiwater tracking into struct io_tlb_area, where area->used is
> already maintained under area->lock:
> 
> - Add unsigned long used_hiwater to struct io_tlb_area.  Update it
>   inside the existing area->lock critical section in
>   swiotlb_search_pool_area() immediately after area->used is
>   incremented, at zero additional synchronisation cost.
> 
> - Remove atomic_long_t total_used and used_hiwater from struct
>   io_tlb_mem; make inc_used_and_hiwater()/dec_used() empty stubs.
> 
> - Unify the two CONFIG_DEBUG_FS / !CONFIG_DEBUG_FS versions of
>   mem_used() into a single implementation that sums per-area used
>   counts with READ_ONCE().
> 
> - Rewrite io_tlb_hiwater_get() to sum area->used_hiwater across all
>   areas (locklessly with READ_ONCE()); rewrite io_tlb_hiwater_set() to
>   reset each area under its own spinlock.
> 
> Result: zero global atomic operations on the DMA map/unmap hot path.

Another result (as I describe below in the code) is that the reported
hiwater value can now be egregiously wrong. Back when I added
reporting the hiwater value in commit 8b0977ecc8b3, there was
considerable discussion of the potential impact of the atomic operations,
and the decision was to place reporting under CONFIG_DEBUG_FS as a
mitigation option if needed.

At the time, the use case for the hiwater mark was to enable a sysadmin
to see how much peak swiotlb space is needed for a particular workload.
This info would allow the swiotlb size to be set smaller than the rather large
default that is used in CoCo VMs, and free up memory for real workload
usage. I thought about the approach you've taken here, but my view at
the time was that approach is inaccurate enough so as to be fairly
worthless for the intended use case.

> 
> Without this patch, the inc_used_and_hiwater() accounts for the most time
> from perf assembly record as following log:
> 
>        | spin_unlock_irqrestore():
>        | - call    _raw_spin_unlock_irqrestore
>        | swiotlb_area_find_slots():
>  10.38 |   mov    $0x60(%rsp),%rax
>   0.00 |   mov    $0x2b8(%rax),%rcx
>        | arch_atomic64_add_return():
>   0.00 |   mov    %r15,%rax
>   0.00 |   lock    xadd    %rax,0x58(%rcx)
>  66.74 |   lea    (%r15,%rax,1),%rdx
>        | arch_atomic64_read():
>   0.14 |   mov    $0x60(%rcx),%rax
>        | inc_used_and_hiwater():
>  19.74 |   cmp    %rdx,%rax
>        |   jae    43f
>        | arch_atomic64_try_cmpxchg():
>        |   lock    cmpxchg %rdx,0x60(%rcx)
>        |   jne    432
>        | swiotlb_area_find_slots():
>   0.07 |   mov    %ebx,%eax
>   0.04 |   jmp    289
>        | wrap_area_index():
>        |   xor    %edx,%edx
>        |   jmp    3fb
> 
> Suggested-by: Fan Du <fan.du@intel.com>
> Co-developed-by: Chenhgs <chenhgs@chinatelecom.cn>
> Tested-by: Chenhgs <chenhgs@chinatelecom.cn>
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> ---
>  include/linux/swiotlb.h |   7 ---
>  kernel/dma/swiotlb.c    | 109 +++++++++++++++++-----------------------
>  2 files changed, 45 insertions(+), 71 deletions(-)
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index 3dae0f592063..a92496786cb5 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -97,11 +97,6 @@ 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.
>   * @transient_nslabs: The total number of slots in all transient pools that
>   *		are currently used across all areas.
>   */
> @@ -119,8 +114,6 @@ 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;
>  #endif
>  };
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index 1abd3e6146f4..9b4130e3f8e8 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -107,12 +107,15 @@ static unsigned long default_nareas;
>   * This is a single area with a single lock.
>   *
>   * @used:	The number of used IO TLB block.
> + * @used_hiwater: Per-area high-water mark of used slots. Updated under
> + *		@lock; read locklessly for approximate debugfs reporting.
>   * @index:	The slot index to start searching in this area for next round.
>   * @lock:	The lock to protect the above data structures in the map and
>   *		unmap calls.
>   */
>  struct io_tlb_area {
>  	unsigned long used;
> +	unsigned long used_hiwater;
>  	unsigned int index;
>  	spinlock_t lock;
>  };
> @@ -959,40 +962,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)
> @@ -1132,9 +1101,10 @@ static int swiotlb_search_pool_area(struct device *dev, struct io_tlb_pool *pool
>  	 */
>  	area->index = wrap_area_index(pool, index + nslots);
>  	area->used += nslots;
> +	if (area->used > area->used_hiwater)
> +		area->used_hiwater = area->used;
>  	spin_unlock_irqrestore(&area->lock, flags);
> 
> -	inc_used_and_hiwater(dev->dma_io_tlb_mem, nslots);
>  	return slot_index;
>  }
> 
> @@ -1295,29 +1265,12 @@ 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.
>   *
> - * The result is not accurate, see mem_used().
> + * Returns the sum of per-area used slot counts. The result is approximate
> + * since there is no locking across areas.
>   *
>   * Return: Approximate number of used slots.
>   */
> @@ -1327,7 +1280,7 @@ static unsigned long mem_pool_used(struct io_tlb_pool *pool)
>  	unsigned long used = 0;
> 
>  	for (i = 0; i < pool->nareas; i++)
> -		used += pool->areas[i].used;
> +		used += READ_ONCE(pool->areas[i].used);
>  	return used;
>  }
> 
> @@ -1335,8 +1288,8 @@ 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.
> + * Returns the sum of per-area used slot counts across all pools. The result
> + * is approximate since there is no locking across areas.
>   *
>   * Return: Approximate number of used slots.
>   */
> @@ -1357,8 +1310,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 +1459,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 +1480,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;
> @@ -1710,20 +1658,53 @@ static int io_tlb_used_get(void *data, u64 *val)
>  static int io_tlb_hiwater_get(void *data, u64 *val)
>  {
>  	struct io_tlb_mem *mem = data;
> +	unsigned long hiwater = 0;
> +	struct io_tlb_pool *pool;
> +	int i;
> 
> -	*val = atomic_long_read(&mem->used_hiwater);
> +#ifdef CONFIG_SWIOTLB_DYNAMIC
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(pool, &mem->pools, node)
> +		for (i = 0; i < pool->nareas; i++)
> +			hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> +	rcu_read_unlock();
> +#else
> +	pool = &mem->defpool;
> +	for (i = 0; i < pool->nareas; i++)
> +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);

Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
of calculating a separate hiwater mark for each area, and then summing
those per-area hiwater marks, can produce very wrong results.

Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
each with 8 MiB of space. Suppose the workload putters along with mostly
smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2 MiB.
But then suppose a 2 MiB I/O comes in. The hiwater mark for the area that
handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
 another 2 MiB I/O comes in that is handled by a different area, whose
hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
old algorithm would have reported 4+ MiB, which is accurate. With
higher CPU counts and more areas, the discrepancy can get much worse.
This is a somewhat contrived example, but the problem is real enough
to make the reported hiwater mark be unreliable.

I'm sure the contention for the total hiwater mark in the current code is
real, but it's in the context of a lot of other CPU work that is being because
of the bounce buffering, including copying lots of data to/from the bounce
buffers. Is that atomic increment operation a bottleneck even in the
end-to-end context of doing DMA through swiotlb bounce buffers?

Another approach to the contention problem would be to have a separate
CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
computation of the hiwater mark can be dropped entirely in production
environments. Or the setting could be dynamic at runtime via a
static_call, defaulting to not computing the hiwater mark while still
allowing a sysadmin to turn it on to see workload usage of the swiotlb.

Michael

> +#endif
> +	*val = hiwater;
>  	return 0;
>  }
> 
>  static int io_tlb_hiwater_set(void *data, u64 val)
>  {
>  	struct io_tlb_mem *mem = data;
> +	struct io_tlb_pool *pool;
> +	unsigned long flags;
> +	int i;
> 
>  	/* Only allow setting to zero */
>  	if (val != 0)
>  		return -EINVAL;
> 
> -	atomic_long_set(&mem->used_hiwater, val);
> +#ifdef CONFIG_SWIOTLB_DYNAMIC
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(pool, &mem->pools, node)
> +		for (i = 0; i < pool->nareas; i++) {
> +			spin_lock_irqsave(&pool->areas[i].lock, flags);
> +			pool->areas[i].used_hiwater = 0;
> +			spin_unlock_irqrestore(&pool->areas[i].lock, flags);
> +		}
> +	rcu_read_unlock();
> +#else
> +	pool = &mem->defpool;
> +	for (i = 0; i < pool->nareas; i++) {
> +		spin_lock_irqsave(&pool->areas[i].lock, flags);
> +		pool->areas[i].used_hiwater = 0;
> +		spin_unlock_irqrestore(&pool->areas[i].lock, flags);
> +	}
> +#endif
>  	return 0;
>  }
> 
> --
> 2.32.0
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-23  2:35 ` [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking Michael Kelley
@ 2026-06-23 12:22   ` Robin Murphy
  2026-06-25  7:30   ` Du, Fan
  1 sibling, 0 replies; 19+ messages in thread
From: Robin Murphy @ 2026-06-23 12:22 UTC (permalink / raw)
  To: Michael Kelley, Jun Miao, m.szyprowski; +Cc: iommu, chenhgs, fan.du, LKML

On 2026-06-23 3:35 am, Michael Kelley wrote:
[...]
> Another approach to the contention problem would be to have a separate
> CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> computation of the hiwater mark can be dropped entirely in production
> environments. Or the setting could be dynamic at runtime via a
> static_call, defaulting to not computing the hiwater mark while still
> allowing a sysadmin to turn it on to see workload usage of the swiotlb.

Yeah, in terms of usefulness to admins, I'd imagine a sysctl to enable 
stats collection would probably be better than a config option - I doubt 
that most folks who just want to tune a standard distro kernel for their 
VM workload would want to have to rebuild it. A command line option 
might also be a fair compromise if that's how the eventual SWIOTLB 
tuning will be controlled anyway.

Thanks,
Robin.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-23  2:35 ` [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking Michael Kelley
  2026-06-23 12:22   ` Robin Murphy
@ 2026-06-25  7:30   ` Du, Fan
  2026-06-25 15:53     ` Michael Kelley
  1 sibling, 1 reply; 19+ messages in thread
From: Du, Fan @ 2026-06-25  7:30 UTC (permalink / raw)
  To: Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML, Du, Fan



> -----Original Message-----
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Tuesday, June 23, 2026 10:35 AM
> To: Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> robin.murphy@arm.com
> Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; Du, Fan
> <fan.du@intel.com>; LKML <linux-kernel@vger.kernel.org>
> Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> used/hiwater tracking
> 
> 
> From: Jun Miao <jun.miao@intel.com> Sent: Monday, June 22, 2026 5:21 AM
> >
> 
> [Adding LKML to the Cc: list]
> 
> > Under heavy concurrent DMA traffic, 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.
> 
> This statement seems overly broad. Normally the swiotlb is rarely used, so
> heavy concurrent DMA traffic doesn't generate this problem. The bad case
> would be in a CoCo VM where all DMA traffic goes through the swiotlb. If
> your statement is based on the CoCo VM case, please call that out.
> 
> > All CPUs contend
> > on the same cacheline, causing measurable throughput degradation at
> scale.
> > Both counters are debug-only (CONFIG_DEBUG_FS).
> >
> > Move hiwater tracking into struct io_tlb_area, where area->used is
> > already maintained under area->lock:
> >
> > - Add unsigned long used_hiwater to struct io_tlb_area.  Update it
> >   inside the existing area->lock critical section in
> >   swiotlb_search_pool_area() immediately after area->used is
> >   incremented, at zero additional synchronisation cost.
> >
> > - Remove atomic_long_t total_used and used_hiwater from struct
> >   io_tlb_mem; make inc_used_and_hiwater()/dec_used() empty stubs.
> >
> > - Unify the two CONFIG_DEBUG_FS / !CONFIG_DEBUG_FS versions of
> >   mem_used() into a single implementation that sums per-area used
> >   counts with READ_ONCE().
> >
> > - Rewrite io_tlb_hiwater_get() to sum area->used_hiwater across all
> >   areas (locklessly with READ_ONCE()); rewrite io_tlb_hiwater_set() to
> >   reset each area under its own spinlock.
> >
> > Result: zero global atomic operations on the DMA map/unmap hot path.
> 
> Another result (as I describe below in the code) is that the reported
> hiwater value can now be egregiously wrong. Back when I added
> reporting the hiwater value in commit 8b0977ecc8b3, there was
> considerable discussion of the potential impact of the atomic operations,
> and the decision was to place reporting under CONFIG_DEBUG_FS as a
> mitigation option if needed.
> 
> At the time, the use case for the hiwater mark was to enable a sysadmin
> to see how much peak swiotlb space is needed for a particular workload.
> This info would allow the swiotlb size to be set smaller than the rather large
> default that is used in CoCo VMs, and free up memory for real workload



> usage. I thought about the approach you've taken here, but my view at
> the time was that approach is inaccurate enough so as to be fairly
> worthless for the intended use case.
> 
> >
> > Without this patch, the inc_used_and_hiwater() accounts for the most time
> > from perf assembly record as following log:
> >
> >        | spin_unlock_irqrestore():
> >        | - call    _raw_spin_unlock_irqrestore
> >        | swiotlb_area_find_slots():
> >  10.38 |   mov    $0x60(%rsp),%rax
> >   0.00 |   mov    $0x2b8(%rax),%rcx
> >        | arch_atomic64_add_return():
> >   0.00 |   mov    %r15,%rax
> >   0.00 |   lock    xadd    %rax,0x58(%rcx)
> >  66.74 |   lea    (%r15,%rax,1),%rdx
> >        | arch_atomic64_read():
> >   0.14 |   mov    $0x60(%rcx),%rax
> >        | inc_used_and_hiwater():
> >  19.74 |   cmp    %rdx,%rax
> >        |   jae    43f
> >        | arch_atomic64_try_cmpxchg():
> >        |   lock    cmpxchg %rdx,0x60(%rcx)
> >        |   jne    432
> >        | swiotlb_area_find_slots():
> >   0.07 |   mov    %ebx,%eax
> >   0.04 |   jmp    289
> >        | wrap_area_index():
> >        |   xor    %edx,%edx
> >        |   jmp    3fb
> >
> > Suggested-by: Fan Du <fan.du@intel.com>
> > Co-developed-by: Chenhgs <chenhgs@chinatelecom.cn>
> > Tested-by: Chenhgs <chenhgs@chinatelecom.cn>
> > Signed-off-by: Jun Miao <jun.miao@intel.com>
> > ---
> >  include/linux/swiotlb.h |   7 ---
> >  kernel/dma/swiotlb.c    | 109 +++++++++++++++++-----------------------
> >  2 files changed, 45 insertions(+), 71 deletions(-)
> >
> > diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> > index 3dae0f592063..a92496786cb5 100644
> > --- a/include/linux/swiotlb.h
> > +++ b/include/linux/swiotlb.h
> > @@ -97,11 +97,6 @@ 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.
> >   * @transient_nslabs: The total number of slots in all transient pools that
> >   *		are currently used across all areas.
> >   */
> > @@ -119,8 +114,6 @@ 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;
> >  #endif
> >  };
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index 1abd3e6146f4..9b4130e3f8e8 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -107,12 +107,15 @@ static unsigned long default_nareas;
> >   * This is a single area with a single lock.
> >   *
> >   * @used:	The number of used IO TLB block.
> > + * @used_hiwater: Per-area high-water mark of used slots. Updated under
> > + *		@lock; read locklessly for approximate debugfs reporting.
> >   * @index:	The slot index to start searching in this area for next round.
> >   * @lock:	The lock to protect the above data structures in the map and
> >   *		unmap calls.
> >   */
> >  struct io_tlb_area {
> >  	unsigned long used;
> > +	unsigned long used_hiwater;
> >  	unsigned int index;
> >  	spinlock_t lock;
> >  };
> > @@ -959,40 +962,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)
> > @@ -1132,9 +1101,10 @@ static int swiotlb_search_pool_area(struct device
> *dev, struct io_tlb_pool *pool
> >  	 */
> >  	area->index = wrap_area_index(pool, index + nslots);
> >  	area->used += nslots;
> > +	if (area->used > area->used_hiwater)
> > +		area->used_hiwater = area->used;
> >  	spin_unlock_irqrestore(&area->lock, flags);
> >
> > -	inc_used_and_hiwater(dev->dma_io_tlb_mem, nslots);
> >  	return slot_index;
> >  }
> >
> > @@ -1295,29 +1265,12 @@ 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.
> >   *
> > - * The result is not accurate, see mem_used().
> > + * Returns the sum of per-area used slot counts. The result is approximate
> > + * since there is no locking across areas.
> >   *
> >   * Return: Approximate number of used slots.
> >   */
> > @@ -1327,7 +1280,7 @@ static unsigned long mem_pool_used(struct
> io_tlb_pool *pool)
> >  	unsigned long used = 0;
> >
> >  	for (i = 0; i < pool->nareas; i++)
> > -		used += pool->areas[i].used;
> > +		used += READ_ONCE(pool->areas[i].used);
> >  	return used;
> >  }
> >
> > @@ -1335,8 +1288,8 @@ 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.
> > + * Returns the sum of per-area used slot counts across all pools. The result
> > + * is approximate since there is no locking across areas.
> >   *
> >   * Return: Approximate number of used slots.
> >   */
> > @@ -1357,8 +1310,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 +1459,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 +1480,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;
> > @@ -1710,20 +1658,53 @@ static int io_tlb_used_get(void *data, u64 *val)
> >  static int io_tlb_hiwater_get(void *data, u64 *val)
> >  {
> >  	struct io_tlb_mem *mem = data;
> > +	unsigned long hiwater = 0;
> > +	struct io_tlb_pool *pool;
> > +	int i;
> >
> > -	*val = atomic_long_read(&mem->used_hiwater);
> > +#ifdef CONFIG_SWIOTLB_DYNAMIC
> > +	rcu_read_lock();
> > +	list_for_each_entry_rcu(pool, &mem->pools, node)
> > +		for (i = 0; i < pool->nareas; i++)
> > +			hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > +	rcu_read_unlock();
> > +#else
> > +	pool = &mem->defpool;
> > +	for (i = 0; i < pool->nareas; i++)
> > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> 
> Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> of calculating a separate hiwater mark for each area, and then summing
> those per-area hiwater marks, can produce very wrong results.
> 
> Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
> each with 8 MiB of space. Suppose the workload putters along with mostly
> smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
> each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2 MiB.
> But then suppose a 2 MiB I/O comes in. The hiwater mark for the area that
> handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
>  another 2 MiB I/O comes in that is handled by a different area, whose
> hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
> mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
> old algorithm would have reported 4+ MiB, which is accurate. With
> higher CPU counts and more areas, the discrepancy can get much worse.
> This is a somewhat contrived example, but the problem is real enough
> to make the reported hiwater mark be unreliable.

You are correct here, I like your way of thinking.

> I'm sure the contention for the total hiwater mark in the current code is
> real, but it's in the context of a lot of other CPU work that is being because
> of the bounce buffering, including copying lots of data to/from the bounce
> buffers. Is that atomic increment operation a bottleneck even in the
> end-to-end context of doing DMA through swiotlb bounce buffers?

Practical benchmark show case the highest IO performance for each TVM spec. 
even if a few iperf (4)workers would cause the contention here.
My guts feelings, yes, the real workload probably hit the bottleneck here.

> Another approach to the contention problem would be to have a separate
> CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> computation of the hiwater mark can be dropped entirely in production
> environments. Or the setting could be dynamic at runtime via a
> static_call, defaulting to not computing the hiwater mark while still
> allowing a sysadmin to turn it on to see workload usage of the swiotlb.

That's counter-intuitive from my perspective.
With global counters, the observation, which itself impacts the performance, 
wouldn't be able to tell the practical characterization, that's commonly lower than
max performance, in turn breaks the semantics of what's it for.

Even without those global counters, if user wants to know the hiwater value,
snapshotting used value(sum of each area as current behavior) periodically would
produce meaningful value for workload evaluation.


> Michael
> 
> > +#endif
> > +	*val = hiwater;
> >  	return 0;
> >  }
> >
> >  static int io_tlb_hiwater_set(void *data, u64 val)
> >  {
> >  	struct io_tlb_mem *mem = data;
> > +	struct io_tlb_pool *pool;
> > +	unsigned long flags;
> > +	int i;
> >
> >  	/* Only allow setting to zero */
> >  	if (val != 0)
> >  		return -EINVAL;
> >
> > -	atomic_long_set(&mem->used_hiwater, val);
> > +#ifdef CONFIG_SWIOTLB_DYNAMIC
> > +	rcu_read_lock();
> > +	list_for_each_entry_rcu(pool, &mem->pools, node)
> > +		for (i = 0; i < pool->nareas; i++) {
> > +			spin_lock_irqsave(&pool->areas[i].lock, flags);
> > +			pool->areas[i].used_hiwater = 0;
> > +			spin_unlock_irqrestore(&pool->areas[i].lock, flags);
> > +		}
> > +	rcu_read_unlock();
> > +#else
> > +	pool = &mem->defpool;
> > +	for (i = 0; i < pool->nareas; i++) {
> > +		spin_lock_irqsave(&pool->areas[i].lock, flags);
> > +		pool->areas[i].used_hiwater = 0;
> > +		spin_unlock_irqrestore(&pool->areas[i].lock, flags);
> > +	}
> > +#endif
> >  	return 0;
> >  }
> >
> > --
> > 2.32.0
> >


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-25  7:30   ` Du, Fan
@ 2026-06-25 15:53     ` Michael Kelley
  2026-06-26  3:12       ` Du, Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kelley @ 2026-06-25 15:53 UTC (permalink / raw)
  To: Du, Fan, Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML

From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30 AM
> 
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@outlook.com>
> > Sent: Tuesday, June 23, 2026 10:35 AM
> > To: Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> > robin.murphy@arm.com
> > Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; Du, Fan
> > <fan.du@intel.com>; LKML <linux-kernel@vger.kernel.org>
> > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > used/hiwater tracking
> >

[snip]

> > > +	pool = &mem->defpool;
> > > +	for (i = 0; i < pool->nareas; i++)
> > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> >
> > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> > of calculating a separate hiwater mark for each area, and then summing
> > those per-area hiwater marks, can produce very wrong results.
> >
> > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
> > each with 8 MiB of space. Suppose the workload putters along with mostly
> > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> > progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
> > each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2 MiB.
> > But then suppose a 2 MiB I/O comes in. The hiwater mark for the area that
> > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> >  another 2 MiB I/O comes in that is handled by a different area, whose
> > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
> > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
> > old algorithm would have reported 4+ MiB, which is accurate. With
> > higher CPU counts and more areas, the discrepancy can get much worse.
> > This is a somewhat contrived example, but the problem is real enough
> > to make the reported hiwater mark be unreliable.
> 
> You are correct here, I like your way of thinking.
> 
> > I'm sure the contention for the total hiwater mark in the current code is
> > real, but it's in the context of a lot of other CPU work that is being because
> > of the bounce buffering, including copying lots of data to/from the bounce
> > buffers. Is that atomic increment operation a bottleneck even in the
> > end-to-end context of doing DMA through swiotlb bounce buffers?
> 
> Practical benchmark show case the highest IO performance for each TVM spec.
> even if a few iperf (4)workers would cause the contention here.
> My guts feelings, yes, the real workload probably hit the bottleneck here.

Just curious -- what is the NIC in the TDX VM? I'm most familiar with the
Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
uses dedicated send and receive buffers that are allocated and decrypted
when the NIC is configured. Most NIC traffic goes through those buffers
instead of the swiotlb, so I probably haven't seen cases where the swiotlb
is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck for
disk I/O traffic, but the data copying tends to be the gate rather than the
allocation and freeing of swiotlb buffers.

> 
> > Another approach to the contention problem would be to have a separate
> > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> > computation of the hiwater mark can be dropped entirely in production
> > environments. Or the setting could be dynamic at runtime via a
> > static_call, defaulting to not computing the hiwater mark while still
> > allowing a sysadmin to turn it on to see workload usage of the swiotlb.
> 
> That's counter-intuitive from my perspective.
> With global counters, the observation, which itself impacts the performance,
> wouldn't be able to tell the practical characterization, that's commonly lower than
> max performance, in turn breaks the semantics of what's it for.

Agreed. If the global counters affect the performance and throughput
significantly, having an accurate hiwater mark loses some of its value. 

> 
> Even without those global counters, if user wants to know the hiwater value,
> snapshotting used value(sum of each area as current behavior) periodically would
> produce meaningful value for workload evaluation.

I'm a little skeptical of the value of just summing current usage. Doing so
tends to miss any spikes, and the spikes are the problem. If swiotlb capacity
is exceeded even for a short spike, you don't just get a performance blip.
You get I/O failures, which at least on the disk side tends to be fatal to the
application doing the I/O. Maybe the networking stack recovers well enough
and retries, resulting in just a performance reduction. But I've always thought
of swiotlb exhaustion as a fairly serious problem to be avoided at all costs.
That's why CoCo VMs allocate so much swiotlb space, even though most of
it is never used for typical workloads (at least in my experience). 

Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-25 15:53     ` Michael Kelley
@ 2026-06-26  3:12       ` Du, Fan
  2026-06-26 16:00         ` Michael Kelley
  0 siblings, 1 reply; 19+ messages in thread
From: Du, Fan @ 2026-06-26  3:12 UTC (permalink / raw)
  To: Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML, Du, Fan



> -----Original Message-----
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Thursday, June 25, 2026 11:54 PM
> To: Du, Fan <fan.du@intel.com>; Michael Kelley <mhklinux@outlook.com>;
> Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> robin.murphy@arm.com
> Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; LKML <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> used/hiwater tracking
> 
> From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30 AM
> >
> > > -----Original Message-----
> > > From: Michael Kelley <mhklinux@outlook.com>
> > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > To: Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> > > robin.murphy@arm.com
> > > Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; Du, Fan
> > > <fan.du@intel.com>; LKML <linux-kernel@vger.kernel.org>
> > > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > > used/hiwater tracking
> > >
> 
> [snip]
> 
> > > > +	pool = &mem->defpool;
> > > > +	for (i = 0; i < pool->nareas; i++)
> > > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > >
> > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> > > of calculating a separate hiwater mark for each area, and then summing
> > > those per-area hiwater marks, can produce very wrong results.
> > >
> > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
> > > each with 8 MiB of space. Suppose the workload putters along with
> mostly
> > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> > > progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
> > > each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2
> MiB.
> > > But then suppose a 2 MiB I/O comes in. The hiwater mark for the area
> that
> > > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> > >  another 2 MiB I/O comes in that is handled by a different area, whose
> > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
> > > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
> > > old algorithm would have reported 4+ MiB, which is accurate. With
> > > higher CPU counts and more areas, the discrepancy can get much worse.
> > > This is a somewhat contrived example, but the problem is real enough
> > > to make the reported hiwater mark be unreliable.
> >
> > You are correct here, I like your way of thinking.
> >
> > > I'm sure the contention for the total hiwater mark in the current code is
> > > real, but it's in the context of a lot of other CPU work that is being because
> > > of the bounce buffering, including copying lots of data to/from the bounce
> > > buffers. Is that atomic increment operation a bottleneck even in the
> > > end-to-end context of doing DMA through swiotlb bounce buffers?
> >
> > Practical benchmark show case the highest IO performance for each TVM
> spec.
> > even if a few iperf (4)workers would cause the contention here.
> > My guts feelings, yes, the real workload probably hit the bottleneck here.
> 
> Just curious -- what is the NIC in the TDX VM? I'm most familiar with the
> Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
> uses dedicated send and receive buffers that are allocated and decrypted
> when the NIC is configured. Most NIC traffic goes through those buffers
> instead of the swiotlb, so I probably haven't seen cases where the swiotlb
> is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck for
> disk I/O traffic, but the data copying tends to be the gate rather than the
> allocation and freeing of swiotlb buffers.
> 
> >
> > > Another approach to the contention problem would be to have a separate
> > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> > > computation of the hiwater mark can be dropped entirely in production
> > > environments. Or the setting could be dynamic at runtime via a
> > > static_call, defaulting to not computing the hiwater mark while still
> > > allowing a sysadmin to turn it on to see workload usage of the swiotlb.
> >
> > That's counter-intuitive from my perspective.
> > With global counters, the observation, which itself impacts the
> performance,
> > wouldn't be able to tell the practical characterization, that's commonly
> lower than
> > max performance, in turn breaks the semantics of what's it for.
> 
> Agreed. If the global counters affect the performance and throughput
> significantly, having an accurate hiwater mark loses some of its value.
> 
> >
> > Even without those global counters, if user wants to know the hiwater value,
> > snapshotting used value(sum of each area as current behavior) periodically
> would
> > produce meaningful value for workload evaluation.
> 
> I'm a little skeptical of the value of just summing current usage. Doing so
> tends to miss any spikes, and the spikes are the problem. If swiotlb capacity

That's current design when CONFIG_DEBUG_FS is off, and used as swiotlb
shortage indicator for user.

Statistically that sampled value is approximate to the true value as always.

> is exceeded even for a short spike, you don't just get a performance blip.
> You get I/O failures, which at least on the disk side tends to be fatal to the
> application doing the I/O. Maybe the networking stack recovers well enough
> and retries, resulting in just a performance reduction. But I've always thought
> of swiotlb exhaustion as a fairly serious problem to be avoided at all costs.
> That's why CoCo VMs allocate so much swiotlb space, even though most of
> it is never used for typical workloads (at least in my experience).

That's dynamic SWIOTLB is designed for.
Only when the IO is so intensive, transient buffer/DMA pool is exhausted quickly
before new shared memory pool is created.

> Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-26  3:12       ` Du, Fan
@ 2026-06-26 16:00         ` Michael Kelley
  2026-06-27 23:20           ` Du, Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kelley @ 2026-06-26 16:00 UTC (permalink / raw)
  To: Du, Fan, Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML

From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 8:12 PM
> 
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@outlook.com>
> > Sent: Thursday, June 25, 2026 11:54 PM
> >
> > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30 AM
> > >
> > > > -----Original Message-----
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > > To: Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> > > > robin.murphy@arm.com
> > > > Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; Du, Fan
> > > > <fan.du@intel.com>; LKML <linux-kernel@vger.kernel.org>
> > > > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > > > used/hiwater tracking
> > > >
> >
> > [snip]
> >
> > > > > +	pool = &mem->defpool;
> > > > > +	for (i = 0; i < pool->nareas; i++)
> > > > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > > >
> > > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> > > > of calculating a separate hiwater mark for each area, and then summing
> > > > those per-area hiwater marks, can produce very wrong results.
> > > >
> > > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
> > > > each with 8 MiB of space. Suppose the workload putters along with mostly
> > > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> > > > progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
> > > > each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2 MiB.
> > > > But then suppose a 2 MiB I/O comes in. The hiwater mark for the area that
> > > > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> > > >  another 2 MiB I/O comes in that is handled by a different area, whose
> > > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
> > > > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
> > > > old algorithm would have reported 4+ MiB, which is accurate. With
> > > > higher CPU counts and more areas, the discrepancy can get much worse.
> > > > This is a somewhat contrived example, but the problem is real enough
> > > > to make the reported hiwater mark be unreliable.
> > >
> > > You are correct here, I like your way of thinking.
> > >
> > > > I'm sure the contention for the total hiwater mark in the current code is
> > > > real, but it's in the context of a lot of other CPU work that is being because
> > > > of the bounce buffering, including copying lots of data to/from the bounce
> > > > buffers. Is that atomic increment operation a bottleneck even in the
> > > > end-to-end context of doing DMA through swiotlb bounce buffers?
> > >
> > > Practical benchmark show case the highest IO performance for each TVM spec.
> > > even if a few iperf (4)workers would cause the contention here.
> > > My guts feelings, yes, the real workload probably hit the bottleneck here.
> >
> > Just curious -- what is the NIC in the TDX VM? I'm most familiar with the
> > Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
> > uses dedicated send and receive buffers that are allocated and decrypted
> > when the NIC is configured. Most NIC traffic goes through those buffers
> > instead of the swiotlb, so I probably haven't seen cases where the swiotlb
> > is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck for
> > disk I/O traffic, but the data copying tends to be the gate rather than the
> > allocation and freeing of swiotlb buffers.
> >
> > >
> > > > Another approach to the contention problem would be to have a separate
> > > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> > > > computation of the hiwater mark can be dropped entirely in production
> > > > environments. Or the setting could be dynamic at runtime via a
> > > > static_call, defaulting to not computing the hiwater mark while still
> > > > allowing a sysadmin to turn it on to see workload usage of the swiotlb.
> > >
> > > That's counter-intuitive from my perspective.
> > > With global counters, the observation, which itself impacts the performance,
> > > wouldn't be able to tell the practical characterization, that's commonly lower than
> > > max performance, in turn breaks the semantics of what's it for.
> >
> > Agreed. If the global counters affect the performance and throughput
> > significantly, having an accurate hiwater mark loses some of its value.
> >
> > >
> > > Even without those global counters, if user wants to know the hiwater value,
> > > snapshotting used value(sum of each area as current behavior) periodically would
> > > produce meaningful value for workload evaluation.
> >
> > I'm a little skeptical of the value of just summing current usage. Doing so
> > tends to miss any spikes, and the spikes are the problem. If swiotlb capacity
> 
> That's current design when CONFIG_DEBUG_FS is off, and used as swiotlb
> shortage indicator for user.
> 
> Statistically that sampled value is approximate to the true value as always.

OK, yes, statistical sampling could work. The kernel queues a work item
that runs periodically to sum current usage across all areas. That sum is
compared against the previous sum to calculate a hiwater mark. An
experiment to compare the statistical hiwater mark against the current
exact calculation would be interesting. I wonder how many samples would
be needed, and hence how frequently it would need to run, to get a good
result.

> 
> > is exceeded even for a short spike, you don't just get a performance blip.
> > You get I/O failures, which at least on the disk side tends to be fatal to the
> > application doing the I/O. Maybe the networking stack recovers well enough
> > and retries, resulting in just a performance reduction. But I've always thought
> > of swiotlb exhaustion as a fairly serious problem to be avoided at all costs.
> > That's why CoCo VMs allocate so much swiotlb space, even though most of
> > it is never used for typical workloads (at least in my experience).
> 
> That's dynamic SWIOTLB is designed for.
> Only when the IO is so intensive, transient buffer/DMA pool is exhausted quickly
> before new shared memory pool is created.

I don't think dynamic SWIOTLB in its current implementation is very useful
for large CoCo VMs. Exhausting the atomic DMA pool is one problem. The
dynamic swiotlb also can grow in a max of 4 MiB increments, so if 400 MiB is
added dynamically, there's a list of 100 entries that must be searched
to find space (and that list is currently searched linearly). Furthermore,
the pre-allocated swiotlb gets 1 area per CPU, which mostly avoids contention
on the area spin locks. But a dynamically added 4 MiB pool can have a max of
16 areas because each area must be at least 256 KiB. So there's more area
spin lock contention with higher CPUs counts. And that all assumes that the
memory allocator can provide a 4 MiB contiguous area for the new pool. If
the swiotlb needs to grow after there's been memory fragmentation, an
added pool might be limited to 2 MiB or 1 MiB or smaller, with a
corresponding reduction in the area count and increase in area spin lock
contention. Overall, dynamic swiotlb in a big CoCo VM results in complex
and messy behavior with new failure modes and bottlenecks.

Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-26 16:00         ` Michael Kelley
@ 2026-06-27 23:20           ` Du, Fan
  2026-06-28  1:30             ` Michael Kelley
  0 siblings, 1 reply; 19+ messages in thread
From: Du, Fan @ 2026-06-27 23:20 UTC (permalink / raw)
  To: Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML, Du, Fan



> -----Original Message-----
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Saturday, June 27, 2026 12:00 AM
> To: Du, Fan <fan.du@intel.com>; Michael Kelley <mhklinux@outlook.com>;
> Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> robin.murphy@arm.com
> Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; LKML <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> used/hiwater tracking
> 
> From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 8:12 PM
> >
> > > -----Original Message-----
> > > From: Michael Kelley <mhklinux@outlook.com>
> > > Sent: Thursday, June 25, 2026 11:54 PM
> > >
> > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30
> AM
> > > >
> > > > > -----Original Message-----
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > > > To: Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> > > > > robin.murphy@arm.com
> > > > > Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; Du, Fan
> > > > > <fan.du@intel.com>; LKML <linux-kernel@vger.kernel.org>
> > > > > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > > > > used/hiwater tracking
> > > > >
> > >
> > > [snip]
> > >
> > > > > > +	pool = &mem->defpool;
> > > > > > +	for (i = 0; i < pool->nareas; i++)
> > > > > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > > > >
> > > > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> > > > > of calculating a separate hiwater mark for each area, and then
> summing
> > > > > those per-area hiwater marks, can produce very wrong results.
> > > > >
> > > > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8
> areas,
> > > > > each with 8 MiB of space. Suppose the workload putters along with
> mostly
> > > > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> > > > > progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16
> KiB
> > > > > each). Summing across areas produces a hiwater mark of 8 * 256 KiB =
> 2 MiB.
> > > > > But then suppose a 2 MiB I/O comes in. The hiwater mark for the area
> that
> > > > > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> > > > >  another 2 MiB I/O comes in that is handled by a different area, whose
> > > > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a
> hiwater
> > > > > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB.
> The
> > > > > old algorithm would have reported 4+ MiB, which is accurate. With
> > > > > higher CPU counts and more areas, the discrepancy can get much
> worse.
> > > > > This is a somewhat contrived example, but the problem is real enough
> > > > > to make the reported hiwater mark be unreliable.
> > > >
> > > > You are correct here, I like your way of thinking.
> > > >
> > > > > I'm sure the contention for the total hiwater mark in the current code
> is
> > > > > real, but it's in the context of a lot of other CPU work that is being
> because
> > > > > of the bounce buffering, including copying lots of data to/from the
> bounce
> > > > > buffers. Is that atomic increment operation a bottleneck even in the
> > > > > end-to-end context of doing DMA through swiotlb bounce buffers?
> > > >
> > > > Practical benchmark show case the highest IO performance for each
> TVM spec.
> > > > even if a few iperf (4)workers would cause the contention here.
> > > > My guts feelings, yes, the real workload probably hit the bottleneck
> here.
> > >
> > > Just curious -- what is the NIC in the TDX VM? I'm most familiar with the
> > > Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
> > > uses dedicated send and receive buffers that are allocated and decrypted
> > > when the NIC is configured. Most NIC traffic goes through those buffers
> > > instead of the swiotlb, so I probably haven't seen cases where the swiotlb
> > > is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck for
> > > disk I/O traffic, but the data copying tends to be the gate rather than the
> > > allocation and freeing of swiotlb buffers.
> > >
> > > >
> > > > > Another approach to the contention problem would be to have a
> separate
> > > > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> > > > > computation of the hiwater mark can be dropped entirely in
> production
> > > > > environments. Or the setting could be dynamic at runtime via a
> > > > > static_call, defaulting to not computing the hiwater mark while still
> > > > > allowing a sysadmin to turn it on to see workload usage of the swiotlb.
> > > >
> > > > That's counter-intuitive from my perspective.
> > > > With global counters, the observation, which itself impacts the
> performance,
> > > > wouldn't be able to tell the practical characterization, that's commonly
> lower than
> > > > max performance, in turn breaks the semantics of what's it for.
> > >
> > > Agreed. If the global counters affect the performance and throughput
> > > significantly, having an accurate hiwater mark loses some of its value.
> > >
> > > >
> > > > Even without those global counters, if user wants to know the hiwater
> value,
> > > > snapshotting used value(sum of each area as current behavior)
> periodically would
> > > > produce meaningful value for workload evaluation.
> > >
> > > I'm a little skeptical of the value of just summing current usage. Doing so
> > > tends to miss any spikes, and the spikes are the problem. If swiotlb
> capacity
> >
> > That's current design when CONFIG_DEBUG_FS is off, and used as swiotlb
> > shortage indicator for user.
> >
> > Statistically that sampled value is approximate to the true value as always.
> 
> OK, yes, statistical sampling could work. The kernel queues a work item
> that runs periodically to sum current usage across all areas. That sum is
> compared against the previous sum to calculate a hiwater mark. An
> experiment to compare the statistical hiwater mark against the current
> exact calculation would be interesting. I wonder how many samples would
> be needed, and hence how frequently it would need to run, to get a good
> result.

Kernel doesn't do that sampling, kernel only report the sum as user space requested.
Take a look at the flow when DEBUG_FS is off, and essentially it works as vmstat,
user set query interval.

> >
> > > is exceeded even for a short spike, you don't just get a performance blip.
> > > You get I/O failures, which at least on the disk side tends to be fatal to the
> > > application doing the I/O. Maybe the networking stack recovers well
> enough
> > > and retries, resulting in just a performance reduction. But I've always
> thought
> > > of swiotlb exhaustion as a fairly serious problem to be avoided at all costs.
> > > That's why CoCo VMs allocate so much swiotlb space, even though most
> of
> > > it is never used for typical workloads (at least in my experience).
> >
> > That's dynamic SWIOTLB is designed for.
> > Only when the IO is so intensive, transient buffer/DMA pool is exhausted
> quickly
> > before new shared memory pool is created.
> 
> I don't think dynamic SWIOTLB in its current implementation is very useful
> for large CoCo VMs. Exhausting the atomic DMA pool is one problem. The
> dynamic swiotlb also can grow in a max of 4 MiB increments, so if 400 MiB is
> added dynamically, there's a list of 100 entries that must be searched
> to find space (and that list is currently searched linearly). Furthermore,
> the pre-allocated swiotlb gets 1 area per CPU, which mostly avoids contention
> on the area spin locks. But a dynamically added 4 MiB pool can have a max of
> 16 areas because each area must be at least 256 KiB. So there's more area
> spin lock contention with higher CPUs counts. And that all assumes that the
> memory allocator can provide a 4 MiB contiguous area for the new pool. If
> the swiotlb needs to grow after there's been memory fragmentation, an
> added pool might be limited to 2 MiB or 1 MiB or smaller, with a
> corresponding reduction in the area count and increase in area spin lock
> contention. Overall, dynamic swiotlb in a big CoCo VM results in complex
> and messy behavior with new failure modes and bottlenecks.

Fragmentation indeed undermine the requested allocation size as dynamic swiotlb
grows by default 64M . In production practice, for large TVM, we see admin will reserve
majority of needed size, and let dynamic swiotlb works to add additional buffer.
Current dynamic switlb doesn't support shrinker to release unused memory back
to buddy yet.

> Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-27 23:20           ` Du, Fan
@ 2026-06-28  1:30             ` Michael Kelley
  2026-07-01  2:53               ` Du, Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kelley @ 2026-06-28  1:30 UTC (permalink / raw)
  To: Du, Fan, Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML

From: Du, Fan <fan.du@intel.com> Sent: Saturday, June 27, 2026 4:21 PM
> 
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@outlook.com>
> > Sent: Saturday, June 27, 2026 12:00 AM
> >
> > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 8:12 PM
> > >
> > > > -----Original Message-----
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > Sent: Thursday, June 25, 2026 11:54 PM
> > > >
> > > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30 AM
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > > > >
> > > >
> > > > [snip]
> > > >
> > > > > > > +	pool = &mem->defpool;
> > > > > > > +	for (i = 0; i < pool->nareas; i++)
> > > > > > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > > > > >
> > > > > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> > > > > > of calculating a separate hiwater mark for each area, and then summing
> > > > > > those per-area hiwater marks, can produce very wrong results.
> > > > > >
> > > > > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
> > > > > > each with 8 MiB of space. Suppose the workload putters along with mostly
> > > > > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> > > > > > progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
> > > > > > each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2 MiB.
> > > > > > But then suppose a 2 MiB I/O comes in. The hiwater mark for the area that
> > > > > > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> > > > > >  another 2 MiB I/O comes in that is handled by a different area, whose
> > > > > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
> > > > > > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
> > > > > > old algorithm would have reported 4+ MiB, which is accurate. With
> > > > > > higher CPU counts and more areas, the discrepancy can get much worse.
> > > > > > This is a somewhat contrived example, but the problem is real enough
> > > > > > to make the reported hiwater mark be unreliable.
> > > > >
> > > > > You are correct here, I like your way of thinking.
> > > > >
> > > > > > I'm sure the contention for the total hiwater mark in the current code is
> > > > > > real, but it's in the context of a lot of other CPU work that is being because
> > > > > > of the bounce buffering, including copying lots of data to/from the bounce
> > > > > > buffers. Is that atomic increment operation a bottleneck even in the
> > > > > > end-to-end context of doing DMA through swiotlb bounce buffers?
> > > > >
> > > > > Practical benchmark show case the highest IO performance for each TVM spec.
> > > > > even if a few iperf (4)workers would cause the contention here.
> > > > > My guts feelings, yes, the real workload probably hit the bottleneck here.
> > > >
> > > > Just curious -- what is the NIC in the TDX VM? I'm most familiar with the
> > > > Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
> > > > uses dedicated send and receive buffers that are allocated and decrypted
> > > > when the NIC is configured. Most NIC traffic goes through those buffers
> > > > instead of the swiotlb, so I probably haven't seen cases where the swiotlb
> > > > is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck for
> > > > disk I/O traffic, but the data copying tends to be the gate rather than the
> > > > allocation and freeing of swiotlb buffers.
> > > >
> > > > >
> > > > > > Another approach to the contention problem would be to have a separate
> > > > > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> > > > > > computation of the hiwater mark can be dropped entirely in production
> > > > > > environments. Or the setting could be dynamic at runtime via a
> > > > > > static_call, defaulting to not computing the hiwater mark while still
> > > > > > allowing a sysadmin to turn it on to see workload usage of the swiotlb.
> > > > >
> > > > > That's counter-intuitive from my perspective.
> > > > > With global counters, the observation, which itself impacts the performance,
> > > > > wouldn't be able to tell the practical characterization, that's commonly lower than
> > > > > max performance, in turn breaks the semantics of what's it for.
> > > >
> > > > Agreed. If the global counters affect the performance and throughput
> > > > significantly, having an accurate hiwater mark loses some of its value.
> > > >
> > > > >
> > > > > Even without those global counters, if user wants to know the hiwater value,
> > > > > snapshotting used value(sum of each area as current behavior) periodically would
> > > > > produce meaningful value for workload evaluation.
> > > >
> > > > I'm a little skeptical of the value of just summing current usage. Doing so
> > > > tends to miss any spikes, and the spikes are the problem. If swiotlb capacity
> > >
> > > That's current design when CONFIG_DEBUG_FS is off, and used as swiotlb
> > > shortage indicator for user.
> > >
> > > Statistically that sampled value is approximate to the true value as always.
> >
> > OK, yes, statistical sampling could work. The kernel queues a work item
> > that runs periodically to sum current usage across all areas. That sum is
> > compared against the previous sum to calculate a hiwater mark. An
> > experiment to compare the statistical hiwater mark against the current
> > exact calculation would be interesting. I wonder how many samples would
> > be needed, and hence how frequently it would need to run, to get a good
> > result.
> 
> Kernel doesn't do that sampling, kernel only report the sum as user space requested.
> Take a look at the flow when DEBUG_FS is off, and essentially it works as vmstat,
> user set query interval.

Agree -- the kernel doesn't do that now. I wasn't clear in my comment that
I was envisioning that the kernel *could* be enhanced to do the statistical
sampling and report the result via sysfs.

> 
> > >
> > > > is exceeded even for a short spike, you don't just get a performance blip.
> > > > You get I/O failures, which at least on the disk side tends to be fatal to the
> > > > application doing the I/O. Maybe the networking stack recovers well enough
> > > > and retries, resulting in just a performance reduction. But I've always thought
> > > > of swiotlb exhaustion as a fairly serious problem to be avoided at all costs.
> > > > That's why CoCo VMs allocate so much swiotlb space, even though most of
> > > > it is never used for typical workloads (at least in my experience).
> > >
> > > That's dynamic SWIOTLB is designed for.
> > > Only when the IO is so intensive, transient buffer/DMA pool is exhausted quickly
> > > before new shared memory pool is created.
> >
> > I don't think dynamic SWIOTLB in its current implementation is very useful
> > for large CoCo VMs. Exhausting the atomic DMA pool is one problem. The
> > dynamic swiotlb also can grow in a max of 4 MiB increments, so if 400 MiB is
> > added dynamically, there's a list of 100 entries that must be searched
> > to find space (and that list is currently searched linearly). Furthermore,
> > the pre-allocated swiotlb gets 1 area per CPU, which mostly avoids contention
> > on the area spin locks. But a dynamically added 4 MiB pool can have a max of
> > 16 areas because each area must be at least 256 KiB. So there's more area
> > spin lock contention with higher CPUs counts. And that all assumes that the
> > memory allocator can provide a 4 MiB contiguous area for the new pool. If
> > the swiotlb needs to grow after there's been memory fragmentation, an
> > added pool might be limited to 2 MiB or 1 MiB or smaller, with a
> > corresponding reduction in the area count and increase in area spin lock
> > contention. Overall, dynamic swiotlb in a big CoCo VM results in complex
> > and messy behavior with new failure modes and bottlenecks.
> 
> Fragmentation indeed undermine the requested allocation size as dynamic
> swiotlb grows by default 64M . 

Actually, I don't think the dynamic swiotlb ever grows by 64 MiB, at least
not for systems with a 4 KiB page size. Yes, swiotlb_dyn_alloc() requests
a new pool with size "default_nslabs", which might be 64 MiB. But the
memory ultimately comes from the buddy allocator, which can provide
a maximum of 4 MiB (unless MAX_ORDER has been overridden).
swiotlb_alloc_pool() tries 64 MiB, which fails, so it then tries 32 MiB,
etc., until it gets down to a size that the buddy allocator can provide.
The most it will get is 4 MiB, and perhaps less if memory is fragmented.

Michael

> In production practice, for large TVM, we see admin will reserve
> majority of needed size, and let dynamic swiotlb works to add additional buffer.
> Current dynamic switlb doesn't support shrinker to release unused memory back
> to buddy yet.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-06-28  1:30             ` Michael Kelley
@ 2026-07-01  2:53               ` Du, Fan
  2026-07-02 16:00                 ` Michael Kelley
  0 siblings, 1 reply; 19+ messages in thread
From: Du, Fan @ 2026-07-01  2:53 UTC (permalink / raw)
  To: Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML, Du, Fan



> -----Original Message-----
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Sunday, June 28, 2026 9:30 AM
> To: Du, Fan <fan.du@intel.com>; Michael Kelley <mhklinux@outlook.com>;
> Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> robin.murphy@arm.com
> Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; LKML <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> used/hiwater tracking
> 
> From: Du, Fan <fan.du@intel.com> Sent: Saturday, June 27, 2026 4:21 PM
> >
> > > -----Original Message-----
> > > From: Michael Kelley <mhklinux@outlook.com>
> > > Sent: Saturday, June 27, 2026 12:00 AM
> > >
> > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 8:12 PM
> > > >
> > > > > -----Original Message-----
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > Sent: Thursday, June 25, 2026 11:54 PM
> > > > >
> > > > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30
> AM
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > > > > >
> > > > >
> > > > > [snip]
> > > > >
> > > > > > > > +	pool = &mem->defpool;
> > > > > > > > +	for (i = 0; i < pool->nareas; i++)
> > > > > > > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > > > > > >
> > > > > > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The
> approach
> > > > > > > of calculating a separate hiwater mark for each area, and then
> summing
> > > > > > > those per-area hiwater marks, can produce very wrong results.
> > > > > > >
> > > > > > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8
> areas,
> > > > > > > each with 8 MiB of space. Suppose the workload putters along
> with mostly
> > > > > > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16
> I/Os in
> > > > > > > progress, the area hiwater mark might be 256 KiB (16 I/Os
> averaging 16 KiB
> > > > > > > each). Summing across areas produces a hiwater mark of 8 * 256
> KiB = 2 MiB.
> > > > > > > But then suppose a 2 MiB I/O comes in. The hiwater mark for the
> area that
> > > > > > > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> > > > > > >  another 2 MiB I/O comes in that is handled by a different area,
> whose
> > > > > > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a
> hiwater
> > > > > > > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB.
> The
> > > > > > > old algorithm would have reported 4+ MiB, which is accurate. With
> > > > > > > higher CPU counts and more areas, the discrepancy can get much
> worse.
> > > > > > > This is a somewhat contrived example, but the problem is real
> enough
> > > > > > > to make the reported hiwater mark be unreliable.
> > > > > >
> > > > > > You are correct here, I like your way of thinking.
> > > > > >
> > > > > > > I'm sure the contention for the total hiwater mark in the current
> code is
> > > > > > > real, but it's in the context of a lot of other CPU work that is being
> because
> > > > > > > of the bounce buffering, including copying lots of data to/from the
> bounce
> > > > > > > buffers. Is that atomic increment operation a bottleneck even in the
> > > > > > > end-to-end context of doing DMA through swiotlb bounce buffers?
> > > > > >
> > > > > > Practical benchmark show case the highest IO performance for each
> TVM spec.
> > > > > > even if a few iperf (4)workers would cause the contention here.
> > > > > > My guts feelings, yes, the real workload probably hit the bottleneck
> here.
> > > > >
> > > > > Just curious -- what is the NIC in the TDX VM? I'm most familiar with
> the
> > > > > Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
> > > > > uses dedicated send and receive buffers that are allocated and
> decrypted
> > > > > when the NIC is configured. Most NIC traffic goes through those buffers
> > > > > instead of the swiotlb, so I probably haven't seen cases where the
> swiotlb
> > > > > is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck
> for
> > > > > disk I/O traffic, but the data copying tends to be the gate rather than
> the
> > > > > allocation and freeing of swiotlb buffers.
> > > > >
> > > > > >
> > > > > > > Another approach to the contention problem would be to have a
> separate
> > > > > > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that
> the
> > > > > > > computation of the hiwater mark can be dropped entirely in
> production
> > > > > > > environments. Or the setting could be dynamic at runtime via a
> > > > > > > static_call, defaulting to not computing the hiwater mark while still
> > > > > > > allowing a sysadmin to turn it on to see workload usage of the
> swiotlb.
> > > > > >
> > > > > > That's counter-intuitive from my perspective.
> > > > > > With global counters, the observation, which itself impacts the
> performance,
> > > > > > wouldn't be able to tell the practical characterization, that's
> commonly lower than
> > > > > > max performance, in turn breaks the semantics of what's it for.
> > > > >
> > > > > Agreed. If the global counters affect the performance and throughput
> > > > > significantly, having an accurate hiwater mark loses some of its value.
> > > > >
> > > > > >
> > > > > > Even without those global counters, if user wants to know the
> hiwater value,
> > > > > > snapshotting used value(sum of each area as current behavior)
> periodically would
> > > > > > produce meaningful value for workload evaluation.
> > > > >
> > > > > I'm a little skeptical of the value of just summing current usage. Doing
> so
> > > > > tends to miss any spikes, and the spikes are the problem. If swiotlb
> capacity
> > > >
> > > > That's current design when CONFIG_DEBUG_FS is off, and used as
> swiotlb
> > > > shortage indicator for user.
> > > >
> > > > Statistically that sampled value is approximate to the true value as
> always.
> > >
> > > OK, yes, statistical sampling could work. The kernel queues a work item
> > > that runs periodically to sum current usage across all areas. That sum is
> > > compared against the previous sum to calculate a hiwater mark. An
> > > experiment to compare the statistical hiwater mark against the current
> > > exact calculation would be interesting. I wonder how many samples would
> > > be needed, and hence how frequently it would need to run, to get a good
> > > result.
> >
> > Kernel doesn't do that sampling, kernel only report the sum as user space
> requested.
> > Take a look at the flow when DEBUG_FS is off, and essentially it works as
> vmstat,
> > user set query interval.
> 
> Agree -- the kernel doesn't do that now. I wasn't clear in my comment that
> I was envisioning that the kernel *could* be enhanced to do the statistical
> sampling and report the result via sysfs.

OK, thanks for the inputs!

Based on previous discussions, to keep backward compatibility, how about keep
hiwater as it is, while guarded by dynamic knob(default off), and export used
memory from sum of per area as current design does, then user can still have a
chance to track the overall usage w/o visible overhead introduced by global atomic
counter? 


> >
> > > >
> > > > > is exceeded even for a short spike, you don't just get a performance
> blip.
> > > > > You get I/O failures, which at least on the disk side tends to be fatal to
> the
> > > > > application doing the I/O. Maybe the networking stack recovers well
> enough
> > > > > and retries, resulting in just a performance reduction. But I've always
> thought
> > > > > of swiotlb exhaustion as a fairly serious problem to be avoided at all
> costs.
> > > > > That's why CoCo VMs allocate so much swiotlb space, even though
> most of
> > > > > it is never used for typical workloads (at least in my experience).
> > > >
> > > > That's dynamic SWIOTLB is designed for.
> > > > Only when the IO is so intensive, transient buffer/DMA pool is
> exhausted quickly
> > > > before new shared memory pool is created.
> > >
> > > I don't think dynamic SWIOTLB in its current implementation is very useful
> > > for large CoCo VMs. Exhausting the atomic DMA pool is one problem. The
> > > dynamic swiotlb also can grow in a max of 4 MiB increments, so if 400 MiB
> is
> > > added dynamically, there's a list of 100 entries that must be searched
> > > to find space (and that list is currently searched linearly). Furthermore,
> > > the pre-allocated swiotlb gets 1 area per CPU, which mostly avoids
> contention
> > > on the area spin locks. But a dynamically added 4 MiB pool can have a
> max of
> > > 16 areas because each area must be at least 256 KiB. So there's more area
> > > spin lock contention with higher CPUs counts. And that all assumes that
> the
> > > memory allocator can provide a 4 MiB contiguous area for the new pool.
> If
> > > the swiotlb needs to grow after there's been memory fragmentation, an
> > > added pool might be limited to 2 MiB or 1 MiB or smaller, with a
> > > corresponding reduction in the area count and increase in area spin lock
> > > contention. Overall, dynamic swiotlb in a big CoCo VM results in complex
> > > and messy behavior with new failure modes and bottlenecks.
> >
> > Fragmentation indeed undermine the requested allocation size as dynamic
> > swiotlb grows by default 64M .
> 
> Actually, I don't think the dynamic swiotlb ever grows by 64 MiB, at least
> not for systems with a 4 KiB page size. Yes, swiotlb_dyn_alloc() requests
> a new pool with size "default_nslabs", which might be 64 MiB. But the
> memory ultimately comes from the buddy allocator, which can provide
> a maximum of 4 MiB (unless MAX_ORDER has been overridden).
> swiotlb_alloc_pool() tries 64 MiB, which fails, so it then tries 32 MiB,
> etc., until it gets down to a size that the buddy allocator can provide.
> The most it will get is 4 MiB, and perhaps less if memory is fragmented.
> 
> Michael
> 
> > In production practice, for large TVM, we see admin will reserve
> > majority of needed size, and let dynamic swiotlb works to add additional
> buffer.
> > Current dynamic switlb doesn't support shrinker to release unused memory
> back
> > to buddy yet.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-07-01  2:53               ` Du, Fan
@ 2026-07-02 16:00                 ` Michael Kelley
  2026-07-13  1:18                   ` Du, Fan
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kelley @ 2026-07-02 16:00 UTC (permalink / raw)
  To: Du, Fan, Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML

From: Du, Fan <fan.du@intel.com> Sent: Tuesday, June 30, 2026 7:54 PM
> 
> > -----Original Message-----
> > From: Michael Kelley <mhklinux@outlook.com>
> > Sent: Sunday, June 28, 2026 9:30 AM
> > To: Du, Fan <fan.du@intel.com>; Michael Kelley <mhklinux@outlook.com>;
> > Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> > robin.murphy@arm.com
> > Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; LKML <linux-
> > kernel@vger.kernel.org>
> > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > used/hiwater tracking
> >
> > From: Du, Fan <fan.du@intel.com> Sent: Saturday, June 27, 2026 4:21 PM
> > >
> > > > -----Original Message-----
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > Sent: Saturday, June 27, 2026 12:00 AM
> > > >
> > > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 8:12 PM
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > > Sent: Thursday, June 25, 2026 11:54 PM
> > > > > >
> > > > > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 12:30 AM
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > > > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > > > > > >
> > > > > >
> > > > > > [snip]
> > > > > >
> > > > > > > > > +	pool = &mem->defpool;
> > > > > > > > > +	for (i = 0; i < pool->nareas; i++)
> > > > > > > > > +		hiwater += READ_ONCE(pool->areas[i].used_hiwater);
> > > > > > > >
> > > > > > > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The approach
> > > > > > > > of calculating a separate hiwater mark for each area, and then summing
> > > > > > > > those per-area hiwater marks, can produce very wrong results.
> > > > > > > >
> > > > > > > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be 8 areas,
> > > > > > > > each with 8 MiB of space. Suppose the workload putters along with mostly
> > > > > > > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16 I/Os in
> > > > > > > > progress, the area hiwater mark might be 256 KiB (16 I/Os averaging 16 KiB
> > > > > > > > each). Summing across areas produces a hiwater mark of 8 * 256 KiB = 2 MiB.
> > > > > > > > But then suppose a 2 MiB I/O comes in. The hiwater mark for the area that
> > > > > > > > handles that I/O will grow to 2+ MiB. After the first big I/O finishes,
> > > > > > > >  another 2 MiB I/O comes in that is handled by a different area, whose
> > > > > > > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a hiwater
> > > > > > > > mark of 2+ MiB, and the total hiwater mark is reported as 16+ MiB. The
> > > > > > > > old algorithm would have reported 4+ MiB, which is accurate. With
> > > > > > > > higher CPU counts and more areas, the discrepancy can get much worse.
> > > > > > > > This is a somewhat contrived example, but the problem is real enough
> > > > > > > > to make the reported hiwater mark be unreliable.
> > > > > > >
> > > > > > > You are correct here, I like your way of thinking.
> > > > > > >
> > > > > > > > I'm sure the contention for the total hiwater mark in the current code is
> > > > > > > > real, but it's in the context of a lot of other CPU work that is being because
> > > > > > > > of the bounce buffering, including copying lots of data to/from the bounce
> > > > > > > > buffers. Is that atomic increment operation a bottleneck even in the
> > > > > > > > end-to-end context of doing DMA through swiotlb bounce buffers?
> > > > > > >
> > > > > > > Practical benchmark show case the highest IO performance for each TVM spec.
> > > > > > > even if a few iperf (4)workers would cause the contention here.
> > > > > > > My guts feelings, yes, the real workload probably hit the bottleneck here.
> > > > > >
> > > > > > Just curious -- what is the NIC in the TDX VM? I'm most familiar with the
> > > > > > Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That driver
> > > > > > uses dedicated send and receive buffers that are allocated and decrypted
> > > > > > when the NIC is configured. Most NIC traffic goes through those buffers
> > > > > > instead of the swiotlb, so I probably haven't seen cases where the swiotlb
> > > > > > is the bottleneck for NIC traffic. I do see the swiotlb as the bottleneck for
> > > > > > disk I/O traffic, but the data copying tends to be the gate rather than the
> > > > > > allocation and freeing of swiotlb buffers.
> > > > > >
> > > > > > >
> > > > > > > > Another approach to the contention problem would be to have a separate
> > > > > > > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so that the
> > > > > > > > computation of the hiwater mark can be dropped entirely in production
> > > > > > > > environments. Or the setting could be dynamic at runtime via a
> > > > > > > > static_call, defaulting to not computing the hiwater mark while still
> > > > > > > > allowing a sysadmin to turn it on to see workload usage of the swiotlb.
> > > > > > >
> > > > > > > That's counter-intuitive from my perspective.
> > > > > > > With global counters, the observation, which itself impacts the performance,
> > > > > > > wouldn't be able to tell the practical characterization, that's commonly lower than
> > > > > > > max performance, in turn breaks the semantics of what's it for.
> > > > > >
> > > > > > Agreed. If the global counters affect the performance and throughput
> > > > > > significantly, having an accurate hiwater mark loses some of its value.
> > > > > >
> > > > > > >
> > > > > > > Even without those global counters, if user wants to know the hiwater value,
> > > > > > > snapshotting used value(sum of each area as current behavior) periodically would
> > > > > > > produce meaningful value for workload evaluation.
> > > > > >
> > > > > > I'm a little skeptical of the value of just summing current usage. Doing so
> > > > > > tends to miss any spikes, and the spikes are the problem. If swiotlb capacity
> > > > >
> > > > > That's current design when CONFIG_DEBUG_FS is off, and used as swiotlb
> > > > > shortage indicator for user.
> > > > >
> > > > > Statistically that sampled value is approximate to the true value as always.
> > > >
> > > > OK, yes, statistical sampling could work. The kernel queues a work item
> > > > that runs periodically to sum current usage across all areas. That sum is
> > > > compared against the previous sum to calculate a hiwater mark. An
> > > > experiment to compare the statistical hiwater mark against the current
> > > > exact calculation would be interesting. I wonder how many samples would
> > > > be needed, and hence how frequently it would need to run, to get a good
> > > > result.
> > >
> > > Kernel doesn't do that sampling, kernel only report the sum as user space requested.
> > > Take a look at the flow when DEBUG_FS is off, and essentially it works as vmstat,
> > > user set query interval.
> >
> > Agree -- the kernel doesn't do that now. I wasn't clear in my comment that
> > I was envisioning that the kernel *could* be enhanced to do the statistical
> > sampling and report the result via sysfs.
> 
> OK, thanks for the inputs!
> 
> Based on previous discussions, to keep backward compatibility, how about keep
> hiwater as it is, while guarded by dynamic knob(default off), and export used
> memory from sum of per area as current design does, then user can still have a
> chance to track the overall usage w/o visible overhead introduced by global atomic
> counter?
> 

Yes, that works for me.  A couple of points:

1) If doing the exact hiwater calculation is dynamic and defaults to "off", that
dynamic config would replace being under #ifdef CONFIG_DEBUG_FS, right?

2) 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.

Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking
  2026-07-02 16:00                 ` Michael Kelley
@ 2026-07-13  1:18                   ` Du, Fan
  0 siblings, 0 replies; 19+ messages in thread
From: Du, Fan @ 2026-07-13  1:18 UTC (permalink / raw)
  To: Michael Kelley, Miao, Jun, m.szyprowski, robin.murphy
  Cc: iommu, chenhgs, LKML, Du, Fan



> -----Original Message-----
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Friday, July 3, 2026 12:01 AM
> To: Du, Fan <fan.du@intel.com>; Michael Kelley <mhklinux@outlook.com>;
> Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> robin.murphy@arm.com
> Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; LKML <linux-
> kernel@vger.kernel.org>
> Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> used/hiwater tracking
> 
> From: Du, Fan <fan.du@intel.com> Sent: Tuesday, June 30, 2026 7:54 PM
> >
> > > -----Original Message-----
> > > From: Michael Kelley <mhklinux@outlook.com>
> > > Sent: Sunday, June 28, 2026 9:30 AM
> > > To: Du, Fan <fan.du@intel.com>; Michael Kelley
> <mhklinux@outlook.com>;
> > > Miao, Jun <jun.miao@intel.com>; m.szyprowski@samsung.com;
> > > robin.murphy@arm.com
> > > Cc: iommu@lists.linux.dev; chenhgs@chinatelecom.cn; LKML <linux-
> > > kernel@vger.kernel.org>
> > > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > > used/hiwater tracking
> > >
> > > From: Du, Fan <fan.du@intel.com> Sent: Saturday, June 27, 2026 4:21 PM
> > > >
> > > > > -----Original Message-----
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > Sent: Saturday, June 27, 2026 12:00 AM
> > > > >
> > > > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026 8:12
> PM
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > > > Sent: Thursday, June 25, 2026 11:54 PM
> > > > > > >
> > > > > > > From: Du, Fan <fan.du@intel.com> Sent: Thursday, June 25, 2026
> 12:30 AM
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > > > > > Sent: Tuesday, June 23, 2026 10:35 AM
> > > > > > > > >
> > > > > > >
> > > > > > > [snip]
> > > > > > >
> > > > > > > > > > +	pool = &mem->defpool;
> > > > > > > > > > +	for (i = 0; i < pool->nareas; i++)
> > > > > > > > > > +		hiwater += READ_ONCE(pool-
> >areas[i].used_hiwater);
> > > > > > > > >
> > > > > > > > > Let's ignore the SWIOTLB_DYNAMIC case for simplicity. The
> approach
> > > > > > > > > of calculating a separate hiwater mark for each area, and then
> summing
> > > > > > > > > those per-area hiwater marks, can produce very wrong results.
> > > > > > > > >
> > > > > > > > > Consider a 64MiB swiotlb in a system with 8 CPUs. There will be
> 8 areas,
> > > > > > > > > each with 8 MiB of space. Suppose the workload putters along
> with mostly
> > > > > > > > > smallish I/Os, say between 4 KiB and 32 KiB. If each area has 16
> I/Os in
> > > > > > > > > progress, the area hiwater mark might be 256 KiB (16 I/Os
> averaging 16 KiB
> > > > > > > > > each). Summing across areas produces a hiwater mark of 8 *
> 256 KiB = 2 MiB.
> > > > > > > > > But then suppose a 2 MiB I/O comes in. The hiwater mark for
> the area that
> > > > > > > > > handles that I/O will grow to 2+ MiB. After the first big I/O
> finishes,
> > > > > > > > >  another 2 MiB I/O comes in that is handled by a different area,
> whose
> > > > > > > > > hiwater mark also goes to 2+ MiB. Pretty soon all 8 areas have a
> hiwater
> > > > > > > > > mark of 2+ MiB, and the total hiwater mark is reported as 16+
> MiB. The
> > > > > > > > > old algorithm would have reported 4+ MiB, which is accurate.
> With
> > > > > > > > > higher CPU counts and more areas, the discrepancy can get
> much worse.
> > > > > > > > > This is a somewhat contrived example, but the problem is real
> enough
> > > > > > > > > to make the reported hiwater mark be unreliable.
> > > > > > > >
> > > > > > > > You are correct here, I like your way of thinking.
> > > > > > > >
> > > > > > > > > I'm sure the contention for the total hiwater mark in the current
> code is
> > > > > > > > > real, but it's in the context of a lot of other CPU work that is
> being because
> > > > > > > > > of the bounce buffering, including copying lots of data to/from
> the bounce
> > > > > > > > > buffers. Is that atomic increment operation a bottleneck even in
> the
> > > > > > > > > end-to-end context of doing DMA through swiotlb bounce
> buffers?
> > > > > > > >
> > > > > > > > Practical benchmark show case the highest IO performance for
> each TVM spec.
> > > > > > > > even if a few iperf (4)workers would cause the contention here.
> > > > > > > > My guts feelings, yes, the real workload probably hit the
> bottleneck here.
> > > > > > >
> > > > > > > Just curious -- what is the NIC in the TDX VM? I'm most familiar
> with the
> > > > > > > Hyper-V case, where the NIC is the Hyper-V synthetic NIC. That
> driver
> > > > > > > uses dedicated send and receive buffers that are allocated and
> decrypted
> > > > > > > when the NIC is configured. Most NIC traffic goes through those
> buffers
> > > > > > > instead of the swiotlb, so I probably haven't seen cases where the
> swiotlb
> > > > > > > is the bottleneck for NIC traffic. I do see the swiotlb as the
> bottleneck for
> > > > > > > disk I/O traffic, but the data copying tends to be the gate rather
> than the
> > > > > > > allocation and freeing of swiotlb buffers.
> > > > > > >
> > > > > > > >
> > > > > > > > > Another approach to the contention problem would be to have
> a separate
> > > > > > > > > CONFIG option that is narrower than CONFIG_DEBUG_FS, so
> that the
> > > > > > > > > computation of the hiwater mark can be dropped entirely in
> production
> > > > > > > > > environments. Or the setting could be dynamic at runtime via a
> > > > > > > > > static_call, defaulting to not computing the hiwater mark while
> still
> > > > > > > > > allowing a sysadmin to turn it on to see workload usage of the
> swiotlb.
> > > > > > > >
> > > > > > > > That's counter-intuitive from my perspective.
> > > > > > > > With global counters, the observation, which itself impacts the
> performance,
> > > > > > > > wouldn't be able to tell the practical characterization, that's
> commonly lower than
> > > > > > > > max performance, in turn breaks the semantics of what's it for.
> > > > > > >
> > > > > > > Agreed. If the global counters affect the performance and
> throughput
> > > > > > > significantly, having an accurate hiwater mark loses some of its
> value.
> > > > > > >
> > > > > > > >
> > > > > > > > Even without those global counters, if user wants to know the
> hiwater value,
> > > > > > > > snapshotting used value(sum of each area as current behavior)
> periodically would
> > > > > > > > produce meaningful value for workload evaluation.
> > > > > > >
> > > > > > > I'm a little skeptical of the value of just summing current usage.
> Doing so
> > > > > > > tends to miss any spikes, and the spikes are the problem. If swiotlb
> capacity
> > > > > >
> > > > > > That's current design when CONFIG_DEBUG_FS is off, and used as
> swiotlb
> > > > > > shortage indicator for user.
> > > > > >
> > > > > > Statistically that sampled value is approximate to the true value as
> always.
> > > > >
> > > > > OK, yes, statistical sampling could work. The kernel queues a work
> item
> > > > > that runs periodically to sum current usage across all areas. That sum
> is
> > > > > compared against the previous sum to calculate a hiwater mark. An
> > > > > experiment to compare the statistical hiwater mark against the current
> > > > > exact calculation would be interesting. I wonder how many samples
> would
> > > > > be needed, and hence how frequently it would need to run, to get a
> good
> > > > > result.
> > > >
> > > > Kernel doesn't do that sampling, kernel only report the sum as user
> space requested.
> > > > Take a look at the flow when DEBUG_FS is off, and essentially it works as
> vmstat,
> > > > user set query interval.
> > >
> > > Agree -- the kernel doesn't do that now. I wasn't clear in my comment that
> > > I was envisioning that the kernel *could* be enhanced to do the statistical
> > > sampling and report the result via sysfs.
> >
> > OK, thanks for the inputs!
> >
> > Based on previous discussions, to keep backward compatibility, how about
> keep
> > hiwater as it is, while guarded by dynamic knob(default off), and export
> used
> > memory from sum of per area as current design does, then user can still
> have a
> > chance to track the overall usage w/o visible overhead introduced by global
> atomic
> > counter?
> >
> 
> Yes, that works for me.  A couple of points:
> 
> 1) If doing the exact hiwater calculation is dynamic and defaults to "off", that
> dynamic config would replace being under #ifdef CONFIG_DEBUG_FS, right?
> 
> 2) 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.

That's what in my mind too, will post the update version, let's have a review then.

> Michael

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in
       [not found] <20260622122114.2563254-1-jun.miao@intel.com>
  2026-06-23  2:35 ` [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking Michael Kelley
@ 2026-07-14  3:25 ` Frank Chen
  2026-07-20 18:30   ` Michael Kelley
  2026-08-05  2:29 ` [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS Frank Chen
  2026-08-12  7:04 ` [PATCH v4] " Frank Chen
  3 siblings, 1 reply; 19+ messages in thread
From: Frank Chen @ 2026-07-14  3:25 UTC (permalink / raw)
  To: mhklinux, hch, m.szyprowski, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

From: chenhuguanshen <chenhgs@chinatelecom.cn>

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.

- 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@intel.com>
Signed-off-by: Jun Miao <jun.miao@intel.com>
Co-developed-by: Fan Du <fan.du@intel.com>
Signed-off-by: Fan Du <fan.du@intel.com>
Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>
---
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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in
  2026-07-14  3:25 ` [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in Frank Chen
@ 2026-07-20 18:30   ` Michael Kelley
  2026-08-04 10:55     ` Marek Szyprowski
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kelley @ 2026-07-20 18:30 UTC (permalink / raw)
  To: Frank Chen, hch, m.szyprowski, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

From: Frank Chen <frankchen158@126.com> Sent: Monday, July 13, 2026 8:25 PM
> 
> From: chenhuguanshen <chenhgs@chinatelecom.cn>
> 
> 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@intel.com>
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> Co-developed-by: Fan Du <fan.du@intel.com>
> Signed-off-by: Fan Du <fan.du@intel.com>
> Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
> Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>
> ---
> 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
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in
  2026-07-20 18:30   ` Michael Kelley
@ 2026-08-04 10:55     ` Marek Szyprowski
  0 siblings, 0 replies; 19+ messages in thread
From: Marek Szyprowski @ 2026-08-04 10:55 UTC (permalink / raw)
  To: Michael Kelley, Frank Chen, hch, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

On 20.07.2026 20:30, Michael Kelley wrote:
> From: Frank Chen <frankchen158@126.com> Sent: Monday, July 13, 2026 8:25 PM
>> From: chenhuguanshen <chenhgs@chinatelecom.cn>
>>
>> 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.

Indeed keeping the accurate measurements is important to track potential

spikes of hiwater level. However imho it would be enough to use boot time

enablement.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS
       [not found] <20260622122114.2563254-1-jun.miao@intel.com>
  2026-06-23  2:35 ` [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking Michael Kelley
  2026-07-14  3:25 ` [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in Frank Chen
@ 2026-08-05  2:29 ` Frank Chen
  2026-08-09 15:50   ` Michael Kelley
  2026-08-12  7:04 ` [PATCH v4] " Frank Chen
  3 siblings, 1 reply; 19+ messages in thread
From: Frank Chen @ 2026-08-05  2:29 UTC (permalink / raw)
  To: mhklinux, hch, m.szyprowski, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

From: chenhuguanshen <chenhgs@chinatelecom.cn>

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
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:

- 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.

Suggested-by: Fan Du <fan.du@intel.com>
Signed-off-by: Jun Miao <jun.miao@intel.com>
Co-developed-by: Fan Du <fan.du@intel.com>
Signed-off-by: Fan Du <fan.du@intel.com>
Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>

---
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
 
 	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
+ * 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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS
  2026-08-05  2:29 ` [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS Frank Chen
@ 2026-08-09 15:50   ` Michael Kelley
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Kelley @ 2026-08-09 15:50 UTC (permalink / raw)
  To: Frank Chen, hch, m.szyprowski, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

From: Frank Chen <frankchen158@126.com> Sent: Tuesday, August 4, 2026 7:30 PM
> 
> From: chenhuguanshen <chenhgs@chinatelecom.cn>
> 
> 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@intel.com>
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> Co-developed-by: Fan Du <fan.du@intel.com>
> Signed-off-by: Fan Du <fan.du@intel.com>
> Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
> Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>
> 
> ---
> 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@outlook.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v4] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS
       [not found] <20260622122114.2563254-1-jun.miao@intel.com>
                   ` (2 preceding siblings ...)
  2026-08-05  2:29 ` [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS Frank Chen
@ 2026-08-12  7:04 ` Frank Chen
  2026-08-12 15:01   ` Michael Kelley
  3 siblings, 1 reply; 19+ messages in thread
From: Frank Chen @ 2026-08-12  7:04 UTC (permalink / raw)
  To: mhklinux, hch, m.szyprowski, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

From: chenhuguanshen <chenhgs@chinatelecom.cn>

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. 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.

Note that when CONFIG_DEBUG_FS is enabled but hiwater tracking is disabled,
the "io_tlb_used" metric reports an approximate value rather than an
instantaneously exact one.

Suggested-by: Fan Du <fan.du@intel.com>
Signed-off-by: Jun Miao <jun.miao@intel.com>
Co-developed-by: Fan Du <fan.du@intel.com>
Signed-off-by: Fan Du <fan.du@intel.com>
Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>

---
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.

v3 -> v4:
 - Change commit message and documentation.
---
 .../admin-guide/kernel-parameters.txt         |   4 +-
 include/linux/swiotlb.h                       |   8 +-
 kernel/dma/swiotlb.c                          | 153 +++++++++++-------
 3 files changed, 101 insertions(+), 64 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b5493a7f8f22..eb3e8db2c178 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,8 @@ 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.
+			                 Only available when CONFIG_DEBUG_FS is set.
 
 	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..e0bc7ca4a7ad 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 an approximation, because there is no locking of
+ * 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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v4] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS
  2026-08-12  7:04 ` [PATCH v4] " Frank Chen
@ 2026-08-12 15:01   ` Michael Kelley
  2026-08-13  6:24     ` Marek Szyprowski
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Kelley @ 2026-08-12 15:01 UTC (permalink / raw)
  To: Frank Chen, hch, m.szyprowski, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

From: Frank Chen <frankchen158@126.com> Sent: Wednesday, August 12, 2026 12:05 AM
> 
> From: chenhuguanshen <chenhgs@chinatelecom.cn>
> 
> 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. 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.
> 
> Note that when CONFIG_DEBUG_FS is enabled but hiwater tracking is disabled,
> the "io_tlb_used" metric reports an approximate value rather than an
> instantaneously exact one.
> 
> Suggested-by: Fan Du <fan.du@intel.com>
> Signed-off-by: Jun Miao <jun.miao@intel.com>
> Co-developed-by: Fan Du <fan.du@intel.com>
> Signed-off-by: Fan Du <fan.du@intel.com>
> Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
> Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>
> 
> ---
> 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.
> 
> v3 -> v4:
>  - Change commit message and documentation.

LGTM.

Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Tested-by: Michael Kelley <mhklinux@outlook.com> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS
  2026-08-12 15:01   ` Michael Kelley
@ 2026-08-13  6:24     ` Marek Szyprowski
  0 siblings, 0 replies; 19+ messages in thread
From: Marek Szyprowski @ 2026-08-13  6:24 UTC (permalink / raw)
  To: Michael Kelley, Frank Chen, hch, robin.murphy
  Cc: linux-kernel, iommu, fan.du, jun.miao, chenhgs

On 12.08.2026 17:01, Michael Kelley wrote:
> From: Frank Chen <frankchen158@126.com> Sent: Wednesday, August 12, 2026 12:05 AM
>> From: chenhuguanshen <chenhgs@chinatelecom.cn>
>>
>> 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. 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.
>>
>> Note that when CONFIG_DEBUG_FS is enabled but hiwater tracking is disabled,
>> the "io_tlb_used" metric reports an approximate value rather than an
>> instantaneously exact one.
>>
>> Suggested-by: Fan Du <fan.du@intel.com>
>> Signed-off-by: Jun Miao <jun.miao@intel.com>
>> Co-developed-by: Fan Du <fan.du@intel.com>
>> Signed-off-by: Fan Du <fan.du@intel.com>
>> Tested-by: chenhuguanshen <chenhgs@chinatelecom.cn>
>> Signed-off-by: chenhuguanshen <chenhgs@chinatelecom.cn>
>>
>> ---
>> 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.
>>
>> v3 -> v4:
>>  - Change commit message and documentation.
> LGTM.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com> 

Applied to dma-mapping-for-next, thanks!

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2026-08-13  6:24 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20260622122114.2563254-1-jun.miao@intel.com>
2026-06-23  2:35 ` [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking Michael Kelley
2026-06-23 12:22   ` Robin Murphy
2026-06-25  7:30   ` Du, Fan
2026-06-25 15:53     ` Michael Kelley
2026-06-26  3:12       ` Du, Fan
2026-06-26 16:00         ` Michael Kelley
2026-06-27 23:20           ` Du, Fan
2026-06-28  1:30             ` Michael Kelley
2026-07-01  2:53               ` Du, Fan
2026-07-02 16:00                 ` Michael Kelley
2026-07-13  1:18                   ` Du, Fan
2026-07-14  3:25 ` [PATCH] dma/swiotlb: make high watermark tracking boot-time opt-in Frank Chen
2026-07-20 18:30   ` Michael Kelley
2026-08-04 10:55     ` Marek Szyprowski
2026-08-05  2:29 ` [PATCH v3] dma/swiotlb: decouple high watermark tracking from CONFIG_DEBUG_FS Frank Chen
2026-08-09 15:50   ` Michael Kelley
2026-08-12  7:04 ` [PATCH v4] " Frank Chen
2026-08-12 15:01   ` Michael Kelley
2026-08-13  6:24     ` Marek Szyprowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®