|| ||Joel Becker <Joel.Becker-AT-oracle.com> |
|| ||Linus Torvalds <torvalds-AT-linux-foundation.org> |
|| ||Re: [PATCH] Revert "writeback: limit write_cache_pages integrity scanning to current EOF" |
|| ||Mon, 28 Jun 2010 18:58:23 -0700|
|| ||Mark Fasheh <mfasheh-AT-suse.com>, Dave Chinner <david-AT-fromorbit.com>,
Linux Kernel <linux-kernel-AT-vger.kernel.org>,
ocfs2-devel-AT-oss.oracle.com, Dave Chinner <dchinner-AT-redhat.com>|
|| ||Article, Thread
On Mon, Jun 28, 2010 at 06:12:35PM -0700, Linus Torvalds wrote:
> On Mon, Jun 28, 2010 at 5:54 PM, Joel Becker <Joel.Becker@oracle.com> wrote:
> > Â Â Â Â Your contention is that we've never gotten those tail blocks to
> > disk. Â Instead, our code either handles the future extensions of i_size
> > or we've just gotten lucky with our testing. Â Our current BUG trigger is
> > because we have a new check that catches this case. Â Does that summarize
> > your position correctly?
> Maybe Dave has some more exhaustive answer, but his point that
> block_write_full_page() already just drops the page does seem to be
> very valid. Which makes me suspect that it would be better to remove
> the ocfs2 BUG_ON() as a stop-gap measure, rather than reverting the
> commit. It seems to be true that the "don't bother flushing past EOF"
> commit really just uncovered an older bug.
Well, shit. Something has changed in here, or we're really
really (un)lucky. We visited this code a year ago or so when we had
serious zeroing problems, and we tested the hell out of it. Now it is
broken again. And it sure looks like that block_write_full_page() check
has been there since before git.
> So maybe ocfs2 should just replace the bug-on with invalidating the
> page (perhaps with a WARN_ONCE() to make sure the problem doesn't get
> forgotten about?)
Oh, no, that's not it at all. This is a disaster. I can't see
for the life of me why we haven't had 100,000 bug reports. You're going
to have an ocfs2 patch by the end of the week. It will be ugly, I'm
sure of it, but it has to be done. For every extend, we're going to
have to zero and potentially CoW around old_i_size if the old allocation
isn't within the bounds of the current write.
"In a crisis, don't hide behind anything or anybody. They're going
to find you anyway."
- Paul "Bear" Bryant
Consulting Software Developer
Phone: (650) 506-8127
to post comments)