On Thu, Mar 10, 2022 at 05:25:47PM +0200, Vladimir Oltean wrote:

quoted

quoted

quoted

+	err = mv88e6xxx_sid_get(chip, attr->orig_dev, vattr->msti, &new_sid);
+	if (err)
+		goto unlock;
+
+	if (vlan.sid) {
+		err = mv88e6xxx_sid_put(chip, vlan.sid);
+		if (err)
+			goto unlock;
+	}
+
+	vlan.sid = new_sid;
+	err = mv88e6xxx_vtu_loadpurge(chip, &vlan);

Maybe you could move mv88e6xxx_sid_put() after this succeeds?

Yep. Also made sure to avoid needless updates of the VTU entry if it
already belonged to the correct SID.

I realize I gave you conflicting advice here, first with inverting the
refcount_inc() with the refcount_dec(), then with having fast handling
of noop-changes to vlan.sid. I hope you're able to make some sense out
of that and avoid the obvious issue with the refcount temporarily
dropping to zero before going back to 1, which makes the sanity checker
complain.

Oh wow... I didn't look at the code again, and commented based on a
false memory. Disregard, sorry. You aren't reversing sid_get with sid_put,
nor did I suggest that. There's a lot that happened just in my head,
apparently.