From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 372BCC433F5 for ; Tue, 8 Mar 2022 01:40:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id ABB508D0002; Mon, 7 Mar 2022 20:40:35 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id A6A0C8D0001; Mon, 7 Mar 2022 20:40:35 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 931E78D0002; Mon, 7 Mar 2022 20:40:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.27]) by kanga.kvack.org (Postfix) with ESMTP id 83ADC8D0001 for ; Mon, 7 Mar 2022 20:40:35 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 45D6D20636 for ; Tue, 8 Mar 2022 01:40:35 +0000 (UTC) X-FDA: 79219514430.03.6C3BE84 Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by imf23.hostedemail.com (Postfix) with ESMTP id C6B2B140006 for ; Tue, 8 Mar 2022 01:40:34 +0000 (UTC) Received: by mail-pl1-f170.google.com with SMTP id s18so2449206plp.1 for ; Mon, 07 Mar 2022 17:40:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=In56Q2F318ovMwvLgvHjGx4bpPv8YUwX7l8iUmQKcxs=; b=A7qhRMNJ7VrqboeRiCx1fb62NhPps/u3YBRgBaB9fMhtrO8KvpSSBDpnKLHEw5FGQe 1B8KgGnuSC6/lhvXTnEIJsnAu19kTuFtkvUus7hqgU9QquaSpCGtksYUTjK6wK6zp2LH YbL5cHGTAVY7kp1jAiAHlPH9l/VQkbjI7NvnJc6h5MqP8QFrtI8smOO3w8VfuBRJa60E XHhGbEgET/XQAmJqvWXuREDMVljKNTz2K0vQ7xioG8qJU4A6rKvUTe7w33ngEoGSjHL9 lbRvs2DDiCfW9nJigQhYvtOX0RtOZ44lWeOmEQfrxf6GA00YxahHXLrSaCkRvTm27Ysg dIZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=In56Q2F318ovMwvLgvHjGx4bpPv8YUwX7l8iUmQKcxs=; b=bYgSofp0MA5W2esaW5JykO8xnZx6EWCtP5DTOQ5RHSlpbdn8ZEPt3pffDeiaZlfY1P K3RJajarFtWQwOIouocKUwVrg8TH9sTItAiI27O5Vox0hEagOmHXBuUIbALKSPlycF/N 4R2A3y8C7Z3k1oY1KMy3XwzTVzJiU44Ir3h7cwyzpsJrhMBRvUsvzUMVFb+pMaaUA/L9 Vb0Hhd/nVNIb+9X1NIh/aOB8OaVUvfjBJBywx7I/ZrzmOXpe98JZOPTXMUXTRYS+TGIp uG8fWyNl7plgEhiTQT7N7d/+xHSyts/ANAj0ttx4HHyiIrzGzqnj5THPx0RpzHcJb0af Z3jQ== X-Gm-Message-State: AOAM530/qXaOTe2wNW3F2zJ2514KBKTUSJoCKAPeMXx9vPJFTjXDtJiL u9dCqg5KLRP5Y0nz6hwl7y9YcgK4ihmXfJO9SWU= X-Google-Smtp-Source: ABdhPJwNvQPh6ByDLUBeKqCDF+ZHTYDXopMivvlSv3PaXHLzWnSNJbjYX84JgxUIZjdZZagfs/PXgU8QjoUcamcWD/E= X-Received: by 2002:a17:90a:4411:b0:1bc:99b0:acad with SMTP id s17-20020a17090a441100b001bc99b0acadmr2027583pjg.25.1646703633757; Mon, 07 Mar 2022 17:40:33 -0800 (PST) MIME-Version: 1.0 References: <20220307074057.902222-1-42.hyeyoo@gmail.com> <20220307074057.902222-3-42.hyeyoo@gmail.com> In-Reply-To: <20220307074057.902222-3-42.hyeyoo@gmail.com> From: Xiongwei Song Date: Tue, 8 Mar 2022 09:40:07 +0800 Message-ID: Subject: Re: [PATCH v3 2/2] mm/slub: refactor deactivate_slab() To: Hyeonggon Yoo <42.hyeyoo@gmail.com> Cc: linux-mm@kvack.org, Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Marco Elver , Matthew WilCox , Roman Gushchin , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: C6B2B140006 X-Stat-Signature: thy1ma99zkhb88tjtqd79g77tg5ax19y Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=A7qhRMNJ; spf=pass (imf23.hostedemail.com: domain of sxwjean@gmail.com designates 209.85.214.170 as permitted sender) smtp.mailfrom=sxwjean@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-HE-Tag: 1646703634-966578 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hello, On Mon, Mar 7, 2022 at 3:41 PM Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote: > > Simplify deactivate_slab() by unlocking n->list_lock and retrying > cmpxchg_double() when cmpxchg_double() fails, and perform > add_{partial,full} only when it succeed. > > Releasing and taking n->list_lock again here is not harmful as SLUB > avoids deactivating slabs as much as possible. > > [ vbabka@suse.cz: perform add_{partial,full} when cmpxchg_double() > succeed. > > count deactivating full slabs even if debugging flag is not set. ] > > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > --- > mm/slub.c | 91 +++++++++++++++++++++++-------------------------------- > 1 file changed, 38 insertions(+), 53 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 1ce09b0347ad..f0cb9d0443ac 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -2348,10 +2348,10 @@ static void init_kmem_cache_cpus(struct kmem_cache *s) > static void deactivate_slab(struct kmem_cache *s, struct slab *slab, > void *freelist) > { > - enum slab_modes { M_NONE, M_PARTIAL, M_FULL, M_FREE }; > + enum slab_modes { M_NONE, M_PARTIAL, M_FULL, M_FREE, M_FULL_NOLIST }; > struct kmem_cache_node *n = get_node(s, slab_nid(slab)); > - int lock = 0, free_delta = 0; > - enum slab_modes l = M_NONE, m = M_NONE; > + int free_delta = 0; > + enum slab_modes mode = M_NONE; > void *nextfree, *freelist_iter, *freelist_tail; > int tail = DEACTIVATE_TO_HEAD; > unsigned long flags = 0; > @@ -2393,14 +2393,10 @@ static void deactivate_slab(struct kmem_cache *s, struct slab *slab, > * Ensure that the slab is unfrozen while the list presence > * reflects the actual number of objects during unfreeze. > * > - * We setup the list membership and then perform a cmpxchg > - * with the count. If there is a mismatch then the slab > - * is not unfrozen but the slab is on the wrong list. > - * > - * Then we restart the process which may have to remove > - * the slab from the list that we just put it on again > - * because the number of objects in the slab may have > - * changed. > + * We first perform cmpxchg holding lock and insert to list > + * when it succeed. If there is mismatch then the slab is not > + * unfrozen and number of objects in the slab may have changed. > + * Then release lock and retry cmpxchg again. > */ > redo: > > @@ -2420,61 +2416,50 @@ static void deactivate_slab(struct kmem_cache *s, struct slab *slab, > new.frozen = 0; > > if (!new.inuse && n->nr_partial >= s->min_partial) > - m = M_FREE; > + mode = M_FREE; > else if (new.freelist) { > - m = M_PARTIAL; > - if (!lock) { > - lock = 1; > - /* > - * Taking the spinlock removes the possibility that > - * acquire_slab() will see a slab that is frozen > - */ > - spin_lock_irqsave(&n->list_lock, flags); > - } > - } else { > - m = M_FULL; > - if (kmem_cache_debug_flags(s, SLAB_STORE_USER) && !lock) { > - lock = 1; > - /* > - * This also ensures that the scanning of full > - * slabs from diagnostic functions will not see > - * any frozen slabs. > - */ > - spin_lock_irqsave(&n->list_lock, flags); > - } > - } > - > - if (l != m) { > - if (l == M_PARTIAL) > - remove_partial(n, slab); > - else if (l == M_FULL) > - remove_full(s, n, slab); > + mode = M_PARTIAL; > + /* > + * Taking the spinlock removes the possibility that > + * acquire_slab() will see a slab that is frozen > + */ > + spin_lock_irqsave(&n->list_lock, flags); > + } else if (kmem_cache_debug_flags(s, SLAB_STORE_USER)) { > + mode = M_FULL; > + /* > + * This also ensures that the scanning of full > + * slabs from diagnostic functions will not see > + * any frozen slabs. > + */ > + spin_lock_irqsave(&n->list_lock, flags); > + } else > + mode = M_FULL_NOLIST; > > - if (m == M_PARTIAL) > - add_partial(n, slab, tail); > - else if (m == M_FULL) > - add_full(s, n, slab); > - } > > - l = m; > if (!cmpxchg_double_slab(s, slab, > old.freelist, old.counters, > new.freelist, new.counters, > - "unfreezing slab")) > + "unfreezing slab")) { > + if (mode == M_PARTIAL || mode == M_FULL) > + spin_unlock_irqrestore(&n->list_lock, flags); The slab doesn't belong to any node here, should we remove locking/unlocking spin for cmpxchg_double_slab() call? Just calling spin_lock_irqsave() before add_partial()/add_full call is fine? > goto redo; How about do {...} while(!cmpxchg_double_slab())? The readability looks better? Regards, Xiongwei > + } > > - if (lock) > - spin_unlock_irqrestore(&n->list_lock, flags); > > - if (m == M_PARTIAL) > + if (mode == M_PARTIAL) { > + add_partial(n, slab, tail); > + spin_unlock_irqrestore(&n->list_lock, flags); > stat(s, tail); > - else if (m == M_FULL) > - stat(s, DEACTIVATE_FULL); > - else if (m == M_FREE) { > + } else if (mode == M_FREE) { > stat(s, DEACTIVATE_EMPTY); > discard_slab(s, slab); > stat(s, FREE_SLAB); > - } > + } else if (mode == M_FULL) { > + add_full(s, n, slab); > + spin_unlock_irqrestore(&n->list_lock, flags); > + stat(s, DEACTIVATE_FULL); > + } else if (mode == M_FULL_NOLIST) > + stat(s, DEACTIVATE_FULL); > } > > #ifdef CONFIG_SLUB_CPU_PARTIAL > -- > 2.33.1 > >