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 X-Spam-Level: X-Spam-Status: No, score=-6.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0103FC433E0 for ; Mon, 15 Jun 2020 07:25:15 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B57BA2068E for ; Mon, 15 Jun 2020 07:25:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ITjU0Iqb" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B57BA2068E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 447656B0002; Mon, 15 Jun 2020 03:25:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3F8926B0003; Mon, 15 Jun 2020 03:25:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 30D1D6B0005; Mon, 15 Jun 2020 03:25:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0021.hostedemail.com [216.40.44.21]) by kanga.kvack.org (Postfix) with ESMTP id 1724F6B0002 for ; Mon, 15 Jun 2020 03:25:15 -0400 (EDT) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id B2F641EE6 for ; Mon, 15 Jun 2020 07:25:14 +0000 (UTC) X-FDA: 76930610148.12.bells50_330900326df4 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin12.hostedemail.com (Postfix) with ESMTP id 7C19B1800EC5D for ; Mon, 15 Jun 2020 07:25:14 +0000 (UTC) X-HE-Tag: bells50_330900326df4 X-Filterd-Recvd-Size: 5727 Received: from mail-qk1-f195.google.com (mail-qk1-f195.google.com [209.85.222.195]) by imf41.hostedemail.com (Postfix) with ESMTP for ; Mon, 15 Jun 2020 07:25:13 +0000 (UTC) Received: by mail-qk1-f195.google.com with SMTP id c14so14780357qka.11 for ; Mon, 15 Jun 2020 00:25:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=Xho6qyVeexXYA3wm+wd4bvhMpPUxi3GFns+ObYF/cv4=; b=ITjU0Iqb2xbgvV8fsTzj2XBjV3w/Y5c9TvXyiFr3rgcx0eCK6gauD6dUmDpD+jI0fE 0yhvcgmaCKMDfwZAX9ndCZcWZeKR6Q187szmkAIMVSpyniilg5pt3yIetv8kDIE/o6/K GQkDWS3zsFIDBTpoMoy/eRLu9DXOny3LRRgWaB56r/KzIEI9MFRbgbNlTdWujFH019Rv +Ki/BeUpxpmSG1nKlpqiuINQmCUO4Gm5jkPMBJ8PYW2AXZo8EhB+UOYt3BFnQWoSSiBD GGCrimL4Dc09BSE/mZQzUIji4YIBR5CQvAUXUzVkMwOpyHCa/+HnVFuGxowtx6bWqKzX aASg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Xho6qyVeexXYA3wm+wd4bvhMpPUxi3GFns+ObYF/cv4=; b=hW6Yny3X540iHEbg0v8whSlaGeP5TvERldO2jbiRIoWEHl+HTitQXsqEzYHOZaK2LM qP7Fk6pDBz48xSaV1HIuj1p40u+YvNsCSSPHeZI+zKRfnjsGRL5388/wRI1SJhqvH6Ab 82RwRld3zjsU9+XeC42qpVXIhqK0D7pJ6urUJYe1yjF5toxuK2eoqqEWs7HgKTLxTPml MTPanPf9lHbJjzoAkX13sMjnVr9excMzeXbK0zaUsS82fNMpRU6QdBN3kBqcix8NxHl+ w+IS5odSol4p7MJCeLnk7+cBy3jWqpmpnAMTIG/Ma/7blh5P8L5PAgpCmPNKSBnK/XTj 9MPQ== X-Gm-Message-State: AOAM533NS5w583o5l8M4hEhg1M/o4543Frj4Kg2y0CLyd8nb+IA5vx+P TylHoNlHa5sEfKC/IsC5Da4yWSRORUlUjcJzLYM= X-Google-Smtp-Source: ABdhPJyQ1DPgJZRSejZYsMilYn66nlMcBpuWoODr2xR0F92SFOCmDp/xGQ8J95NknBu8PiX5WmYpFj4pOSDsyXCI02s= X-Received: by 2002:a37:624a:: with SMTP id w71mr14325231qkb.452.1592205913355; Mon, 15 Jun 2020 00:25:13 -0700 (PDT) MIME-Version: 1.0 References: <20200614123923.99189-1-songmuchun@bytedance.com> <20200614123923.99189-4-songmuchun@bytedance.com> In-Reply-To: From: Joonsoo Kim Date: Mon, 15 Jun 2020 16:25:02 +0900 Message-ID: Subject: Re: [External] Re: [PATCH 3/3] mm/slub: Fix release all resources used by a slab cache To: Muchun Song Cc: Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Linux Memory Management List , LKML Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 7C19B1800EC5D X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam01 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: 2020=EB=85=84 6=EC=9B=94 15=EC=9D=BC (=EC=9B=94) =EC=98=A4=ED=9B=84 3:41, M= uchun Song =EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84=B1: > > On Mon, Jun 15, 2020 at 2:23 PM Joonsoo Kim wrote: > > > > 2020=EB=85=84 6=EC=9B=94 14=EC=9D=BC (=EC=9D=BC) =EC=98=A4=ED=9B=84 9:3= 9, Muchun Song =EB=8B=98=EC=9D=B4 =EC=9E=91=EC=84= =B1: > > > > > > The function of __kmem_cache_shutdown() is that release all resources > > > used by the slab cache, while currently it stop release resources whe= n > > > the preceding node is not empty. > > > > > > Signed-off-by: Muchun Song > > > --- > > > mm/slub.c | 7 ++++--- > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > diff --git a/mm/slub.c b/mm/slub.c > > > index b73505df3de2..4e477ef0f2b9 100644 > > > --- a/mm/slub.c > > > +++ b/mm/slub.c > > > @@ -3839,6 +3839,7 @@ bool __kmem_cache_empty(struct kmem_cache *s) > > > */ > > > int __kmem_cache_shutdown(struct kmem_cache *s) > > > { > > > + int ret =3D 0; > > > int node; > > > struct kmem_cache_node *n; > > > > > > @@ -3846,11 +3847,11 @@ int __kmem_cache_shutdown(struct kmem_cache *= s) > > > /* Attempt to free all objects */ > > > for_each_kmem_cache_node(s, node, n) { > > > free_partial(s, n); > > > - if (node_nr_slabs(n)) > > > - return 1; > > > + if (!ret && node_nr_slabs(n)) > > > + ret =3D 1; > > > } > > > > I don't think that this is an improvement. > > > > If the shutdown condition isn't met, we don't need to process further. > > Just 'return 1' looks okay to me. > > > > And, with this change, sysfs_slab_remove() is called even if the > > shutdown is failed. > > It's better not to have side effects when failing. > > If someone calls __kmem_cache_shutdown, he may want to release > resources used by the slab cache as much as possible. If we continue, > we may release more pages. From this point, is it an improvement? My opinion is not strong but I still think that it's not useful enough to complicate the code. If shutdown is failed, it implies there are some bugs and someone should fi= x it. Releasing more resources would mitigate the resource problem but doesn't change the situation that someone should fix it soon. Anyway, I don't object more if you don't agree with my opinion. In that cas= e, please fix not to call sysfs_slab_remove() when ret is 1. Thanks.