In functions such as [__]slab_update_freelist() and __slab_update_freelist_fast/slow() we pass old and new freelist and counters as 4 separate parameters. The underlying __update_freelist_fast() then constructs struct freelist_counters variables for passing the full freelist+counter combinations to cmpxchg double. In most cases we actually start with struct freelist_counters variables, but then pass the individual fields, only to construct new struct freelist_counters variables. While it's all inlined and thus should be efficient, we can simplify this code. Thus replace the 4 parameters for individual fields with two freelist_aba_t pointers wherever applicable. __update_freelist_fast() can then pass them directly to try_cmpxchg_freelist(). The code is also more obvious as the pattern becomes unified such that we set up "old" and "new" struct freelist_counters variables upfront as we fully need them to be, and simply call [__]slab_update_freelist() on them. Previously some of the "new" values would be hidden as one of the many parameters and thus make it harder to figure out what the code does. Signed-off-by: Vlastimil Babka --- mm/slub.c | 126 ++++++++++++++++++++++++++------------------------------------ 1 file changed, 52 insertions(+), 74 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index a55e0af26ec7..ddd71f4937fa 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -759,34 +759,29 @@ static __always_inline void slab_unlock(struct slab *slab) } static inline bool -__update_freelist_fast(struct slab *slab, - void *freelist_old, unsigned long counters_old, - void *freelist_new, unsigned long counters_new) +__update_freelist_fast(struct slab *slab, struct freelist_counters *old, + struct freelist_counters *new) { #ifdef system_has_freelist_aba - struct freelist_counters old = { .freelist = freelist_old, .counters = counters_old }; - struct freelist_counters new = { .freelist = freelist_new, .counters = counters_new }; - return try_cmpxchg_freelist(&slab->freelist_counters, - &old.freelist_counters, - new.freelist_counters); + &old->freelist_counters, + new->freelist_counters); #else return false; #endif } static inline bool -__update_freelist_slow(struct slab *slab, - void *freelist_old, unsigned long counters_old, - void *freelist_new, unsigned long counters_new) +__update_freelist_slow(struct slab *slab, struct freelist_counters *old, + struct freelist_counters *new) { bool ret = false; slab_lock(slab); - if (slab->freelist == freelist_old && - slab->counters == counters_old) { - slab->freelist = freelist_new; - slab->counters = counters_new; + if (slab->freelist == old->freelist && + slab->counters == old->counters) { + slab->freelist = new->freelist; + slab->counters = new->counters; ret = true; } slab_unlock(slab); @@ -802,22 +797,18 @@ __update_freelist_slow(struct slab *slab, * interrupt the operation. */ static inline bool __slab_update_freelist(struct kmem_cache *s, struct slab *slab, - void *freelist_old, unsigned long counters_old, - void *freelist_new, unsigned long counters_new, - const char *n) + struct freelist_counters *old, struct freelist_counters *new, const char *n) { bool ret; if (USE_LOCKLESS_FAST_PATH()) lockdep_assert_irqs_disabled(); - if (s->flags & __CMPXCHG_DOUBLE) { - ret = __update_freelist_fast(slab, freelist_old, counters_old, - freelist_new, counters_new); - } else { - ret = __update_freelist_slow(slab, freelist_old, counters_old, - freelist_new, counters_new); - } + if (s->flags & __CMPXCHG_DOUBLE) + ret = __update_freelist_fast(slab, old, new); + else + ret = __update_freelist_slow(slab, old, new); + if (likely(ret)) return true; @@ -832,21 +823,17 @@ static inline bool __slab_update_freelist(struct kmem_cache *s, struct slab *sla } static inline bool slab_update_freelist(struct kmem_cache *s, struct slab *slab, - void *freelist_old, unsigned long counters_old, - void *freelist_new, unsigned long counters_new, - const char *n) + struct freelist_counters *old, struct freelist_counters *new, const char *n) { bool ret; if (s->flags & __CMPXCHG_DOUBLE) { - ret = __update_freelist_fast(slab, freelist_old, counters_old, - freelist_new, counters_new); + ret = __update_freelist_fast(slab, old, new); } else { unsigned long flags; local_irq_save(flags); - ret = __update_freelist_slow(slab, freelist_old, counters_old, - freelist_new, counters_new); + ret = __update_freelist_slow(slab, old, new); local_irq_restore(flags); } if (likely(ret)) @@ -3774,10 +3761,7 @@ static void deactivate_slab(struct kmem_cache *s, struct slab *slab, } else { new.freelist = old.freelist; } - } while (!slab_update_freelist(s, slab, - old.freelist, old.counters, - new.freelist, new.counters, - "unfreezing slab")); + } while (!slab_update_freelist(s, slab, &old, &new, "unfreezing slab")); /* * Stage three: Manipulate the slab list based on the updated state. @@ -4389,27 +4373,24 @@ __update_cpu_freelist_fast(struct kmem_cache *s, */ static inline void *get_freelist(struct kmem_cache *s, struct slab *slab) { - struct freelist_counters new; - unsigned long counters; - void *freelist; + struct freelist_counters old, new; lockdep_assert_held(this_cpu_ptr(&s->cpu_slab->lock)); do { - freelist = slab->freelist; - counters = slab->counters; + old.freelist = slab->freelist; + old.counters = slab->counters; - new.counters = counters; + new.freelist = NULL; + new.counters = old.counters; - new.inuse = slab->objects; - new.frozen = freelist != NULL; + new.inuse = old.objects; + new.frozen = old.freelist != NULL; - } while (!__slab_update_freelist(s, slab, - freelist, counters, - NULL, new.counters, - "get_freelist")); - return freelist; + } while (!__slab_update_freelist(s, slab, &old, &new, "get_freelist")); + + return old.freelist; } /* @@ -4417,26 +4398,22 @@ static inline void *get_freelist(struct kmem_cache *s, struct slab *slab) */ static inline void *freeze_slab(struct kmem_cache *s, struct slab *slab) { - struct freelist_counters new; - unsigned long counters; - void *freelist; + struct freelist_counters old, new; do { - freelist = slab->freelist; - counters = slab->counters; + old.freelist = slab->freelist; + old.counters = slab->counters; - new.counters = counters; + new.freelist = NULL; + new.counters = old.counters; VM_BUG_ON(new.frozen); - new.inuse = slab->objects; + new.inuse = old.objects; new.frozen = 1; - } while (!slab_update_freelist(s, slab, - freelist, counters, - NULL, new.counters, - "freeze_slab")); + } while (!slab_update_freelist(s, slab, &old, &new, "freeze_slab")); - return freelist; + return old.freelist; } /* @@ -5864,10 +5841,8 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, unsigned long addr) { - void *old_head; bool was_frozen, was_full; - struct freelist_counters new; - unsigned long counters; + struct freelist_counters old, new; struct kmem_cache_node *n = NULL; unsigned long flags; bool on_node_partial; @@ -5891,13 +5866,19 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, spin_unlock_irqrestore(&n->list_lock, flags); n = NULL; } - old_head = slab->freelist; - counters = slab->counters; - set_freepointer(s, tail, old_head); - new.counters = counters; - was_frozen = !!new.frozen; - was_full = (old_head == NULL); + + old.freelist = slab->freelist; + old.counters = slab->counters; + + was_full = (old.freelist == NULL); + was_frozen = old.frozen; + + set_freepointer(s, tail, old.freelist); + + new.freelist = head; + new.counters = old.counters; new.inuse -= cnt; + /* * Might need to be taken off (due to becoming empty) or added * to (due to not being full anymore) the partial list. @@ -5926,10 +5907,7 @@ static void __slab_free(struct kmem_cache *s, struct slab *slab, } } - } while (!slab_update_freelist(s, slab, - old_head, counters, - head, new.counters, - "__slab_free")); + } while (!slab_update_freelist(s, slab, &old, &new, "__slab_free")); if (likely(!n)) { -- 2.51.1