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=-23.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL 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 3DD4FC4338F for ; Wed, 11 Aug 2021 03:06:38 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CE01D60EC0 for ; Wed, 11 Aug 2021 03:06:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CE01D60EC0 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 103156B0071; Tue, 10 Aug 2021 23:06:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0B4D06B0072; Tue, 10 Aug 2021 23:06:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EE4456B0073; Tue, 10 Aug 2021 23:06:36 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0138.hostedemail.com [216.40.44.138]) by kanga.kvack.org (Postfix) with ESMTP id D4A206B0071 for ; Tue, 10 Aug 2021 23:06:36 -0400 (EDT) Received: from smtpin34.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 7FC401F872 for ; Wed, 11 Aug 2021 03:06:36 +0000 (UTC) X-FDA: 78461311992.34.5B2AC24 Received: from mail-oi1-f171.google.com (mail-oi1-f171.google.com [209.85.167.171]) by imf04.hostedemail.com (Postfix) with ESMTP id 2858F500C865 for ; Wed, 11 Aug 2021 03:06:36 +0000 (UTC) Received: by mail-oi1-f171.google.com with SMTP id be20so2426180oib.8 for ; Tue, 10 Aug 2021 20:06:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:in-reply-to:message-id:references :mime-version; bh=yvXblaEhrLHud7IBPtFUbb5wiOL8eqhEzjzPSEArjFU=; b=hZrFuqJRZ0p++5SxTE7qie0w7HKtw9vS4l/RLL9/zaWysr1c2tKd0cHVI1OeslNuDq o7XfrvjAvQpF8iQr21wQG1vNYwWb2oZZgGJOj3u1jHEaLBeaS7d0QtVPVzF9asNNHXkl Eyg7vZCXL/yeaa6ATznx2RI8XyWcPhgi2bL2LLNahlruFriwDxPwCRycVRs1bKO2pv/D S6MZCwMxv8iEMURX56t1EtV6BACNTtnJIC9WlxgztGdY1tBClcNdl5Duj8Z7jdb/FNLg Wzh2qG8HO0nOAuB+gLilBzgJ076FYVAjOIGkUZ2bv1chAPgd0UHeFsyLMW2Qy6bePet5 W92w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:in-reply-to:message-id :references:mime-version; bh=yvXblaEhrLHud7IBPtFUbb5wiOL8eqhEzjzPSEArjFU=; b=mmKe0HLfg/tWKaqJHhnSrEymWdjX8rO7ZC6oibYa8NRlahRXCvFKqasK7JKPypZSsA GLPnXzxQil5DfGoIzM4zZnmEzuChsRmFJ7Dm6rGSx77f1VCzHMzwpJFFK/neRZtzBdf/ KQUF9eZLM2dJ0TksVSwVjYlPZjIWDHgI52vXFuB1xHwMN9pwAFjq+PgOOx1lTNOIP8tl DIfnGwL2Fn+8hm3UxsGgbSSAdz8c+W4s2cNgF/Cm3J/+s3c59iGC9pgJ+zzkIhtdJLov QgeVI9WV74jnbDZzxCvg+kaZKc1qgA78T0avR8Os/BwMERA+JvEW/e3zpzpxK7jJjt// on2A== X-Gm-Message-State: AOAM532pv0xqAo2OqcxNhEyk30YoK935Rm389SOJ4nLRXmHBz+YnylJa PD37PIB9i7ZR4fqQQNwmiR2vVg== X-Google-Smtp-Source: ABdhPJwm09ozIZyMJjb1MSiuyfVdGcz5vJzk3MsBGHpRRCwCneM3v6zYzUK065S5Tksp83npg84Yxw== X-Received: by 2002:aca:c416:: with SMTP id u22mr16026197oif.71.1628651195325; Tue, 10 Aug 2021 20:06:35 -0700 (PDT) Received: from ripple.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id r137sm3645139oie.17.2021.08.10.20.06.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Aug 2021 20:06:34 -0700 (PDT) Date: Tue, 10 Aug 2021 20:06:23 -0700 (PDT) From: Hugh Dickins X-X-Sender: hugh@ripple.anvils To: Sebastian Andrzej Siewior cc: linux-mm@kvack.org, Hugh Dickins , Andrew Morton , Thomas Gleixner Subject: Re: [PATCH] shmem: Use raw_spinlock_t for ->stat_lock In-Reply-To: <20210806142916.jdwkb5bx62q5fwfo@linutronix.de> Message-ID: References: <20210806142916.jdwkb5bx62q5fwfo@linutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 2858F500C865 Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=hZrFuqJR; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf04.hostedemail.com: domain of hughd@google.com designates 209.85.167.171 as permitted sender) smtp.mailfrom=hughd@google.com X-Stat-Signature: hd8u4i8h1wjy1p6549s7c4bsa91xz3np X-HE-Tag: 1628651196-908666 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 Fri, 6 Aug 2021, Sebastian Andrzej Siewior wrote: > Each CPU has SHMEM_INO_BATCH inodes available in `->ino_batch' which is > per-CPU. Access here is serialized by disabling preemption. If the pool is > empty, it gets reloaded from `->next_ino'. Access here is serialized by > ->stat_lock which is a spinlock_t and can not be acquired with disabled > preemption. > One way around it would make per-CPU ino_batch struct containing the inode > number a local_lock_t. > Another solution is to promote ->stat_lock to a raw_spinlock_t. The critical > sections are short. The mpol_put() must be moved outside of the critical > section to avoid invoking the destructor with disabled preemption. > > Signed-off-by: Sebastian Andrzej Siewior Acked-by: Hugh Dickins I was a little worried at first by the use in shmem_reconfigure(), potentially around a percpu_counter_compare(), which in some cases does a sum across all cpus - not as short a critical section as the rest of them. But I see that __percpu_counter_sum() uses a raw_spinlock_t itself, and has done so for twelve years: if that one is good, then this use here must be good too. Thanks. > --- > include/linux/shmem_fs.h | 2 +- > mm/shmem.c | 31 +++++++++++++++++-------------- > 2 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h > index 8e775ce517bb3..0a8499fb9c3cb 100644 > --- a/include/linux/shmem_fs.h > +++ b/include/linux/shmem_fs.h > @@ -31,7 +31,7 @@ struct shmem_sb_info { > struct percpu_counter used_blocks; /* How many are allocated */ > unsigned long max_inodes; /* How many inodes are allowed */ > unsigned long free_inodes; /* How many are left for allocation */ > - spinlock_t stat_lock; /* Serialize shmem_sb_info changes */ > + raw_spinlock_t stat_lock; /* Serialize shmem_sb_info changes */ > umode_t mode; /* Mount mode for root directory */ > unsigned char huge; /* Whether to try for hugepages */ > kuid_t uid; /* Mount uid for root directory */ > diff --git a/mm/shmem.c b/mm/shmem.c > index 70d9ce294bb49..1b95afc4483c1 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -278,10 +278,10 @@ static int shmem_reserve_inode(struct super_block *sb, ino_t *inop) > ino_t ino; > > if (!(sb->s_flags & SB_KERNMOUNT)) { > - spin_lock(&sbinfo->stat_lock); > + raw_spin_lock(&sbinfo->stat_lock); > if (sbinfo->max_inodes) { > if (!sbinfo->free_inodes) { > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > return -ENOSPC; > } > sbinfo->free_inodes--; > @@ -304,7 +304,7 @@ static int shmem_reserve_inode(struct super_block *sb, ino_t *inop) > } > *inop = ino; > } > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > } else if (inop) { > /* > * __shmem_file_setup, one of our callers, is lock-free: it > @@ -319,13 +319,14 @@ static int shmem_reserve_inode(struct super_block *sb, ino_t *inop) > * to worry about things like glibc compatibility. > */ > ino_t *next_ino; > + > next_ino = per_cpu_ptr(sbinfo->ino_batch, get_cpu()); > ino = *next_ino; > if (unlikely(ino % SHMEM_INO_BATCH == 0)) { > - spin_lock(&sbinfo->stat_lock); > + raw_spin_lock(&sbinfo->stat_lock); > ino = sbinfo->next_ino; > sbinfo->next_ino += SHMEM_INO_BATCH; > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > if (unlikely(is_zero_ino(ino))) > ino++; > } > @@ -341,9 +342,9 @@ static void shmem_free_inode(struct super_block *sb) > { > struct shmem_sb_info *sbinfo = SHMEM_SB(sb); > if (sbinfo->max_inodes) { > - spin_lock(&sbinfo->stat_lock); > + raw_spin_lock(&sbinfo->stat_lock); > sbinfo->free_inodes++; > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > } > } > > @@ -1453,10 +1454,10 @@ static struct mempolicy *shmem_get_sbmpol(struct shmem_sb_info *sbinfo) > { > struct mempolicy *mpol = NULL; > if (sbinfo->mpol) { > - spin_lock(&sbinfo->stat_lock); /* prevent replace/use races */ > + raw_spin_lock(&sbinfo->stat_lock); /* prevent replace/use races */ > mpol = sbinfo->mpol; > mpol_get(mpol); > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > } > return mpol; > } > @@ -3500,9 +3501,10 @@ static int shmem_reconfigure(struct fs_context *fc) > struct shmem_options *ctx = fc->fs_private; > struct shmem_sb_info *sbinfo = SHMEM_SB(fc->root->d_sb); > unsigned long inodes; > + struct mempolicy *mpol = NULL; > const char *err; > > - spin_lock(&sbinfo->stat_lock); > + raw_spin_lock(&sbinfo->stat_lock); > inodes = sbinfo->max_inodes - sbinfo->free_inodes; > if ((ctx->seen & SHMEM_SEEN_BLOCKS) && ctx->blocks) { > if (!sbinfo->max_blocks) { > @@ -3547,14 +3549,15 @@ static int shmem_reconfigure(struct fs_context *fc) > * Preserve previous mempolicy unless mpol remount option was specified. > */ > if (ctx->mpol) { > - mpol_put(sbinfo->mpol); > + mpol = sbinfo->mpol; > sbinfo->mpol = ctx->mpol; /* transfers initial ref */ > ctx->mpol = NULL; > } > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > + mpol_put(mpol); > return 0; > out: > - spin_unlock(&sbinfo->stat_lock); > + raw_spin_unlock(&sbinfo->stat_lock); > return invalfc(fc, "%s", err); > } > > @@ -3671,7 +3674,7 @@ static int shmem_fill_super(struct super_block *sb, struct fs_context *fc) > sbinfo->mpol = ctx->mpol; > ctx->mpol = NULL; > > - spin_lock_init(&sbinfo->stat_lock); > + raw_spin_lock_init(&sbinfo->stat_lock); > if (percpu_counter_init(&sbinfo->used_blocks, 0, GFP_KERNEL)) > goto failed; > spin_lock_init(&sbinfo->shrinklist_lock); > -- > 2.32.0 > >