linux-ext4 - [PATCH 6/6] XFS: remove congestion_wait() loop from xfs_buf_alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 17 Sep 2021 12:56:57 +1000
From:   NeilBrown <neilb@...e.de>
To:     Andrew Morton <akpm@...ux-foundation.org>,
        Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        "Darrick J. Wong" <djwong@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Mel Gorman <mgorman@...e.de>, Michal Hocko <mhocko@...e.com>,
        ". Dave Chinner" <david@...morbit.com>,
        Jonathan Corbet <corbet@....net>
Cc:     linux-xfs@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-nfs@...r.kernel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: [PATCH 6/6] XFS: remove congestion_wait() loop from
 xfs_buf_alloc_pages()

Documentation commment in gfp.h discourages indefinite retry loops on
ENOMEM and says of __GFP_NOFAIL that it

    is definitely preferable to use the flag rather than opencode
    endless loop around allocator.

congestion_wait() is indistinguishable from
schedule_timeout_uninterruptible() in practice and it is not a good way
to wait for memory to become available.

So add __GFP_NOFAIL to gfp if failure is not an option, and remove the
congestion_wait().  We now only loop when failure is an option, and
alloc_bulk_pages_array() made some progres, but not enough.

Signed-off-by: NeilBrown <neilb@...e.de>
---
 fs/xfs/xfs_buf.c |   14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 5fa6cd947dd4..b19ab52c551b 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -352,7 +352,7 @@ xfs_buf_alloc_pages(
 	if (flags & XBF_READ_AHEAD)
 		gfp_mask |= __GFP_NORETRY;
 	else
-		gfp_mask |= GFP_NOFS;
+		gfp_mask |= GFP_NOFS | __GFP_NOFAIL;
 
 	/* Make sure that we have a page list */
 	bp->b_page_count = DIV_ROUND_UP(BBTOB(bp->b_length), PAGE_SIZE);
@@ -372,8 +372,9 @@ xfs_buf_alloc_pages(
 
 	/*
 	 * Bulk filling of pages can take multiple calls. Not filling the entire
-	 * array is not an allocation failure, so don't back off if we get at
-	 * least one extra page.
+	 * array is not an allocation failure but is worth counting in
+	 * xb_pages_retries statistics.  If we don't even get one page,
+	 * then this must be a READ_AHEAD and we should abort.
 	 */
 	for (;;) {
 		long	last = filled;
@@ -385,16 +386,13 @@ xfs_buf_alloc_pages(
 			break;
 		}
 
-		if (filled != last)
-			continue;
-
-		if (flags & XBF_READ_AHEAD) {
+		if (filled == last) {
+			ASSERT(flags & XBF_READ_AHEAD);
 			xfs_buf_free_pages(bp);
 			return -ENOMEM;
 		}
 
 		XFS_STATS_INC(bp->b_mount, xb_page_retries);
-		congestion_wait(BLK_RW_ASYNC, HZ / 50);
 	}
 	return 0;
 }