From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: linux-xfs@vger.kernel.org
Cc: viro@ZenIV.linux.org.uk, Christoph Hellwig <hch@infradead.org>
Subject: [PATCH v2 2/3] xfs: evict all inodes involved with log redo item recovery
Date: Fri, 11 Aug 2017 12:50:10 -0700 [thread overview]
Message-ID: <20170811195010.GH24087@magnolia> (raw)
In-Reply-To: <150234263191.13754.5037661818451431724.stgit@magnolia>
When we introduced the bmap redo log items, we set MS_ACTIVE on the
mountpoint and XFS_IRECOVERY on the inode to prevent unlinked inodes
from being truncated prematurely during log recovery. This also had the
effect of putting linked inodes on the lru instead of evicting them.
Unfortunately, we neglected to find all those unreferenced lru inodes
and evict them after finishing log recovery, which means that we leak
them if anything goes wrong in the rest of xfs_mountfs, because the lru
is only cleaned out on unmount.
Therefore, evict unreferenced inodes in the lru list immediately
after clearing MS_ACTIVE.
Fixes: 17c12bcd30 ("xfs: when replaying bmap operations, don't let unlinked inodes get reaped")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Cc: viro@ZenIV.linux.org.uk
---
v2: use the vfs evict_inodes instead of duplicating it
---
fs/inode.c | 1 +
fs/internal.h | 1 -
fs/xfs/xfs_mount.c | 12 ++++++++++++
include/linux/fs.h | 1 +
4 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/fs/inode.c b/fs/inode.c
index 5037059..6a1626e 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -637,6 +637,7 @@ void evict_inodes(struct super_block *sb)
dispose_list(&dispose);
}
+EXPORT_SYMBOL_GPL(evict_inodes);
/**
* invalidate_inodes - attempt to free all inodes on a superblock
diff --git a/fs/internal.h b/fs/internal.h
index 9676fe1..fedfe94 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -132,7 +132,6 @@ static inline bool atime_needs_update_rcu(const struct path *path,
extern void inode_io_list_del(struct inode *inode);
extern long get_nr_dirty_inodes(void);
-extern void evict_inodes(struct super_block *);
extern int invalidate_inodes(struct super_block *, bool);
/*
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index a46c9d7..351e2c3 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -956,10 +956,22 @@ xfs_mountfs(
* inodes. Turn it off immediately after xfs_log_mount_finish
* so that we don't leak the quota inodes if subsequent mount
* activities fail.
+ *
+ * We let all inodes involved in redo item processing end up on
+ * the LRU instead of being evicted immediately so that if we do
+ * something to an unlinked inode, the irele won't cause
+ * premature truncation and freeing of the inode, which results
+ * in log recovery failure. We have to evict the unreferenced
+ * lru inodes after clearing MS_ACTIVE because we don't
+ * otherwise clean up the lru if there's a subsequent failure in
+ * xfs_mountfs, which leads to us leaking the inodes if nothing
+ * else (e.g. quotacheck) references the inodes before the
+ * mount failure occurs.
*/
mp->m_super->s_flags |= MS_ACTIVE;
error = xfs_log_mount_finish(mp);
mp->m_super->s_flags &= ~MS_ACTIVE;
+ evict_inodes(mp->m_super);
if (error) {
xfs_warn(mp, "log mount finish failed");
goto out_rtunmount;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7b5d681..e730438 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2830,6 +2830,7 @@ static inline void lockdep_annotate_inode_mutex_key(struct inode *inode) { };
#endif
extern void unlock_new_inode(struct inode *);
extern unsigned int get_next_ino(void);
+extern void evict_inodes(struct super_block *sb);
extern void __iget(struct inode * inode);
extern void iget_failed(struct inode *);
next prev parent reply other threads:[~2017-08-11 19:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-10 5:23 [PATCH 1/3] xfs: clear MS_ACTIVE after finishing log recovery to avoid inode leak Darrick J. Wong
2017-08-10 5:23 ` [PATCH 2/3] xfs: evict all inodes involved with log redo item recovery Darrick J. Wong
2017-08-10 14:51 ` Brian Foster
2017-08-10 17:18 ` Darrick J. Wong
2017-08-10 17:54 ` Brian Foster
2017-08-11 11:22 ` Christoph Hellwig
2017-08-11 16:51 ` Darrick J. Wong
2017-08-11 19:50 ` Darrick J. Wong [this message]
2017-08-11 23:42 ` [PATCH v2 " Dave Chinner
2017-08-11 23:59 ` Darrick J. Wong
2017-08-10 5:23 ` [PATCH 3/3] xfs: don't leak quotacheck dquots when cow recovery fails Darrick J. Wong
2017-08-10 14:51 ` Brian Foster
2017-08-11 11:19 ` Christoph Hellwig
2017-08-11 19:48 ` [PATCH v3 " Darrick J. Wong
2017-08-14 12:40 ` Brian Foster
2017-08-10 18:15 ` [PATCH 1/3] xfs: clear MS_ACTIVE after finishing log recovery to avoid inode leak Allison Henderson
2017-08-11 11:13 ` Christoph Hellwig
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=20170811195010.GH24087@magnolia \
--to=darrick.wong@oracle.com \
--cc=hch@infradead.org \
--cc=linux-xfs@vger.kernel.org \
--cc=viro@ZenIV.linux.org.uk \
/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