Re: [PATCH v2 04/10] iommu/arm-smmu-v3: Allocate IOTLB cache tag if no id to reuse

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Jan 27, 2026 at 12:29:08PM -0400, Jason Gunthorpe wrote:
> On Mon, Jan 26, 2026 at 02:23:36PM -0800, Nicolin Chen wrote:
> > On Mon, Jan 26, 2026 at 05:06:40PM -0400, Jason Gunthorpe wrote:
> > > On Wed, Jan 21, 2026 at 05:24:22PM -0800, Nicolin Chen wrote:
> > > > @@ -3220,6 +3241,9 @@ arm_smmu_master_build_inv(struct arm_smmu_master *master,
> > > >  		break;
> > > >  	}
> > > >  
> > > > +	/* Set a default users counter */
> > > > +	refcount_set(&cur->users, 1);
> > > 
> > > I think abusing users here is a little too hard to read..
> > > 
> > > Can we just keep track in the state somehow with a flag?
> > >
> > > Or maybe union in a "bool needs_free" that is for the on-stack version
> > > of this structure?
> > 
> > This doesn't only apply to the case when a tag is newly allocated
> > during the attach, but it can be used when an old tag has users=0
> > during a detach, right?
> 
> Oh, IDK, I didn't look so closely..
> 
> I though this was just about cleaning up the tag during attach, the
> detach flow worked differently and did use users = 0?

Yes. Detach doesn't allocate a new tag, as it gets the old tag
and decrease its users counter in arm_smmu_invs_unref():

+ * This function will not fail. Any entry with users=0 will be marked as trash,
+ * and caller will be notified about the trashed entry via @to_unref by setting
+ * a users=0.

With that being said, we may still add a "bool is_trash", if we
want. I just don't feel very necessary since we check "is_trash"
throughout the driver via the users counter already..

FWIW, I kept three refcount_set calls in the latest base series
v10, copied from your earlier suggestion. I moved that to this
arm_smmu_master_build_inv() as I felt a bit redundant. Yet, it
is probably better for readability reason:

+	arm_smmu_invs_for_each_cmp(invs, i, to_unref, j, cmp) {
+		if (cmp < 0) {
+			/* not found in to_unref, leave alone */
+			WRITE_ONCE(to_unref->inv[j].users, 1);
+			num_invs = i + 1;
+		} else if (cmp == 0) {
+			int users = READ_ONCE(invs->inv[i].users) - 1;
+
+			if (WARN_ON(users < 0))
+				continue;
+
+			/* same item */
+			WRITE_ONCE(invs->inv[i].users, users);
+			if (users) {
+				WRITE_ONCE(to_unref->inv[j].users, 1);
+				num_invs = i + 1;
+				continue;
+			}
+
+			/* Notify the caller about the trash entry */
+			WRITE_ONCE(to_unref->inv[j].users, 0);
+			invs->num_trashes++;
+		} else {
+			/* item in to_unref is not in invs or already a trash */
+			WARN_ON(true);
+		}
+	}

Nicolin