| Submitter | Allison Henderson |
|---|---|
| Date | Aug. 24, 2011, 7:07 p.m. |
| Message ID | <1314212842-16741-6-git-send-email-achender@linux.vnet.ibm.com> |
| Download | mbox | patch |
| Permalink | /patch/111407/ |
| State | Superseded |
| Headers | show |
Comments
On Wed, Aug 24, 2011 at 12:07:22PM -0700, Allison Henderson wrote: > While running extended fsx tests to verify the preceeding patches, > a similar bug was also found in the write operation > > When ever a write operation begins or ends in a hole, > or extends EOF, the partial page contained in the hole > or beyond EOF needs to be zeroed out. > > To correct this the new ext4_discard_partial_page_buffers_no_lock > routine is used to zero out the partial page, but only for buffer > heads that are already unmapped. > > Signed-off-by: Allison Henderson <achender@linux.vnet.ibm.com> I haven't had a chance to debug this yet, but FYI, with this patch applied, xfstests #224 causes OOM's to result when running under KVM with 1024megs. I tried doubling the memory to 2048 megs, and it passed in 4k standard ext4 mode, but then got hit with the OOM killer on test #224 in ext3 emulation mode (nodealloc, and with extents not enabled in the file system). Without this patch, and the first four patches in the v6 patch series applied, xfstests #224 passes in all of the standard configurations (4k, ext3, 1k, data=journal, etc.) I don't know why this is happening, but I'll try to dig into it in the next day or two. Any thoughts or fixes you might suggest would be much appreciated. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/26/2011 08:47 AM, Ted Ts'o wrote: > On Wed, Aug 24, 2011 at 12:07:22PM -0700, Allison Henderson wrote: >> While running extended fsx tests to verify the preceeding patches, >> a similar bug was also found in the write operation >> >> When ever a write operation begins or ends in a hole, >> or extends EOF, the partial page contained in the hole >> or beyond EOF needs to be zeroed out. >> >> To correct this the new ext4_discard_partial_page_buffers_no_lock >> routine is used to zero out the partial page, but only for buffer >> heads that are already unmapped. >> >> Signed-off-by: Allison Henderson<achender@linux.vnet.ibm.com> > > I haven't had a chance to debug this yet, but FYI, with this patch > applied, xfstests #224 causes OOM's to result when running under KVM > with 1024megs. I tried doubling the memory to 2048 megs, and it > passed in 4k standard ext4 mode, but then got hit with the OOM killer > on test #224 in ext3 emulation mode (nodealloc, and with extents not > enabled in the file system). Without this patch, and the first four > patches in the v6 patch series applied, xfstests #224 passes in all of > the standard configurations (4k, ext3, 1k, data=journal, etc.) > > I don't know why this is happening, but I'll try to dig into it in the > next day or two. Any thoughts or fixes you might suggest would be > much appreciated. > > - Ted Alrighty then, I will look into this one and see if I can figure out whats going on. Thx again! Allison Henderson > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 6328428..1674fa7 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -2214,6 +2214,7 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping, pgoff_t index; struct inode *inode = mapping->host; handle_t *handle; + loff_t page_len; index = pos >> PAGE_CACHE_SHIFT; @@ -2260,6 +2261,13 @@ retry: */ if (pos + len > inode->i_size) ext4_truncate_failed_write(inode); + } else { + page_len = pos & (PAGE_CACHE_SIZE - 1); + if (page_len > 0) { + ret = ext4_discard_partial_page_buffers_no_lock(handle, + inode, page, pos - page_len, page_len, + EXT4_DSCRD_PARTIAL_PG_ZERO_UNMAPED); + } } if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries)) @@ -2302,6 +2310,7 @@ static int ext4_da_write_end(struct file *file, loff_t new_i_size; unsigned long start, end; int write_mode = (int)(unsigned long)fsdata; + loff_t page_len; if (write_mode == FALL_BACK_TO_NONDELALLOC) { if (ext4_should_order_data(inode)) { @@ -2350,6 +2359,16 @@ static int ext4_da_write_end(struct file *file, } ret2 = generic_write_end(file, mapping, pos, len, copied, page, fsdata); + + page_len = PAGE_CACHE_SIZE - + ((pos + copied - 1) & (PAGE_CACHE_SIZE - 1)); + + if (page_len > 0) { + ret = ext4_discard_partial_page_buffers_no_lock(handle, + inode, page, pos + copied - 1, page_len, + EXT4_DSCRD_PARTIAL_PG_ZERO_UNMAPED); + } + copied = ret2; if (ret2 < 0) ret = ret2;
While running extended fsx tests to verify the preceeding patches, a similar bug was also found in the write operation When ever a write operation begins or ends in a hole, or extends EOF, the partial page contained in the hole or beyond EOF needs to be zeroed out. To correct this the new ext4_discard_partial_page_buffers_no_lock routine is used to zero out the partial page, but only for buffer heads that are already unmapped. Signed-off-by: Allison Henderson <achender@linux.vnet.ibm.com> --- :100644 100644 6328428... 1674fa7... M fs/ext4/inode.c fs/ext4/inode.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-)