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

Nicolin Chen
2026-01-27T16:52:53Z

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