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:

quoted

On Mon, Jan 26, 2026 at 05:06:40PM -0400, Jason Gunthorpe wrote:

quoted

On Wed, Jan 21, 2026 at 05:24:22PM -0800, Nicolin Chen wrote:

quoted

@@ -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