From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752301AbbJMGav (ORCPT ); Tue, 13 Oct 2015 02:30:51 -0400 Received: from relaymsk-01.paragon-software.com ([91.226.210.14]:19586 "EHLO relaymsk-01.paragon-software.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751723AbbJMGat (ORCPT ); Tue, 13 Oct 2015 02:30:49 -0400 X-Greylist: delayed 1448 seconds by postgrey-1.27 at vger.kernel.org; Tue, 13 Oct 2015 02:30:48 EDT Date: Tue, 13 Oct 2015 09:06:45 +0300 From: "Leonid V. Fedorenchik" To: "Theodore Ts'o" CC: Ext4 Developers List , "Linux Kernel Developers List" , , , Subject: Re: [PATCH] ext4: use private version of page_zero_new_buffers() for data=journal mode Message-ID: <20151013090645.58575d09@paragon-software.com> In-Reply-To: <1444363269-25956-1-git-send-email-tytso@mit.edu> References: <20151007154303.GC24678@thunk.org> <1444363269-25956-1-git-send-email-tytso@mit.edu> Organization: Paragon Software Group X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [172.30.116.218] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 9 Oct 2015 00:01:09 -0400 Theodore Ts'o wrote: > 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 > --- > 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. Small typo: s/instad/instead/ > + */ > +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, -- Best regards, Leonid Fedorenchik Software Engineer Paragon Software Group Skype: leonid.fedorenchik http://www.paragon-software.com