lkml.org 
[lkml]   [2022]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/5] jump_label: Prevent key->enabled int overflow
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!

\
 
 \ /
  Last update: 2022-11-23 11:06    [W:0.059 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site