[PATCH v2] netfs: fix writeback ENOMEM by using __GFP_NOFAIL for rolling buffer

Yun Zhou posted 1 patch 13 hours ago
fs/netfs/buffered_read.c       | 4 ++--
fs/netfs/rolling_buffer.c      | 4 ++--
fs/netfs/write_issue.c         | 7 ++++++-
include/linux/rolling_buffer.h | 2 +-
4 files changed, 11 insertions(+), 6 deletions(-)
[PATCH v2] netfs: fix writeback ENOMEM by using __GFP_NOFAIL for rolling buffer
Posted by Yun Zhou 13 hours ago
rolling_buffer_init() uses plain GFP_NOFS for its folio_queue allocation.
In the writeback path this can fail and trigger WARN_ON_ONCE(folio != NULL)
in netfs_writepages() when writeback_iter() returns additional dirty folios
left unhandled.

Writeback must not fail with -ENOMEM. Fix this by adding a gfp_t parameter
to rolling_buffer_init() and passing GFP_NOFS | __GFP_NOFAIL from the
writeback path, ensuring the allocation always succeeds. Read paths
continue to use plain GFP_NOFS.

Reported-by: syzbot+0da43efa72f88bd3a8af@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0da43efa72f88bd3a8af
Fixes: ac5f95ac5d6d ("netfs: Fix writeback error handling")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
Changes in v2:
  - Dropped the writeback_iter drain loop approach (v1) per review feedback
    from Christoph Hellwig and David Howells.
  - Instead, fix the root cause: use __GFP_NOFAIL for rolling_buffer_init()
    in the writeback path so that -ENOMEM cannot occur.
  - Added gfp_t parameter to rolling_buffer_init() to allow writeback and
    read paths to use different allocation flags.
---
 fs/netfs/buffered_read.c       | 4 ++--
 fs/netfs/rolling_buffer.c      | 4 ++--
 fs/netfs/write_issue.c         | 7 ++++++-
 include/linux/rolling_buffer.h | 2 +-
 4 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/netfs/buffered_read.c b/fs/netfs/buffered_read.c
index 24a8a5418e31..fe84e1dd707c 100644
--- a/fs/netfs/buffered_read.c
+++ b/fs/netfs/buffered_read.c
@@ -359,7 +359,7 @@ void netfs_readahead(struct readahead_control *ractl)
 	netfs_rreq_expand(rreq, ractl);
 
 	rreq->submitted = rreq->start;
-	if (rolling_buffer_init(&rreq->buffer, rreq->debug_id, ITER_DEST) < 0)
+	if (rolling_buffer_init(&rreq->buffer, rreq->debug_id, ITER_DEST, GFP_NOFS) < 0)
 		goto cleanup_free;
 	netfs_read_to_pagecache(rreq, ractl);
 
@@ -378,7 +378,7 @@ static int netfs_create_singular_buffer(struct netfs_io_request *rreq, struct fo
 {
 	ssize_t added;
 
-	if (rolling_buffer_init(&rreq->buffer, rreq->debug_id, ITER_DEST) < 0)
+	if (rolling_buffer_init(&rreq->buffer, rreq->debug_id, ITER_DEST, GFP_NOFS) < 0)
 		return -ENOMEM;
 
 	added = rolling_buffer_append(&rreq->buffer, folio, rollbuf_flags);
diff --git a/fs/netfs/rolling_buffer.c b/fs/netfs/rolling_buffer.c
index a17fbf9853a4..3c4b1e244c46 100644
--- a/fs/netfs/rolling_buffer.c
+++ b/fs/netfs/rolling_buffer.c
@@ -60,11 +60,11 @@ EXPORT_SYMBOL(netfs_folioq_free);
  * consumer.
  */
 int rolling_buffer_init(struct rolling_buffer *roll, unsigned int rreq_id,
-			unsigned int direction)
+			unsigned int direction, gfp_t gfp)
 {
 	struct folio_queue *fq;
 
-	fq = netfs_folioq_alloc(rreq_id, GFP_NOFS, netfs_trace_folioq_rollbuf_init);
+	fq = netfs_folioq_alloc(rreq_id, gfp, netfs_trace_folioq_rollbuf_init);
 	if (!fq)
 		return -ENOMEM;
 
diff --git a/fs/netfs/write_issue.c b/fs/netfs/write_issue.c
index f2761c99795a..28bcd10ef330 100644
--- a/fs/netfs/write_issue.c
+++ b/fs/netfs/write_issue.c
@@ -98,6 +98,7 @@ struct netfs_io_request *netfs_create_write_req(struct address_space *mapping,
 			     origin == NETFS_WRITEBACK_SINGLE ||
 			     origin == NETFS_WRITETHROUGH ||
 			     origin == NETFS_PGPRIV2_COPY_TO_CACHE);
+	gfp_t gfp = GFP_NOFS;
 
 	wreq = netfs_alloc_request(mapping, file, start, 0, origin);
 	if (IS_ERR(wreq))
@@ -108,7 +109,11 @@ struct netfs_io_request *netfs_create_write_req(struct address_space *mapping,
 	ictx = netfs_inode(wreq->inode);
 	if (is_cacheable)
 		fscache_begin_write_operation(&wreq->cache_resources, netfs_i_cookie(ictx));
-	if (rolling_buffer_init(&wreq->buffer, wreq->debug_id, ITER_SOURCE) < 0)
+
+	/* Writeback is part of memory reclaim and must not fail due to ENOMEM. */
+	if (origin == NETFS_WRITEBACK || origin == NETFS_WRITEBACK_SINGLE)
+		gfp |= __GFP_NOFAIL;
+	if (rolling_buffer_init(&wreq->buffer, wreq->debug_id, ITER_SOURCE, gfp) < 0)
 		goto nomem;
 
 	wreq->cleaned_to = wreq->start;
diff --git a/include/linux/rolling_buffer.h b/include/linux/rolling_buffer.h
index ac15b1ffdd83..39b7248838e2 100644
--- a/include/linux/rolling_buffer.h
+++ b/include/linux/rolling_buffer.h
@@ -43,7 +43,7 @@ struct rolling_buffer_snapshot {
 #define ROLLBUF_MARK_2	BIT(1)
 
 int rolling_buffer_init(struct rolling_buffer *roll, unsigned int rreq_id,
-			unsigned int direction);
+			unsigned int direction, gfp_t gfp);
 int rolling_buffer_make_space(struct rolling_buffer *roll);
 ssize_t rolling_buffer_load_from_ra(struct rolling_buffer *roll,
 				    struct readahead_control *ractl,
-- 
2.43.0