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@xxxxxxx>
---
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;
}
