From: Amir Goldstein <amir73il@gmail.com>
To: "Darrick J . Wong" <djwong@kernel.org>
Cc: Leah Rumancik <leah.rumancik@gmail.com>,
Chandan Babu R <chandan.babu@oracle.com>,
linux-xfs@vger.kernel.org, fstests@vger.kernel.org,
Brian Foster <bfoster@redhat.com>,
Dave Chinner <dchinner@redhat.com>
Subject: [PATCH 5.10 CANDIDATE 1/7] xfs: remove infinite loop when reserving free block pool
Date: Sun, 28 Aug 2022 15:46:08 +0300 [thread overview]
Message-ID: <20220828124614.2190592-2-amir73il@gmail.com> (raw)
In-Reply-To: <20220828124614.2190592-1-amir73il@gmail.com>
commit 15f04fdc75aaaa1cccb0b8b3af1be290e118a7bc upstream.
[Added wrapper xfs_fdblocks_unavailable() for 5.10.y backport]
Infinite loops in kernel code are scary. Calls to xfs_reserve_blocks
should be rare (people should just use the defaults!) so we really don't
need to try so hard. Simplify the logic here by removing the infinite
loop.
Cc: Brian Foster <bfoster@redhat.com>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/xfs/xfs_fsops.c | 52 +++++++++++++++++++---------------------------
fs/xfs/xfs_mount.h | 8 +++++++
2 files changed, 29 insertions(+), 31 deletions(-)
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index ef1d5bb88b93..6d4f4271e7be 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -376,46 +376,36 @@ xfs_reserve_blocks(
* If the request is larger than the current reservation, reserve the
* blocks before we update the reserve counters. Sample m_fdblocks and
* perform a partial reservation if the request exceeds free space.
+ *
+ * The code below estimates how many blocks it can request from
+ * fdblocks to stash in the reserve pool. This is a classic TOCTOU
+ * race since fdblocks updates are not always coordinated via
+ * m_sb_lock.
*/
- error = -ENOSPC;
- do {
- free = percpu_counter_sum(&mp->m_fdblocks) -
- mp->m_alloc_set_aside;
- if (free <= 0)
- break;
-
- delta = request - mp->m_resblks;
- lcounter = free - delta;
- if (lcounter < 0)
- /* We can't satisfy the request, just get what we can */
- fdblks_delta = free;
- else
- fdblks_delta = delta;
-
+ free = percpu_counter_sum(&mp->m_fdblocks) -
+ xfs_fdblocks_unavailable(mp);
+ delta = request - mp->m_resblks;
+ if (delta > 0 && free > 0) {
/*
* We'll either succeed in getting space from the free block
- * count or we'll get an ENOSPC. If we get a ENOSPC, it means
- * things changed while we were calculating fdblks_delta and so
- * we should try again to see if there is anything left to
- * reserve.
- *
- * Don't set the reserved flag here - we don't want to reserve
- * the extra reserve blocks from the reserve.....
+ * count or we'll get an ENOSPC. Don't set the reserved flag
+ * here - we don't want to reserve the extra reserve blocks
+ * from the reserve.
*/
+ fdblks_delta = min(free, delta);
spin_unlock(&mp->m_sb_lock);
error = xfs_mod_fdblocks(mp, -fdblks_delta, 0);
spin_lock(&mp->m_sb_lock);
- } while (error == -ENOSPC);
- /*
- * Update the reserve counters if blocks have been successfully
- * allocated.
- */
- if (!error && fdblks_delta) {
- mp->m_resblks += fdblks_delta;
- mp->m_resblks_avail += fdblks_delta;
+ /*
+ * Update the reserve counters if blocks have been successfully
+ * allocated.
+ */
+ if (!error) {
+ mp->m_resblks += fdblks_delta;
+ mp->m_resblks_avail += fdblks_delta;
+ }
}
-
out:
if (outval) {
outval->resblks = mp->m_resblks;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index dfa429b77ee2..3a6bc9dc11b5 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -406,6 +406,14 @@ extern int xfs_initialize_perag(xfs_mount_t *mp, xfs_agnumber_t agcount,
xfs_agnumber_t *maxagi);
extern void xfs_unmountfs(xfs_mount_t *);
+/* Accessor added for 5.10.y backport */
+static inline uint64_t
+xfs_fdblocks_unavailable(
+ struct xfs_mount *mp)
+{
+ return mp->m_alloc_set_aside;
+}
+
extern int xfs_mod_fdblocks(struct xfs_mount *mp, int64_t delta,
bool reserved);
extern int xfs_mod_frextents(struct xfs_mount *mp, int64_t delta);
--
2.25.1
next prev parent reply other threads:[~2022-08-28 12:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-28 12:46 [PATCH 5.10 CANDIDATE 0/7] xfs stable candidate patches for 5.10.y (from v5.18+) Amir Goldstein
2022-08-28 12:46 ` Amir Goldstein [this message]
2022-08-28 12:46 ` [PATCH 5.10 CANDIDATE 2/7] xfs: always succeed at setting the reserve pool size Amir Goldstein
2022-08-28 12:46 ` [PATCH 5.10 CANDIDATE 3/7] xfs: fix overfilling of reserve pool Amir Goldstein
2022-08-28 12:46 ` [PATCH 5.10 CANDIDATE 4/7] xfs: fix soft lockup via spinning in filestream ag selection loop Amir Goldstein
2022-08-28 12:46 ` [PATCH 5.10 CANDIDATE 5/7] xfs: revert "xfs: actually bump warning counts when we send warnings" Amir Goldstein
2022-08-28 12:46 ` [PATCH 5.10 CANDIDATE 6/7] xfs: reorder iunlink remove operation in xfs_ifree Amir Goldstein
2022-08-28 12:46 ` [PATCH 5.10 CANDIDATE 7/7] xfs: validate inode fork size against fork format Amir Goldstein
2022-08-29 14:21 ` [PATCH 5.10 CANDIDATE 0/7] xfs stable candidate patches for 5.10.y (from v5.18+) Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220828124614.2190592-2-amir73il@gmail.com \
--to=amir73il@gmail.com \
--cc=bfoster@redhat.com \
--cc=chandan.babu@oracle.com \
--cc=dchinner@redhat.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=leah.rumancik@gmail.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).