Re: [PATCH v6 06/11] x86/virt/tdx: Optimize tdx_pamt_get/put()

[Edgecombe, Rick P]

On Fri, 2026-06-05 at 09:23 -0700, Dave Hansen wrote:
> On 6/5/26 04:42, Kiryl Shutsemau wrote:
> > > > I don't see a reason why we can't keep the scoped_guard() on get side.
> > > One additional reason to drop scoped_guard() is that it mixes cleanup helpers
> > > with goto, which is discouraged. See [*]
> > > 
> > >   :Lastly, given that the benefit of cleanup helpers is removal of “goto”, and
> > >   :that the “goto” statement can jump between scopes, the expectation is that
> > >   :usage of “goto” and cleanup helpers is never mixed in the same function.
> > Fair enough.
> > 
> > But it can also be address if we free the PAMT page array with the guard
> > too :P
> 
> How important is this patch? I see "Optimize" but I read "Optional".
> 
> If we're arguing about it, maybe we should just kick it out and focus on
> the more important bits.

I had done some testing previously to see if the refcount solution avoided
contention enough:
   V2 of the series includes a global lock to be used around actual 
   installation/removal of the DPAMT backing, combined with opportunistic 
   checking outside the lock to avoid taking it most of the time. In testing, 
   booting 10 16GB TDs, the lock only hit contention 1136 times, with 4ms 
   waiting. This is very small for an operation that took 60s of wall time. 
   So despite being an (ugly) global lock, the actual impact was small. It 
   will probably further be reduced in the case of huge pages, where most of 
   the time 4KB DPAMT installation will not be necessary.
   https://lore.kernel.org/all/20250918232224.2202592-1-rick.p.edgecombe@xxxxxxxxx/
   
But I did not test without this global lock avoiding optimization. I can look
into it.

The other way to drop this patch I was considering was to limit the situations
when dynamic PAMT gets enabled somehow, like early LASS support. A boot time
param would be the simplest. Or define a policy based on the number of keyids
with the reasoning that if you are only able to run one TD, you will at least
not contend with other TDs. Then we could revisit the optimization after huge
pages is settled.

For the subject of the arguing though, my vote would be to drop the scope guard
stuff and stick with the old pattern from the beginning.