[PATCH] ext4: Warn when dirtying page without buffers in data=journal mode

[PATCH] ext4: Warn when dirtying page without buffers in data=journal mode

[Jan Kara]

Recently I've got a report of BUG_ON trigerring during transaction
commit in ext4_journalled_writepage_callback() because we've spotted a
dirty page without buffers. Add WARN_ON_ONCE to
ext4_journalled_set_page_dirty() to catch the problematic condition
earlier where we have better chance of understanding which code path is
creating dirty data without preparing the page properly. Also update the
comment with current information when we are at it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 01c9e4f743ba..f8f15fd25c6f 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3541,10 +3541,11 @@ const struct iomap_ops ext4_iomap_report_ops = {
 };
 
 /*
- * Pages can be marked dirty completely asynchronously from ext4's journalling
- * activity.  By filemap_sync_pte(), try_to_unmap_one(), etc.  We cannot do
- * much here because ->set_page_dirty is called under VFS locks.  The page is
- * not necessarily locked.
+ * Whenever the page is being dirtied, corresponding buffers should already be
+ * attached to the transaction (we take care of this in ext4_page_mkwrite() and
+ * ext4_write_begin()). However we cannot move buffers to dirty transaction
+ * lists here because ->set_page_dirty is called under VFS locks and the page
+ * is not necessarily locked.
  *
  * We cannot just dirty the page and leave attached buffers clean, because the
  * buffers' dirty state is "definitive".  We cannot just set the buffers dirty
@@ -3555,6 +3556,7 @@ const struct iomap_ops ext4_iomap_report_ops = {
  */
 static int ext4_journalled_set_page_dirty(struct page *page)
 {
+	WARN_ON_ONCE(!page_has_buffers(page));
 	SetPageChecked(page);
 	return __set_page_dirty_nobuffers(page);
 }
-- 
2.34.1

Re: [PATCH] ext4: Warn when dirtying page without buffers in data=journal mode

[Theodore Ts'o]

On Thu, 10 Mar 2022 11:18:32 +0100, Jan Kara wrote:
> Recently I've got a report of BUG_ON trigerring during transaction
> commit in ext4_journalled_writepage_callback() because we've spotted a
> dirty page without buffers. Add WARN_ON_ONCE to
> ext4_journalled_set_page_dirty() to catch the problematic condition
> earlier where we have better chance of understanding which code path is
> creating dirty data without preparing the page properly. Also update the
> comment with current information when we are at it.
> 
> [...]

Applied, thanks!

[1/1] ext4: Warn when dirtying page without buffers in data=journal mode
      commit: 2bb8dd401a4f96973d6a9de4218d7f01fbd497ee

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>