| Message ID | 54956fd06ab4a9938421f345ecf2e1518161cb38.1616533074.git.gladkov.alexey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Count rlimits in each user namespace | expand |
Alexey Gladkov <gladkov.alexey@gmail.com> writes: > The current implementation of the ucounts reference counter requires the > use of spin_lock. We're going to use get_ucounts() in more performance > critical areas like a handling of RLIMIT_SIGPENDING. > > Now we need to use spin_lock only if we want to change the hashtable. > > v9: > * Use a negative value to check that the ucounts->count is close to > overflow. Overall this looks good, one small issue below. Eric > diff --git a/kernel/ucount.c b/kernel/ucount.c > index 50cc1dfb7d28..7bac19bb3f1e 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -11,7 +11,7 @@ > struct ucounts init_ucounts = { > .ns = &init_user_ns, > .uid = GLOBAL_ROOT_UID, > - .count = 1, > + .count = ATOMIC_INIT(1), > }; > > #define UCOUNTS_HASHTABLE_BITS 10 > @@ -139,6 +139,15 @@ static void hlist_add_ucounts(struct ucounts *ucounts) > spin_unlock_irq(&ucounts_lock); > } > > +struct ucounts *get_ucounts(struct ucounts *ucounts) > +{ > + if (ucounts && atomic_add_negative(1, &ucounts->count)) { > + atomic_dec(&ucounts->count); ^^^^^^^^^^^^^^^^^^^^^^^^^^^ To handle the pathological case of all of the other uses calling put_ucounts after the value goes negative, the above should be put_ucounts intead of atomic_dec. > + ucounts = NULL; > + } > + return ucounts; > +} > + > struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid) > { > struct hlist_head *hashent = ucounts_hashentry(ns, uid); > @@ -155,7 +164,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid) > > new->ns = ns; > new->uid = uid; > - new->count = 0; > + atomic_set(&new->count, 1); > > spin_lock_irq(&ucounts_lock); > ucounts = find_ucounts(ns, uid, hashent); > @@ -163,33 +172,12 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid) > kfree(new); > } else { > hlist_add_head(&new->node, hashent); > - ucounts = new; > + spin_unlock_irq(&ucounts_lock); > + return new; > } > } > - if (ucounts->count == INT_MAX) > - ucounts = NULL; > - else > - ucounts->count += 1; > spin_unlock_irq(&ucounts_lock); > - return ucounts; > -} > - > -struct ucounts *get_ucounts(struct ucounts *ucounts) > -{ > - unsigned long flags; > - > - if (!ucounts) > - return NULL; > - > - spin_lock_irqsave(&ucounts_lock, flags); > - if (ucounts->count == INT_MAX) { > - WARN_ONCE(1, "ucounts: counter has reached its maximum value"); > - ucounts = NULL; > - } else { > - ucounts->count += 1; > - } > - spin_unlock_irqrestore(&ucounts_lock, flags); > - > + ucounts = get_ucounts(ucounts); > return ucounts; > } > > @@ -197,15 +185,12 @@ void put_ucounts(struct ucounts *ucounts) > { > unsigned long flags; > > - spin_lock_irqsave(&ucounts_lock, flags); > - ucounts->count -= 1; > - if (!ucounts->count) > + if (atomic_dec_and_test(&ucounts->count)) { > + spin_lock_irqsave(&ucounts_lock, flags); > hlist_del_init(&ucounts->node); > - else > - ucounts = NULL; > - spin_unlock_irqrestore(&ucounts_lock, flags); > - > - kfree(ucounts); > + spin_unlock_irqrestore(&ucounts_lock, flags); > + kfree(ucounts); > + } > } > > static inline bool atomic_long_inc_below(atomic_long_t *v, int u)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h index f71b5a4a3e74..d84cc2c0b443 100644 --- a/include/linux/user_namespace.h +++ b/include/linux/user_namespace.h @@ -92,7 +92,7 @@ struct ucounts { struct hlist_node node; struct user_namespace *ns; kuid_t uid; - int count; + atomic_t count; atomic_long_t ucount[UCOUNT_COUNTS]; }; @@ -104,7 +104,7 @@ void retire_userns_sysctls(struct user_namespace *ns); struct ucounts *inc_ucount(struct user_namespace *ns, kuid_t uid, enum ucount_type type); void dec_ucount(struct ucounts *ucounts, enum ucount_type type); struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid); -struct ucounts *get_ucounts(struct ucounts *ucounts); +struct ucounts * __must_check get_ucounts(struct ucounts *ucounts); void put_ucounts(struct ucounts *ucounts); #ifdef CONFIG_USER_NS diff --git a/kernel/ucount.c b/kernel/ucount.c index 50cc1dfb7d28..7bac19bb3f1e 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -11,7 +11,7 @@ struct ucounts init_ucounts = { .ns = &init_user_ns, .uid = GLOBAL_ROOT_UID, - .count = 1, + .count = ATOMIC_INIT(1), }; #define UCOUNTS_HASHTABLE_BITS 10 @@ -139,6 +139,15 @@ static void hlist_add_ucounts(struct ucounts *ucounts) spin_unlock_irq(&ucounts_lock); } +struct ucounts *get_ucounts(struct ucounts *ucounts) +{ + if (ucounts && atomic_add_negative(1, &ucounts->count)) { + atomic_dec(&ucounts->count); + ucounts = NULL; + } + return ucounts; +} + struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid) { struct hlist_head *hashent = ucounts_hashentry(ns, uid); @@ -155,7 +164,7 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid) new->ns = ns; new->uid = uid; - new->count = 0; + atomic_set(&new->count, 1); spin_lock_irq(&ucounts_lock); ucounts = find_ucounts(ns, uid, hashent); @@ -163,33 +172,12 @@ struct ucounts *alloc_ucounts(struct user_namespace *ns, kuid_t uid) kfree(new); } else { hlist_add_head(&new->node, hashent); - ucounts = new; + spin_unlock_irq(&ucounts_lock); + return new; } } - if (ucounts->count == INT_MAX) - ucounts = NULL; - else - ucounts->count += 1; spin_unlock_irq(&ucounts_lock); - return ucounts; -} - -struct ucounts *get_ucounts(struct ucounts *ucounts) -{ - unsigned long flags; - - if (!ucounts) - return NULL; - - spin_lock_irqsave(&ucounts_lock, flags); - if (ucounts->count == INT_MAX) { - WARN_ONCE(1, "ucounts: counter has reached its maximum value"); - ucounts = NULL; - } else { - ucounts->count += 1; - } - spin_unlock_irqrestore(&ucounts_lock, flags); - + ucounts = get_ucounts(ucounts); return ucounts; } @@ -197,15 +185,12 @@ void put_ucounts(struct ucounts *ucounts) { unsigned long flags; - spin_lock_irqsave(&ucounts_lock, flags); - ucounts->count -= 1; - if (!ucounts->count) + if (atomic_dec_and_test(&ucounts->count)) { + spin_lock_irqsave(&ucounts_lock, flags); hlist_del_init(&ucounts->node); - else - ucounts = NULL; - spin_unlock_irqrestore(&ucounts_lock, flags); - - kfree(ucounts); + spin_unlock_irqrestore(&ucounts_lock, flags); + kfree(ucounts); + } } static inline bool atomic_long_inc_below(atomic_long_t *v, int u)
The current implementation of the ucounts reference counter requires the use of spin_lock. We're going to use get_ucounts() in more performance critical areas like a handling of RLIMIT_SIGPENDING. Now we need to use spin_lock only if we want to change the hashtable. v9: * Use a negative value to check that the ucounts->count is close to overflow. Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com> --- include/linux/user_namespace.h | 4 +-- kernel/ucount.c | 53 ++++++++++++---------------------- 2 files changed, 21 insertions(+), 36 deletions(-)