From: "Theodore Ts'o" <tytso@mit.edu>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [REGRESSION] 998ef75ddb and aio-dio-invalidate-failure w/ data=journal
Date: Tue, 6 Oct 2015 23:34:48 -0400 [thread overview]
Message-ID: <20151007033448.GB24678@thunk.org> (raw)
In-Reply-To: <5612BBB3.7010201@intel.com>
On Mon, Oct 05, 2015 at 11:04:35AM -0700, Dave Hansen wrote:
>
> The warning comes out of ext4_walk_page_buffers() and the dirty state
> comes from page_zero_new_buffers(). That seems a _bit_ goofy that the
> filesystem is marking the page dirty and then so shortly warning about it.
Yes, this is a bug in ext4 --- and in fact in ext3, which apparently
we've lived with for *years*. The problem is that when we are
journalling data buffers, we can't use page_zero_new_buffers(),
because instead of calling mark_buffer_dirty(bh), we need to call
ext4_handle_dirty_metadata(bh). This will call mark_buffer_dirty(bh)
if journalling is not enabled, or if journalling is enabled, it will
call jbd2_journal_dirty_metadata(handle,bh).
Apprently it is extremely rare that (copied < len) --- especially when
mm/filemap.c was doing a prefault. :-)
So your patch looks good, but in addition to that, if copied is > 0
and less than len, we shouldn't be calling page_zero_new_buffers().
We're going to need our own version of it that doesn't call
mark_buffer_dirty().
So if Linus wants to revert 998ef75ddb patch, we can do that, but I'm
also happy applying your patch as a way of preventing the failure.
We'll need to do more work to make ext4_journalled_write_end(), but
that's a bigger change which I'd rather not do at this point in the
development cycle.
Thanks again for taking a closer look at things. I'm currently
running a full soak test to make sure your patch to
ext4_journalled_write_end() doesn't introduce any other problems, but
I'm quite confident it should be fine.
Cheers,
- Ted
next prev parent reply other threads:[~2015-10-07 3:34 UTC|newest]
Thread overview: 23+ 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 [this message]
2015-10-07 7:32 ` Linus Torvalds
2015-10-07 15:43 ` Theodore Ts'o
2015-10-09 4:01 ` [PATCH] ext4: use private version of page_zero_new_buffers() for data=journal mode Theodore Ts'o
2015-10-13 6:06 ` Leonid V. Fedorenchik
2015-10-15 11:17 ` Jan Kara
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=20151007033448.GB24678@thunk.org \
--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;
as well as URLs for NNTP newsgroup(s).