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 E38F5C433F5 for ; Tue, 8 Mar 2022 03:58:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 54D2C8D0003; Mon, 7 Mar 2022 22:58:44 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4D5228D0001; Mon, 7 Mar 2022 22:58:44 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 377018D0003; Mon, 7 Mar 2022 22:58:44 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0230.hostedemail.com [216.40.44.230]) by kanga.kvack.org (Postfix) with ESMTP id 29C688D0001 for ; Mon, 7 Mar 2022 22:58:44 -0500 (EST) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id CB43DA943F for ; Tue, 8 Mar 2022 03:58:43 +0000 (UTC) X-FDA: 79219862526.25.4CF1BE5 Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by imf29.hostedemail.com (Postfix) with ESMTP id 5EAF8120007 for ; Tue, 8 Mar 2022 03:58:43 +0000 (UTC) Received: by mail-pj1-f50.google.com with SMTP id k92so6874512pjh.5 for ; Mon, 07 Mar 2022 19:58:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=iM+PV4fpxYW+ye1hFiFOovw7UPOtpG0DgMDRwiqD2pI=; b=n/TquMYDiCS5XjFnrqg2Rfynuci2nh/x9QjaQhiXqLLqPStY3bnxMEPjbLiw6cZoGp A9otGqZJeMd6QGFiV/WNAeCfNIDR7ISElcUT49MAqD/QjwZFCvK4eP8dXpJXCUbWbc9h uEteC7JvE5XvAdQRtp13A+i0HoYrpaB/J5UgctQgpbHN4rbjmiy7vjZ61ROpwvnJUoEk bq4138IxI9XWGpynxni7wJhx5OCIejex8YoIJAc5Q/3JPSiO6iWszBoEBkHDVhDlesgQ Kqsa1qmSA9bwHrzY3zoOjWR4Ue9q574rukZgL12erl1XBX3nhjvwfv9vUqYnonEcxHn6 9xGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=iM+PV4fpxYW+ye1hFiFOovw7UPOtpG0DgMDRwiqD2pI=; b=6USuhC8fTFWzxEBf+NcDSRnQKICt7Bf8rfyl7vlOBGzm+CKB34ZSBf0fX4IWomYP4X U+q5hlr/mp5M7IoccRFbsHpN8FGVrtzh458vsKBAPxT3FsJYxU/gW6nbgY2dj3PJJrSD YmDhLLQ66cfuA/B2oaSsnRaYSI78oY9c49ngZVDUnoglDWbld3pbb43QaeBb8Ywb2bB1 0AKHCR9apInK3vJrIafCxQK2sbjr18RpNaN3VzFPkccJfOq9ZdS1jKp1AT+zWr/YJF8b EVrqq4NkZ+XLZwmhtZbcQMENlfCDHeT699P5tQUaw0FdVqHTXI2SzzlBE4BVu9ByFSsQ Ri4A== X-Gm-Message-State: AOAM530C9cPn3+Ra0bPAlfK4sW/KN1rn+8iyqEb8rZmXDJRr2rzDh5Wv pmI7BGo8hJfrt8nNf6t3NPw= X-Google-Smtp-Source: ABdhPJzJPZcseuHrMrdVO5mr7+LcGX6jZWezgCPAs/E3Ubo62ruRz2MrzGgdUxgq7qTTe/ZD/Irr1g== X-Received: by 2002:a17:902:7802:b0:150:baa:bc1a with SMTP id p2-20020a170902780200b001500baabc1amr15501161pll.110.1646711922412; Mon, 07 Mar 2022 19:58:42 -0800 (PST) Received: from ip-172-31-19-208.ap-northeast-1.compute.internal (ec2-18-181-137-102.ap-northeast-1.compute.amazonaws.com. [18.181.137.102]) by smtp.gmail.com with ESMTPSA id x9-20020aa79409000000b004f704d33ca0sm4909272pfo.136.2022.03.07.19.58.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Mar 2022 19:58:41 -0800 (PST) Date: Tue, 8 Mar 2022 03:58:37 +0000 From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Vlastimil Babka Cc: linux-mm@kvack.org, Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Marco Elver , Matthew WilCox , Roman Gushchin , linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 2/2] mm/slub: refactor deactivate_slab() Message-ID: References: <20220307074057.902222-1-42.hyeyoo@gmail.com> <20220307074057.902222-3-42.hyeyoo@gmail.com> <07d4f687-544f-17d4-51cd-7b86aa23fb21@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <07d4f687-544f-17d4-51cd-7b86aa23fb21@suse.cz> X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: 5EAF8120007 X-Rspam-User: Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="n/TquMYD"; spf=pass (imf29.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Stat-Signature: nk3o5bymxwctr9t8apjtz8356gdd5ms7 X-HE-Tag: 1646711923-408020 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: On Mon, Mar 07, 2022 at 05:40:42PM +0100, Vlastimil Babka wrote: > On 3/7/22 08:40, Hyeonggon Yoo 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> > > Reviewed-by: Vlastimil Babka > > adding both to slab-next. Fixed up some nits myself, see below: > > > > > @@ -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) { > > This was against kernel style even before the patch - we use { } in the > 'else if' branch, thus all branches should use { } even if one-line. > Ah, you are right. Agree with this change. "Remove unnecessary brace" rule does not apply here. > > - 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; > > Ditto here (this is new). Yes. > > > - 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); > > goto redo; > > + } > > > > - 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); > > And here. > Yes. > > } > > > > #ifdef CONFIG_SLUB_CPU_PARTIAL > Thanks! -- Thank you, You are awesome! Hyeonggon :-)