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

[Dave Hansen]

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

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?