All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Cc: Dave Chinner <david@fromorbit.com>, Martin Svec <martin.svec@zoner.cz>
Subject: [PATCH 3/3] [RFC] xfs: release buffer list after quotacheck buf reset
Date: Fri, 24 Feb 2017 14:53:21 -0500	[thread overview]
Message-ID: <1487966001-63263-4-git-send-email-bfoster@redhat.com> (raw)
In-Reply-To: <1487966001-63263-1-git-send-email-bfoster@redhat.com>

XXX: This patch is broken and should not be merged.

This purpose of this patch is to consider the notion of reducing
quotacheck memory consumption requirements via earlier release of
the buffer list. The intent is to allow dquot and buffer reclaim to
operate as normal during the subsequent bulkstat (dqadjust) and
dquot flush walk. In turn, this allows quotacheck to complete in
environments with tight memory constraints.

As it is, this approach has several unresolved tradeoffs/problems:

- The change is limited in effectiveness to situations where the
total dquot allocation requirements are the difference between OOM
and a successful mount. OOM is still possible as we still allocate
the entire dquot buffer space.

- In limited but non-OOM situations, this can cause quotacheck to
take significantly longer than normal. On a filesystem with ~4
million inodes and ~600k project quotas, a quotacheck typically runs
for ~50s in a local vm limited to 1GB RAM. With this change, the
same quotacheck requires anywhere from ~9-12m in my tests.

- By releasing the buffer list and allowing reclaim to do some of
the work, quotacheck has lost serialization against I/O completion
of the adjusted dquots. This creates the potential for corruption if
the filesystem crashes after quotacheck completion status has been
logged but before dquot I/O has fully completed. IOWs, release of
the buffer list as such requires the addition of a new, so far
undefined post-quotacheck serialization sequence.

One option here may be a post delwri submit walk of in-core dquots
purely to cycle the flush locks, the idea being that dquots can't be
fully reclaimed until the flush lock is unlocked and thus buffer I/O
has completed. Another option may be a post-quotacheck
xfs_wait_buftarg().

- This patch leads to some apparent buffer refcounting issues
(unrelated to the pushbuf bits). I reproduce _XBF_DELWRI_Q buffer
release time asserts and unmount hangs that need to be tracked down
and resolved. It is not yet clear whether these are generic problems
or require further changes in the quotacheck implementation to deal
with the fact that quotacheck can no longer assume exclusive access
to the dquot buffers.
---
 fs/xfs/xfs_qm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 3815ed3..b072495 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1267,10 +1267,9 @@ xfs_qm_flush_one(
 	 * cycle the flush lock.
 	 */
 	if (!xfs_dqflock_nowait(dqp)) {
-		/* buf is pinned in-core by delwri list */
-		DEFINE_SINGLE_BUF_MAP(map, dqp->q_blkno,
-				      mp->m_quotainfo->qi_dqchunklen);
-		bp = _xfs_buf_find(mp->m_ddev_targp, &map, 1, 0, NULL);
+		bp = xfs_buf_read(mp->m_ddev_targp, dqp->q_blkno,
+				  mp->m_quotainfo->qi_dqchunklen, 0,
+				  &xfs_dquot_buf_ops);
 		if (!bp) {
 			error = -EINVAL;
 			goto out_unlock;
@@ -1351,6 +1350,10 @@ xfs_qm_quotacheck(
 		flags |= XFS_PQUOTA_CHKD;
 	}
 
+	error = xfs_buf_delwri_submit(&buffer_list);
+	if (error)
+		goto error_return;
+
 	do {
 		/*
 		 * Iterate thru all the inodes in the file system,
-- 
2.7.4


      parent reply	other threads:[~2017-02-24 19:53 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-24 19:53 [PATCH v2 0/3] xfs: quotacheck vs. dquot reclaim deadlock Brian Foster
2017-02-24 19:53 ` [PATCH 1/3] xfs: fix up quotacheck buffer list error handling Brian Foster
2017-04-06 22:39   ` Darrick J. Wong
2017-04-07 12:02     ` Brian Foster
2017-04-07 18:20       ` Darrick J. Wong
2017-04-07 18:38         ` Brian Foster
2017-04-10  4:18   ` Dave Chinner
2017-04-10 14:13     ` Brian Foster
2017-02-24 19:53 ` [PATCH 2/3] xfs: push buffer of flush locked dquot to avoid quotacheck deadlock Brian Foster
2017-04-06 22:34   ` Darrick J. Wong
2017-04-07 12:06     ` Brian Foster
2017-04-07 20:07       ` Darrick J. Wong
2017-04-10 14:19         ` Brian Foster
2017-04-10  5:00   ` Dave Chinner
2017-04-10 14:15     ` Brian Foster
2017-04-10 23:55       ` Dave Chinner
2017-04-11 14:53         ` Brian Foster
2017-04-18  2:35           ` Dave Chinner
2017-04-18 13:55             ` Brian Foster
2017-04-19  2:46               ` Dave Chinner
2017-04-19 19:55                 ` Darrick J. Wong
2017-04-19 20:46                   ` Brian Foster
2017-04-21 12:18                     ` Brian Foster
2017-04-19 20:40                 ` Brian Foster
2017-04-11 12:50     ` Brian Foster
2017-02-24 19:53 ` Brian Foster [this message]

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=1487966001-63263-4-git-send-email-bfoster@redhat.com \
    --to=bfoster@redhat.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=martin.svec@zoner.cz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.