Message ID | 20230531095742.2480623-5-qi.zheng@linux.dev |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp2769608vqr; Wed, 31 May 2023 03:21:34 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ7zmOB3zXilMznoixG/mY7jKr3u0KJs9Cn8J6ASq1LSzj7pCjK4L/7ocj7yjnOfr85PA3E2 X-Received: by 2002:a05:6a00:98f:b0:645:c730:f826 with SMTP id u15-20020a056a00098f00b00645c730f826mr5247984pfg.24.1685528494012; Wed, 31 May 2023 03:21:34 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685528493; cv=none; d=google.com; s=arc-20160816; b=NUdszfnx7MImD4z9J1Pg12PiR4VVcMYjpjHr8ADYgQPuLLqS7OfTK5Emxz4I5V7HPe MP4fnajH9IWMGBel7dXM6iYd2cchdQgPsRQ/a3iDbxZazhWs4QTpUFGim8l0Dtmwdn3B McgAMoh4MmvFXT7wIdlKEEMsMx5u8pAuH616aK1OhlpwJi5/eeM1XBqsazuNcZXUPebK eo+x50C4rpJF0ulx5nvjO8u4z4kRtryicT22iPB4fUs4yw8qM98FUzzQpe2noWQECBne xwVVOKF5UjIGp4prA5oUyY0D0WlkW9giacAe/19BMsv7PwAHLQAS5KMYY9Ena6v8I9yW B9LQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=+ES1OZTmJM6HPZiOufnJqkIPYyFHG+uxk+XMAxdW/H0=; b=E/OMVltG6IqzkMoFa/Pgn6zS9jvK1Sn5858i6dki7viCFlAVv4XS6tCedwMfIMKGRC th3xC5/SqOMuICMBxNXxdKPX6Po1KzmVUcdhXqGfo5h4yz3U272+fnMSn9lAcplK+rcT FbJyDdCDAwqlZCKAGeShHgrCq8EBfIqPxSIAeOJUUhFhS/hP2II/gRbR+nmaZyIALXXn 8Xu6Gavi9H2Q6jOMKsvymR7dIecmsrSgAF4DIFHlWnahD7onPvCC4sToOAFWL+kHGdST Yh2FvyuNa0HOnoL3XtIaKcTbTTNIf+QGOs6Acee4dd+mLTERQfwfrav/nxCd4yZlVFyu fkoA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=pME7V5sg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id f195-20020a6238cc000000b0064d48952fd6si1577455pfa.28.2023.05.31.03.21.20; Wed, 31 May 2023 03:21:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=pME7V5sg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235726AbjEaJ7O (ORCPT <rfc822;andrewvogler123@gmail.com> + 99 others); Wed, 31 May 2023 05:59:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54642 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235745AbjEaJ7G (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 31 May 2023 05:59:06 -0400 Received: from out-7.mta0.migadu.com (out-7.mta0.migadu.com [91.218.175.7]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 043E4124 for <linux-kernel@vger.kernel.org>; Wed, 31 May 2023 02:58:41 -0700 (PDT) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1685527112; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+ES1OZTmJM6HPZiOufnJqkIPYyFHG+uxk+XMAxdW/H0=; b=pME7V5sghWDhiBYKlYhlIGGcd+I8ZIhtp+E9A11aBMxPx6kduDLdGh2+5uOlz/UhrC8USH AInETjEYhvnYApFG/QCWzh1ONDgIapx8Zsu0DP7P6tbY+y9jVOZnflKJtLkCkwkXmah55p +X/uwLrssbh7WgAqh01669o0MFk+pHM= From: Qi Zheng <qi.zheng@linux.dev> To: akpm@linux-foundation.org, tkhai@ya.ru, roman.gushchin@linux.dev, vbabka@suse.cz, viro@zeniv.linux.org.uk, brauner@kernel.org, djwong@kernel.org, hughd@google.com, paulmck@kernel.org, muchun.song@linux.dev Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org, Qi Zheng <zhengqi.arch@bytedance.com> Subject: [PATCH 4/8] fs: shrink only (SB_ACTIVE|SB_BORN) superblocks in super_cache_scan() Date: Wed, 31 May 2023 09:57:38 +0000 Message-Id: <20230531095742.2480623-5-qi.zheng@linux.dev> In-Reply-To: <20230531095742.2480623-1-qi.zheng@linux.dev> References: <20230531095742.2480623-1-qi.zheng@linux.dev> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1767404726179978773?= X-GMAIL-MSGID: =?utf-8?q?1767404726179978773?= |
Series |
make unregistration of super_block shrinker more faster
|
|
Commit Message
Qi Zheng
May 31, 2023, 9:57 a.m. UTC
From: Kirill Tkhai <tkhai@ya.ru> This patch prepares superblock shrinker for delayed unregistering. It makes super_cache_scan() avoid shrinking of not active superblocks. SB_ACTIVE is used as such the indicator. In case of superblock is not active, super_cache_scan() just exits with SHRINK_STOP as result. Note, that SB_ACTIVE is cleared in generic_shutdown_super() and this is made under the write lock of s_umount. Function super_cache_scan() also takes the read lock of s_umount, so it can't skip this flag cleared. SB_BORN check is added to super_cache_scan() just for uniformity with super_cache_count(), while super_cache_count() received SB_ACTIVE check just for uniformity with super_cache_scan(). After this patch super_cache_scan() becomes to ignore unregistering superblocks, so this function is OK with splitting unregister_shrinker(). Next patches prepare super_cache_count() to follow this way. Signed-off-by: Kirill Tkhai <tkhai@ya.ru> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> --- fs/super.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Comments
On Wed, May 31, 2023 at 09:57:38AM +0000, Qi Zheng wrote: > From: Kirill Tkhai <tkhai@ya.ru> > > This patch prepares superblock shrinker for delayed unregistering. > It makes super_cache_scan() avoid shrinking of not active superblocks. > SB_ACTIVE is used as such the indicator. In case of superblock is not > active, super_cache_scan() just exits with SHRINK_STOP as result. > > Note, that SB_ACTIVE is cleared in generic_shutdown_super() and this > is made under the write lock of s_umount. Function super_cache_scan() > also takes the read lock of s_umount, so it can't skip this flag cleared. > > SB_BORN check is added to super_cache_scan() just for uniformity > with super_cache_count(), while super_cache_count() received SB_ACTIVE > check just for uniformity with super_cache_scan(). > > After this patch super_cache_scan() becomes to ignore unregistering > superblocks, so this function is OK with splitting unregister_shrinker(). > Next patches prepare super_cache_count() to follow this way. > > Signed-off-by: Kirill Tkhai <tkhai@ya.ru> > Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com> > --- > fs/super.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/super.c b/fs/super.c > index 2ce4c72720f3..2ce54561e82e 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -79,6 +79,11 @@ static unsigned long super_cache_scan(struct shrinker *shrink, > if (!trylock_super(sb)) > return SHRINK_STOP; > > + if ((sb->s_flags & (SB_BORN|SB_ACTIVE)) != (SB_BORN|SB_ACTIVE)) { > + freed = SHRINK_STOP; > + goto unlock; > + } This should not be here - the check to determine if the shrinker should run is done in the ->count method. If we removed the SB_ACTIVE flag between ->count and ->scan, then the superblock should be locked and the trylock_super() call above should fail.... Indeed, the unregister_shrinker() call in deactivate_locked_super() is done with the sb->s_umount held exclusively, and this happens before we clear SB_ACTIVE in the ->kill_sb() -> kill_block_super() -> generic_shutdown_super() path after the shrinker is unregistered. Hence we can't get to this check without SB_ACTIVE being set - the trylock will fail and then the shrinker_unregister() call will do it's thing to ensure the shrinker is never called again. If the change to the shrinker code allows the shrinker to still be actively running when we call ->kill_sb(), then that needs to be fixed. THe superblock shrinker must be stopped completely and never run again before we call ->kill_sb(). > if (sb->s_op->nr_cached_objects) > fs_objects = sb->s_op->nr_cached_objects(sb, sc); > > @@ -110,6 +115,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink, > freed += sb->s_op->free_cached_objects(sb, sc); > } > > +unlock: > up_read(&sb->s_umount); > return freed; > } > @@ -136,7 +142,7 @@ static unsigned long super_cache_count(struct shrinker *shrink, > * avoid this situation, so do the same here. The memory barrier is > * matched with the one in mount_fs() as we don't hold locks here. > */ > - if (!(sb->s_flags & SB_BORN)) > + if ((sb->s_flags & (SB_BORN|SB_ACTIVE)) != (SB_BORN|SB_ACTIVE)) > return 0; This is fine because it's an unlocked check, but I don't think it's actually necessary given the above. Indeed, if you are adding this, you need to expand the comment above on why SB_ACTIVE needs checking, and why the memory barrier doesn't actually apply to that part of the check.... -Dave.
diff --git a/fs/super.c b/fs/super.c index 2ce4c72720f3..2ce54561e82e 100644 --- a/fs/super.c +++ b/fs/super.c @@ -79,6 +79,11 @@ static unsigned long super_cache_scan(struct shrinker *shrink, if (!trylock_super(sb)) return SHRINK_STOP; + if ((sb->s_flags & (SB_BORN|SB_ACTIVE)) != (SB_BORN|SB_ACTIVE)) { + freed = SHRINK_STOP; + goto unlock; + } + if (sb->s_op->nr_cached_objects) fs_objects = sb->s_op->nr_cached_objects(sb, sc); @@ -110,6 +115,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink, freed += sb->s_op->free_cached_objects(sb, sc); } +unlock: up_read(&sb->s_umount); return freed; } @@ -136,7 +142,7 @@ static unsigned long super_cache_count(struct shrinker *shrink, * avoid this situation, so do the same here. The memory barrier is * matched with the one in mount_fs() as we don't hold locks here. */ - if (!(sb->s_flags & SB_BORN)) + if ((sb->s_flags & (SB_BORN|SB_ACTIVE)) != (SB_BORN|SB_ACTIVE)) return 0; smp_rmb();