| Message ID | 4F99C980.3030801@parallels.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello, On Thu, Apr 26, 2012 at 3:17 PM, Glauber Costa <glommer@parallels.com> wrote: > >> No, what I mean is that why can't you do about the same mutexed >> activated inside static_key API function instead of requiring every >> user to worry about the function returning asynchronously. >> ie. synchronize inside static_key API instead of in the callers. >> > > Like this? Yeah, something like that. If keeping the inc operation a single atomic op is important for performance or whatever reasons, you can play some trick with large negative bias value while activation is going on and use atomic_add_return() to determine both whether it's the first incrementer and someone else is in the process of activating. Thanks.
On 04/26/2012 07:22 PM, Tejun Heo wrote: > Hello, > > On Thu, Apr 26, 2012 at 3:17 PM, Glauber Costa<glommer@parallels.com> wrote: >> >>> No, what I mean is that why can't you do about the same mutexed >>> activated inside static_key API function instead of requiring every >>> user to worry about the function returning asynchronously. >>> ie. synchronize inside static_key API instead of in the callers. >>> >> >> Like this? > > Yeah, something like that. If keeping the inc operation a single > atomic op is important for performance or whatever reasons, you can > play some trick with large negative bias value while activation is > going on and use atomic_add_return() to determine both whether it's > the first incrementer and someone else is in the process of > activating. > > Thanks. > We need a broader audience for this, but if I understand the interface right, those functions should not be called in fast paths at all (contrary to the static_branch tests) The static_branch tests can be called from irq context, so we can't just get rid of the atomic op and use the mutex everywhere, we'd have to live with both. I will repost this series, with some more people in the CC list. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 26, 2012 at 3:28 PM, Glauber Costa <glommer@parallels.com> wrote: > We need a broader audience for this, but if I understand the interface > right, those functions should not be called in fast paths at all (contrary > to the static_branch tests) > > The static_branch tests can be called from irq context, so we can't just get > rid of the atomic op and use the mutex everywhere, we'd have > to live with both. > > I will repost this series, with some more people in the CC list. Great, thanks!
diff --git a/kernel/jump_label.c b/kernel/jump_label.c index 4304919..f7cdc18 100644 --- a/kernel/jump_label.c +++ b/kernel/jump_label.c @@ -57,10 +57,11 @@ static void jump_label_update(struct static_key *key, int enable); void static_key_slow_inc(struct static_key *key) { + jump_label_lock(); + if (atomic_inc_not_zero(&key->enabled)) - return; + goto out; - jump_label_lock(); if (atomic_read(&key->enabled) == 0) { if (!jump_label_get_branch_default(key)) jump_label_update(key, JUMP_LABEL_ENABLE); @@ -68,6 +69,7 @@ void static_key_slow_inc(struct static_key *key) jump_label_update(key, JUMP_LABEL_DISABLE); } atomic_inc(&key->enabled); +out: jump_label_unlock(); } EXPORT_SYMBOL_GPL(static_key_slow_inc); @@ -75,10 +77,11 @@ EXPORT_SYMBOL_GPL(static_key_slow_inc); static void __static_key_slow_dec(struct static_key *key, unsigned long rate_limit, struct delayed_work *work) { - if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) { + jump_label_lock(); + if (atomic_dec_and_test(&key->enabled)) { WARN(atomic_read(&key->enabled) < 0, "jump label: negative count!\n"); - return; + goto out; } if (rate_limit) { @@ -90,6 +93,8 @@ static void __static_key_slow_dec(struct static_key *key, else jump_label_update(key, JUMP_LABEL_ENABLE); } + +out: jump_label_unlock(); }