All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Edwards <cfsworks@gmail.com>
To: Xiubo Li <xiubli@redhat.com>, Ilya Dryomov <idryomov@gmail.com>
Cc: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>,
	Christian Brauner <brauner@kernel.org>,
	Milind Changire <mchangir@redhat.com>,
	Jeff Layton <jlayton@kernel.org>,
	ceph-devel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Sam Edwards <CFSworks@gmail.com>
Subject: [PATCH 4/5] ceph: Assert writeback loop invariants
Date: Tue, 30 Dec 2025 18:43:15 -0800	[thread overview]
Message-ID: <20251231024316.4643-5-CFSworks@gmail.com> (raw)
In-Reply-To: <20251231024316.4643-1-CFSworks@gmail.com>

If `locked_pages` is zero, the page array must not be allocated:
ceph_process_folio_batch() uses `locked_pages` to decide when to
allocate `pages`, and redundant allocations trigger
ceph_allocate_page_array()'s BUG_ON(), resulting in a worker oops (and
writeback stall) or even a kernel panic. Consequently, the main loop in
ceph_writepages_start() assumes that the lifetime of `pages` is confined
to a single iteration.

This expectation is currently not clear enough, as evidenced by the
previous two patches which fix oopses caused by `pages` persisting into
the next loop iteration.

Use an explicit BUG_ON() at the top of the loop to assert the loop's
preexisting expectation that `pages` is cleaned up by the previous
iteration. Because this is closely tied to `locked_pages`, also make it
the previous iteration's responsibility to guarantee its reset, and
verify with a second new BUG_ON() instead of handling (and masking)
failures to do so.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 fs/ceph/addr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 91cc43950162..b3569d44d510 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -1669,7 +1669,9 @@ static int ceph_writepages_start(struct address_space *mapping,
 		tag_pages_for_writeback(mapping, ceph_wbc.index, ceph_wbc.end);
 
 	while (!has_writeback_done(&ceph_wbc)) {
-		ceph_wbc.locked_pages = 0;
+		BUG_ON(ceph_wbc.locked_pages);
+		BUG_ON(ceph_wbc.pages);
+
 		ceph_wbc.max_pages = ceph_wbc.wsize >> PAGE_SHIFT;
 
 get_more_pages:
@@ -1703,11 +1705,10 @@ static int ceph_writepages_start(struct address_space *mapping,
 		}
 
 		rc = ceph_submit_write(mapping, wbc, &ceph_wbc);
-		if (rc)
-			goto release_folios;
-
 		ceph_wbc.locked_pages = 0;
 		ceph_wbc.strip_unit_end = 0;
+		if (rc)
+			goto release_folios;
 
 		if (folio_batch_count(&ceph_wbc.fbatch) > 0) {
 			ceph_wbc.nr_folios =
-- 
2.51.2


  parent reply	other threads:[~2025-12-31  2:55 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-31  2:43 [PATCH 0/5] ceph: CephFS writeback correctness and performance fixes Sam Edwards
2025-12-31  2:43 ` [PATCH 1/5] ceph: Do not propagate page array emplacement errors as batch errors Sam Edwards
2026-01-05 20:23   ` Viacheslav Dubeyko
2026-01-06  6:52     ` Sam Edwards
2026-01-06 21:08       ` Viacheslav Dubeyko
2026-01-06 23:50         ` Sam Edwards
2025-12-31  2:43 ` [PATCH 2/5] ceph: Remove error return from ceph_process_folio_batch() Sam Edwards
2026-01-05 20:36   ` Viacheslav Dubeyko
2026-01-06  6:52     ` Sam Edwards
2026-01-06 22:47       ` Viacheslav Dubeyko
2026-01-07  0:15         ` Sam Edwards
2025-12-31  2:43 ` [PATCH 3/5] ceph: Free page array when ceph_submit_write fails Sam Edwards
2026-01-05 21:09   ` Viacheslav Dubeyko
2026-01-06  6:52     ` Sam Edwards
2025-12-31  2:43 ` Sam Edwards [this message]
2026-01-05 22:28   ` [PATCH 4/5] ceph: Assert writeback loop invariants Viacheslav Dubeyko
2026-01-06  6:53     ` Sam Edwards
2026-01-06 23:00       ` Viacheslav Dubeyko
2026-01-07  0:33         ` Sam Edwards
2025-12-31  2:43 ` [PATCH 5/5] ceph: Fix write storm on fscrypted files Sam Edwards
2026-01-05 22:34   ` Viacheslav Dubeyko
2026-01-06  6:53     ` Sam Edwards
2026-01-06 23:11       ` Viacheslav Dubeyko
2026-01-07  0:05         ` Sam Edwards

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=20251231024316.4643-5-CFSworks@gmail.com \
    --to=cfsworks@gmail.com \
    --cc=Slava.Dubeyko@ibm.com \
    --cc=brauner@kernel.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchangir@redhat.com \
    --cc=xiubli@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.