| Message ID | 20210811111921.85999-1-songmuchun@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: slub: remove preemption disabling from put_cpu_partial | expand |
On 8/11/21 1:19 PM, Muchun Song wrote: > The commit d6e0b7fa1186 ("slub: make dead caches discard free slabs > immediately") introduced those logic to speed up the destruction of > per-memcg kmem caches, because kmem caches created for a memory > cgroup are only destroyed after the last page charged to the cgroup > is freed at that time. But since commit 9855609bde03 ("mm: memcg/slab: > use a single set of kmem_caches for all accounted allocations), we > do not have per-memcg kmem caches anymore. Are those code pointless? > No, the kmem_cache->cpu_partial can be set to zero by 'echo 0 > /sys/ > kernel/slab/*/cpu_partial'. In this case, the slab page will be put > into cpu partial list and then moved to node list (because > slub_cpu_partial() returns zero). However, we can skip putting the > slab page to cpu partial list and just move it to node list directly. > We can adjust the condition of kmem_cache_has_cpu_partial() to > slub_cpu_partial() in __slab_free() and remove those code from > put_cpu_partial() for simplification. > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> Please check again current mmotm/next if this still applies, I think it shouldn't anymore. Thanks. > --- > mm/slub.c | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index b6c5205252eb..69c8ada322a0 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2438,7 +2438,6 @@ static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain) > int pages; > int pobjects; > > - preempt_disable(); > do { > pages = 0; > pobjects = 0; > @@ -2470,16 +2469,7 @@ static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain) > page->pobjects = pobjects; > page->next = oldpage; > > - } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page) > - != oldpage); > - if (unlikely(!slub_cpu_partial(s))) { > - unsigned long flags; > - > - local_irq_save(flags); > - unfreeze_partials(s, this_cpu_ptr(s->cpu_slab)); > - local_irq_restore(flags); > - } > - preempt_enable(); > + } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page) != oldpage); > #endif /* CONFIG_SLUB_CPU_PARTIAL */ > } > > @@ -3059,9 +3049,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page, > was_frozen = new.frozen; > new.inuse -= cnt; > if ((!new.inuse || !prior) && !was_frozen) { > - > - if (kmem_cache_has_cpu_partial(s) && !prior) { > - > + if (slub_cpu_partial(s) && !prior) { > /* > * Slab was on no list before and will be > * partially empty > @@ -3069,9 +3057,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page, > * freeze it. > */ > new.frozen = 1; > - > } else { /* Needs to be taken off a list */ > - > n = get_node(s, page_to_nid(page)); > /* > * Speculatively acquire the list_lock. > @@ -3082,17 +3068,14 @@ static void __slab_free(struct kmem_cache *s, struct page *page, > * other processors updating the list of slabs. > */ > spin_lock_irqsave(&n->list_lock, flags); > - > } > } > - > } while (!cmpxchg_double_slab(s, page, > prior, counters, > head, new.counters, > "__slab_free")); > > if (likely(!n)) { > - > if (likely(was_frozen)) { > /* > * The list lock was not taken therefore no list > @@ -3118,7 +3101,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page, > * Objects left in the slab. If it was not on the partial list before > * then add it. > */ > - if (!kmem_cache_has_cpu_partial(s) && unlikely(!prior)) { > + if (unlikely(!prior)) { > remove_full(s, n, page); > add_partial(n, page, DEACTIVATE_TO_TAIL); > stat(s, FREE_ADD_PARTIAL); >
On Wed, Aug 11, 2021 at 8:40 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > On 8/11/21 1:19 PM, Muchun Song wrote: > > The commit d6e0b7fa1186 ("slub: make dead caches discard free slabs > > immediately") introduced those logic to speed up the destruction of > > per-memcg kmem caches, because kmem caches created for a memory > > cgroup are only destroyed after the last page charged to the cgroup > > is freed at that time. But since commit 9855609bde03 ("mm: memcg/slab: > > use a single set of kmem_caches for all accounted allocations), we > > do not have per-memcg kmem caches anymore. Are those code pointless? > > No, the kmem_cache->cpu_partial can be set to zero by 'echo 0 > /sys/ > > kernel/slab/*/cpu_partial'. In this case, the slab page will be put > > into cpu partial list and then moved to node list (because > > slub_cpu_partial() returns zero). However, we can skip putting the > > slab page to cpu partial list and just move it to node list directly. > > We can adjust the condition of kmem_cache_has_cpu_partial() to > > slub_cpu_partial() in __slab_free() and remove those code from > > put_cpu_partial() for simplification. > > > > Signed-off-by: Muchun Song <songmuchun@bytedance.com> > > Please check again current mmotm/next if this still applies, I think it > shouldn't anymore. Thanks. > You are right. I didn't see it before. I guess it was merged recently. But thanks for your reminder.
diff --git a/mm/slub.c b/mm/slub.c index b6c5205252eb..69c8ada322a0 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2438,7 +2438,6 @@ static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain) int pages; int pobjects; - preempt_disable(); do { pages = 0; pobjects = 0; @@ -2470,16 +2469,7 @@ static void put_cpu_partial(struct kmem_cache *s, struct page *page, int drain) page->pobjects = pobjects; page->next = oldpage; - } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page) - != oldpage); - if (unlikely(!slub_cpu_partial(s))) { - unsigned long flags; - - local_irq_save(flags); - unfreeze_partials(s, this_cpu_ptr(s->cpu_slab)); - local_irq_restore(flags); - } - preempt_enable(); + } while (this_cpu_cmpxchg(s->cpu_slab->partial, oldpage, page) != oldpage); #endif /* CONFIG_SLUB_CPU_PARTIAL */ } @@ -3059,9 +3049,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page, was_frozen = new.frozen; new.inuse -= cnt; if ((!new.inuse || !prior) && !was_frozen) { - - if (kmem_cache_has_cpu_partial(s) && !prior) { - + if (slub_cpu_partial(s) && !prior) { /* * Slab was on no list before and will be * partially empty @@ -3069,9 +3057,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page, * freeze it. */ new.frozen = 1; - } else { /* Needs to be taken off a list */ - n = get_node(s, page_to_nid(page)); /* * Speculatively acquire the list_lock. @@ -3082,17 +3068,14 @@ static void __slab_free(struct kmem_cache *s, struct page *page, * other processors updating the list of slabs. */ spin_lock_irqsave(&n->list_lock, flags); - } } - } while (!cmpxchg_double_slab(s, page, prior, counters, head, new.counters, "__slab_free")); if (likely(!n)) { - if (likely(was_frozen)) { /* * The list lock was not taken therefore no list @@ -3118,7 +3101,7 @@ static void __slab_free(struct kmem_cache *s, struct page *page, * Objects left in the slab. If it was not on the partial list before * then add it. */ - if (!kmem_cache_has_cpu_partial(s) && unlikely(!prior)) { + if (unlikely(!prior)) { remove_full(s, n, page); add_partial(n, page, DEACTIVATE_TO_TAIL); stat(s, FREE_ADD_PARTIAL);
The commit d6e0b7fa1186 ("slub: make dead caches discard free slabs immediately") introduced those logic to speed up the destruction of per-memcg kmem caches, because kmem caches created for a memory cgroup are only destroyed after the last page charged to the cgroup is freed at that time. But since commit 9855609bde03 ("mm: memcg/slab: use a single set of kmem_caches for all accounted allocations), we do not have per-memcg kmem caches anymore. Are those code pointless? No, the kmem_cache->cpu_partial can be set to zero by 'echo 0 > /sys/ kernel/slab/*/cpu_partial'. In this case, the slab page will be put into cpu partial list and then moved to node list (because slub_cpu_partial() returns zero). However, we can skip putting the slab page to cpu partial list and just move it to node list directly. We can adjust the condition of kmem_cache_has_cpu_partial() to slub_cpu_partial() in __slab_free() and remove those code from put_cpu_partial() for simplification. Signed-off-by: Muchun Song <songmuchun@bytedance.com> --- mm/slub.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)