mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Ext4 Developers List <linux-ext4@vger.kernel.org>
Cc: Linux Kernel Developers List <linux-kernel@vger.kernel.org>,
	dave.hansen@intel.com, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, "Theodore Ts'o" <tytso@mit.edu>
Subject: [PATCH] ext4: use private version of page_zero_new_buffers() for data=journal mode
Date: Fri,  9 Oct 2015 00:01:09 -0400	[thread overview]
Message-ID: <1444363269-25956-1-git-send-email-tytso@mit.edu> (raw)
In-Reply-To: <20151007154303.GC24678@thunk.org>

If there is a error while copying data from userspace into the page
cache during a write(2) system call, in data=journal mode, in
ext4_journalled_write_end() were using page_zero_new_buffers() from
fs/buffer.c.  Unfortunately, this sets the buffer dirty flag, which is
no good if journalling is enabled.  This is a long-standing bug that
goes back for years and years in ext3, but a combination of (a)
data=journal not being very common, (b) in many case it only results
in a warning message. and (c) only very rarely causes the kernel hang,
means that we only really noticed this as a problem when commit
998ef75ddb caused this failure to happen frequently enough to cause
generic/208 to fail when run in data=journal mode.

The fix is to have our own version of this function that doesn't call
mark_dirty_buffer(), since we will end up calling
ext4_handle_dirty_metadata() on the buffer head(s) in questions very
shortly afterwards in ext4_journalled_write_end().

Thanks to Dave Hansen and Linus Torvalds for helping to identify the
root cause of the problem.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 fs/ext4/inode.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index ae52e32..0a589bb 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1181,6 +1181,38 @@ errout:
 	return ret ? ret : copied;
 }
 
+/*
+ * This is a private version of page_zero_new_buffers() which doesn't
+ * set the buffer to be dirty, since in data=journalled mode we need
+ * to call ext4_handle_dirty_metadata() instad.
+ */
+static void zero_new_buffers(struct page *page, unsigned from, unsigned to)
+{
+	unsigned int block_start = 0, block_end;
+	struct buffer_head *head, *bh;
+
+	bh = head = page_buffers(page);
+	do {
+		block_end = block_start + bh->b_size;
+		if (buffer_new(bh)) {
+			if (block_end > from && block_start < to) {
+				if (!PageUptodate(page)) {
+					unsigned start, size;
+
+					start = max(from, block_start);
+					size = min(to, block_end) - start;
+
+					zero_user(page, start, size);
+					set_buffer_uptodate(bh);
+				}
+				clear_buffer_new(bh);
+			}
+		}
+		block_start = block_end;
+		bh = bh->b_this_page;
+	} while (bh != head);
+}
+
 static int ext4_journalled_write_end(struct file *file,
 				     struct address_space *mapping,
 				     loff_t pos, unsigned len, unsigned copied,
@@ -1207,7 +1239,7 @@ static int ext4_journalled_write_end(struct file *file,
 		if (copied < len) {
 			if (!PageUptodate(page))
 				copied = 0;
-			page_zero_new_buffers(page, from+copied, to);
+			zero_new_buffers(page, from+copied, to);
 		}
 
 		ret = ext4_walk_page_buffers(handle, page_buffers(page), from,
-- 
2.5.0


  reply	other threads:[~2015-10-09  4:01 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-05 15:22 [REGRESSION] 998ef75ddb and aio-dio-invalidate-failure w/ data=journal Theodore Ts'o
2015-10-05 15:58 ` Linus Torvalds
2015-10-05 16:23   ` Dave Hansen
2015-10-05 20:22     ` Linus Torvalds
2015-10-05 20:48       ` Dave Hansen
2015-10-05 21:18         ` Linus Torvalds
2015-10-05 21:55           ` Linus Torvalds
2015-10-05 23:33             ` Dave Hansen
2015-10-06  9:01               ` Linus Torvalds
2015-10-05 20:49       ` H. Peter Anvin
2015-10-06  7:56         ` Ingo Molnar
2015-10-06  9:10           ` Linus Torvalds
2015-10-06  9:27             ` Ingo Molnar
2015-10-06 13:29               ` Linus Torvalds
2015-10-06 13:42                 ` Ingo Molnar
2015-10-05 16:03 ` Dave Hansen
2015-10-05 18:04 ` Dave Hansen
2015-10-07  3:34   ` Theodore Ts'o
2015-10-07  7:32     ` Linus Torvalds
2015-10-07 15:43       ` Theodore Ts'o
2015-10-09  4:01         ` Theodore Ts'o [this message]
2015-10-13  6:06           ` [PATCH] ext4: use private version of page_zero_new_buffers() for data=journal mode Leonid V. Fedorenchik
2015-10-15 11:17           ` Jan Kara
2025-01-26 17:01           ` Mateusz Guzik
2025-01-26 18:48             ` Linus Torvalds
2025-01-26 19:49               ` Mateusz Guzik
2025-01-26 22:03                 ` Linus Torvalds
2025-01-26 22:45                   ` Mateusz Guzik
2025-01-27 20:52                     ` Dave Hansen
2025-01-27 21:46                       ` Dave Chinner

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=1444363269-25956-1-git-send-email-tytso@mit.edu \
    --to=tytso@mit.edu \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@intel.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®