[PATCH] xfs: prevent quotacheck from overloading inode lru

[PATCH] xfs: prevent quotacheck from overloading inode lru

[Brian Foster]

Quotacheck runs at mount time in situations where quota accounting must
be recalculated. In doing so, it uses bulkstat to visit every inode in
the filesystem. Historically, every inode processed during quotacheck
was released and immediately tagged for reclaim because quotacheck runs
before the superblock is marked active by the VFS. In other words,
the final iput() lead to an immediate ->destroy_inode() call, which
allowed the XFS background reclaim worker to start reclaiming inodes.

Commit 17c12bcd3 ("xfs: when replaying bmap operations, don't let
unlinked inodes get reaped") marks the XFS superblock active sooner as
part of the mount process to support caching inodes processed during log
recovery. This occurs before quotacheck and thus means all inodes
processed by quotacheck are inserted to the LRU on release.  The
s_umount lock is held until the mount has completed and thus prevents
the shrinkers from operating on the sb. This means that quotacheck can
excessively populate the inode LRU and lead to OOM conditions on systems
without sufficient RAM.

Update the quotacheck bulkstat handler to set XFS_IGET_DONTCACHE on
inodes processed by quotacheck. This causes ->drop_inode() to return 1
and in turn causes iput_final() to evict the inode. This preserves the
original quotacheck behavior and prevents it from overloading the LRU
and running out of memory.

CC: stable@vger.kernel.org # v4.9
Reported-by: Martin Svec <martin.svec@zoner.cz>
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_qm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 45e50ea..b669b12 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1177,7 +1177,8 @@ xfs_qm_dqusage_adjust(
 	 * the case in all other instances. It's OK that we do this because
 	 * quotacheck is done only at mount time.
 	 */
-	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_EXCL, &ip);
+	error = xfs_iget(mp, NULL, ino, XFS_IGET_DONTCACHE, XFS_ILOCK_EXCL,
+			 &ip);
 	if (error) {
 		*res = BULKSTAT_RV_NOTHING;
 		return error;
-- 
2.7.4

Re: [PATCH] xfs: prevent quotacheck from overloading inode lru

[Eric Sandeen]

On 1/26/17 1:26 PM, Brian Foster wrote:
> Quotacheck runs at mount time in situations where quota accounting must
> be recalculated. In doing so, it uses bulkstat to visit every inode in
> the filesystem. Historically, every inode processed during quotacheck
> was released and immediately tagged for reclaim because quotacheck runs
> before the superblock is marked active by the VFS. In other words,
> the final iput() lead to an immediate ->destroy_inode() call, which
> allowed the XFS background reclaim worker to start reclaiming inodes.
> 
> Commit 17c12bcd3 ("xfs: when replaying bmap operations, don't let
> unlinked inodes get reaped") marks the XFS superblock active sooner as
> part of the mount process to support caching inodes processed during log
> recovery. This occurs before quotacheck and thus means all inodes
> processed by quotacheck are inserted to the LRU on release.  The
> s_umount lock is held until the mount has completed and thus prevents
> the shrinkers from operating on the sb. This means that quotacheck can
> excessively populate the inode LRU and lead to OOM conditions on systems
> without sufficient RAM.
> 
> Update the quotacheck bulkstat handler to set XFS_IGET_DONTCACHE on
> inodes processed by quotacheck. This causes ->drop_inode() to return 1
> and in turn causes iput_final() to evict the inode. This preserves the
> original quotacheck behavior and prevents it from overloading the LRU
> and running out of memory.
> 
> CC: stable@vger.kernel.org # v4.9
> Reported-by: Martin Svec <martin.svec@zoner.cz>
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Makes sense, thanks for digging into this!

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/xfs_qm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 45e50ea..b669b12 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1177,7 +1177,8 @@ xfs_qm_dqusage_adjust(
>  	 * the case in all other instances. It's OK that we do this because
>  	 * quotacheck is done only at mount time.
>  	 */
> -	error = xfs_iget(mp, NULL, ino, 0, XFS_ILOCK_EXCL, &ip);
> +	error = xfs_iget(mp, NULL, ino, XFS_IGET_DONTCACHE, XFS_ILOCK_EXCL,
> +			 &ip);
>  	if (error) {
>  		*res = BULKSTAT_RV_NOTHING;
>  		return error;
>