On Tue, Nov 22, 2022 at 06:55:30PM +0000, Dmitry Safonov wrote:

+/***
+ * static_key_fast_inc_not_negative - adds a user for a static key
+ * @key: static key that must be already enabled
+ *
+ * The caller must make sure that the static key can't get disabled while
+ * in this function. It doesn't patch jump labels, only adds a user to
+ * an already enabled static key.
+ *
+ * Returns true if the increment was done.
+ */

I don't normally do kerneldoc style comments, and this is the first in
the whole file. The moment I get a docs person complaining about some
markup issue I just take the ** off.

+static bool static_key_fast_inc_not_negative(struct static_key *key)
 {
+	int v;
+
 	STATIC_KEY_CHECK_USE(key);
+	/*
+	 * Negative key->enabled has a special meaning: it sends
+	 * static_key_slow_inc() down the slow path, and it is non-zero
+	 * so it counts as "enabled" in jump_label_update().  Note that
+	 * atomic_inc_unless_negative() checks >= 0, so roll our own.
+	 */
+	v = atomic_read(&key->enabled);
+	do {
+		if (v <= 0 || (v + 1) < 0)
+			return false;
+	} while (!likely(atomic_try_cmpxchg(&key->enabled, &v, v + 1)));
+
+	return true;
+}

( vexing how this function and the JUMP_LABEL=n static_key_slow_inc() are
  only a single character different )

So while strictly accurate, I dislike this name (and I see I was not
quick enough responding to your earlier suggestion :/). The whole
negative thing is an implementation detail that should not spread
outside of jump_label.c.

Since you did not like the canonical _inc_not_zero(), how about
inc_not_disabled() ?

Also, perhaps expose this function in this patch, instead of hiding that
in patch 3?



Otherwise, things look good.

Thanks!