From patchwork Thu Feb 19 17:19:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konstantin Khlebnikov X-Patchwork-Id: 5853241 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A8294BF440 for ; Thu, 19 Feb 2015 17:20:08 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D65A420295 for ; Thu, 19 Feb 2015 17:20:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D6C082025B for ; Thu, 19 Feb 2015 17:20:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752732AbbBSRTl (ORCPT ); Thu, 19 Feb 2015 12:19:41 -0500 Received: from forward-corp1f.mail.yandex.net ([95.108.130.40]:49706 "EHLO forward-corp1f.mail.yandex.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751647AbbBSRTj (ORCPT ); Thu, 19 Feb 2015 12:19:39 -0500 Received: from smtpcorp1m.mail.yandex.net (smtpcorp1m.mail.yandex.net [77.88.61.150]) by forward-corp1f.mail.yandex.net (Yandex) with ESMTP id 17650242030F; Thu, 19 Feb 2015 20:19:35 +0300 (MSK) Received: from smtpcorp1m.mail.yandex.net (localhost [127.0.0.1]) by smtpcorp1m.mail.yandex.net (Yandex) with ESMTP id C98912CA03B4; Thu, 19 Feb 2015 20:19:35 +0300 (MSK) Received: from unknown (unknown [2a02:6b8:0:408:8c7a:604f:1217:9f6d]) by smtpcorp1m.mail.yandex.net (nwsmtp/Yandex) with ESMTPSA id 3q2EsvUoCP-JZ2OAp4H; Thu, 19 Feb 2015 20:19:35 +0300 (using TLSv1.2 with cipher AES128-SHA (128/128 bits)) (Client certificate not present) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex-team.ru; s=default; t=1424366375; bh=nV1wcqH1EIZQ6hisLWgYB32L8xMUed7P30gLiXom/JQ=; h=Subject:From:To:Cc:Date:Message-ID:User-Agent:MIME-Version: Content-Type:Content-Transfer-Encoding; b=wKDAzOBotiSw1DCgepvmyppWSM1MdUgFko1R+5O6+nIdIbikIqOkR5QZMyWRRYjOl m531VX+QbyZW1EvAwjecpPdZ3xRJzpzFmcTlAL9U07fQTz6D6KXdB1qqvJJHD5ViCM +qBuogSsnBhc+jFTST/diCPb7Zoh29vTcab9VPrY= Authentication-Results: smtpcorp1m.mail.yandex.net; dkim=pass header.i=@yandex-team.ru Subject: [PATCH] fs: avoid locking sb_lock in grab_super_passive() From: Konstantin Khlebnikov To: linux-mm@kvack.org, Andrew Morton , linux-kernel@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, Alexander Viro Date: Thu, 19 Feb 2015 20:19:35 +0300 Message-ID: <20150219171934.20458.30175.stgit@buzz> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP I've noticed significant locking contention in memory reclaimer around sb_lock inside grab_super_passive(). Grab_super_passive() is called from two places: in icache/dcache shrinkers (function super_cache_scan) and from writeback (function __writeback_inodes_wb). Both are required for progress in memory reclaimer. Also this lock isn't irq-safe. And I've seen suspicious livelock under serious memory pressure where reclaimer was called from interrupt which have happened right in place where sb_lock is held in normal context, so all other cpus were stuck on that lock too. Grab_super_passive() acquires sb_lock to increment sb->s_count and check sb->s_instances. It seems sb->s_umount locked for read is enough here: super-block deactivation always runs under sb->s_umount locked for write. Protecting super-block itself isn't a problem: in super_cache_scan() sb is protected by shrinker_rwsem: it cannot be freed if its slab shrinkers are still active. Inside writeback super-block comes from inode from bdi writeback list under wb->list_lock. This patch removes locking sb_lock and checks s_instances under s_umount: generic_shutdown_super() unlinks it under sb->s_umount locked for write. Now successful grab_super_passive() only locks semaphore, callers must call up_read(&sb->s_umount) instead of drop_super(sb) when they're done. Signed-off-by: Konstantin Khlebnikov --- fs/fs-writeback.c | 2 +- fs/super.c | 18 ++++-------------- 2 files changed, 5 insertions(+), 15 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 073657f..3e92bb7 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -779,7 +779,7 @@ static long __writeback_inodes_wb(struct bdi_writeback *wb, continue; } wrote += writeback_sb_inodes(sb, wb, work); - drop_super(sb); + up_read(&sb->s_umount); /* refer to the same tests at the end of writeback_sb_inodes */ if (wrote) { diff --git a/fs/super.c b/fs/super.c index 65a53ef..6ae33ed 100644 --- a/fs/super.c +++ b/fs/super.c @@ -105,7 +105,7 @@ static unsigned long super_cache_scan(struct shrinker *shrink, freed += sb->s_op->free_cached_objects(sb, sc); } - drop_super(sb); + up_read(&sb->s_umount); return freed; } @@ -356,27 +356,17 @@ static int grab_super(struct super_block *s) __releases(sb_lock) * superblock does not go away while we are working on it. It returns * false if a reference was not gained, and returns true with the s_umount * lock held in read mode if a reference is gained. On successful return, - * the caller must drop the s_umount lock and the passive reference when - * done. + * the caller must drop the s_umount lock when done. */ bool grab_super_passive(struct super_block *sb) { - spin_lock(&sb_lock); - if (hlist_unhashed(&sb->s_instances)) { - spin_unlock(&sb_lock); - return false; - } - - sb->s_count++; - spin_unlock(&sb_lock); - if (down_read_trylock(&sb->s_umount)) { - if (sb->s_root && (sb->s_flags & MS_BORN)) + if (!hlist_unhashed(&sb->s_instances) && + sb->s_root && (sb->s_flags & MS_BORN)) return true; up_read(&sb->s_umount); } - put_super(sb); return false; }