RE: [PATCH] swiotlb: eliminate per-map atomic contention on used/hiwater tracking

[Du, Fan]

> -----Original Message-----
> From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> Sent: Friday, July 3, 2026 12:01 AM
> To: Du, Fan <fan.du@xxxxxxxxx>; Michael Kelley <mhklinux@xxxxxxxxxxx>;
> Miao, Jun <jun.miao@xxxxxxxxx>; m.szyprowski@xxxxxxxxxxx;
> robin.murphy@xxxxxxx
> Cc: iommu@xxxxxxxxxxxxxxx; chenhgs@xxxxxxxxxxxxxxx; LKML <linux-
> kernel@xxxxxxxxxxxxxxx>
> Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> used/hiwater tracking
> 
> From: Du, Fan <fan.du@xxxxxxxxx> Sent: Tuesday, June 30, 2026 7:54 PM
> >
> > > -----Original Message-----
> > > From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> > > Sent: Sunday, June 28, 2026 9:30 AM
> > > To: Du, Fan <fan.du@xxxxxxxxx>; Michael Kelley
> <mhklinux@xxxxxxxxxxx>;
> > > Miao, Jun <jun.miao@xxxxxxxxx>; m.szyprowski@xxxxxxxxxxx;
> > > robin.murphy@xxxxxxx
> > > Cc: iommu@xxxxxxxxxxxxxxx; chenhgs@xxxxxxxxxxxxxxx; LKML <linux-
> > > kernel@xxxxxxxxxxxxxxx>
> > > Subject: RE: [PATCH] swiotlb: eliminate per-map atomic contention on
> > > used/hiwater tracking
> > >
> > > From: Du, Fan <fan.du@xxxxxxxxx> Sent: Saturday, June 27, 2026 4:21 PM
> > > >
> > > > > -----Original Message-----
> > > > > From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> > > > > Sent: Saturday, June 27, 2026 12:00 AM
> > > > >
> > > > > From: Du, Fan <fan.du@xxxxxxxxx> Sent: Thursday, June 25, 2026 8:12
> PM
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> > > > > > > Sent: Thursday, June 25, 2026 11:54 PM
> > > > > > >
> > > > > > > From: Du, Fan <fan.du@xxxxxxxxx> Sent: Thursday, June 25, 2026
> 12:30 AM
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: Michael Kelley <mhklinux@xxxxxxxxxxx>
> > > > > > > > > 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