Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()

[Edgecombe, Rick P]

On Wed, 2026-07-08 at 12:01 -0700, Dave Hansen wrote:
> On 5/25/26 19:35, Rick Edgecombe wrote:
> > +	scoped_guard(spinlock, &pamt_lock) {
> > +		/*
> > +		 * If the pamt page is already added (i.e. refcount >= 1),
> > +		 * then just increment the refcount.
> > +		 */
> > +		if (atomic_read(pamt_refcount)) {
> > +			atomic_inc(pamt_refcount);
> > +			goto out_free;
> > +		}
> 
> I think this pattern is a good fit for atomic_inc_not_zero().

Sure. The intention was to make this one as obviously correct as possible and I
thought to leverage the most familiar concepts of read/inc/if.

> 
> I think it's also fair to say that using an atomic_t here is unnecessary
> because of the spinlock protecting all accesses. But I also understand
> that it's here because you want to optimize this path to avoid the
> spinlock in the future.
> 
> This is not a hot enough path to care about an atomic_t vs. int. In
> practice, the performance is going to be bottlenecked on the big
> spinlock, *not* the atomic_t overhead.
> 
> I'd just add a changelog blurb to handwave it away for the moment:
> 
> 	The pamt_refcount[]s are atomic_t's. They do not strictly need
> 	to be because all access is protected by pamt_lock. The overhead
> 	of an atomic_t in this situation is minuscule compared to the
> 	global lock. Leave the (unnecessary) atomic_t in place to enable
> 	future optimization with minimal churn.
> 
> Does that work for everybody?

Works for me, thanks.