On Tue, Jun 24, 2008 at 08:37:21AM +0530, Aneesh Kumar K.V wrote:
> On Mon, Jun 23, 2008 at 05:31:32PM -0700, Mingming wrote:
> >
> > On Mon, 2008-06-23 at 23:15 +0530, Aneesh Kumar K.V wrote:
> >
> > > I found one place where we fail to update i_disksize. Can you try this
> > > patch ?
> > >
> > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> > > index 33f940b..9fa737f 100644
> > > --- a/fs/ext4/inode.c
> > > +++ b/fs/ext4/inode.c
> > > @@ -1620,7 +1620,10 @@ static int ext4_da_writepage(struct page *page,
> > > loff_t size;
> > > unsigned long len;
> > > handle_t *handle = NULL;
> > > + ext4_lblk_t block;
> > > + loff_t disksize;
> > > struct buffer_head *page_bufs;
> > > + struct buffer_head *bh, *head;
> > > struct inode *inode = page->mapping->host;
> > >
> > > handle = ext4_journal_current_handle();
> > > @@ -1662,6 +1665,38 @@ static int ext4_da_writepage(struct page *page,
> > > else
> > > ret = block_write_full_page(page, ext4_da_get_block_write, wbc);
> > >
> > > + if (ret)
> > > + return ret;
> > > + /*
> > > + * When called via shrink_page_list and if we don't have any unmapped
> > > + * buffer_head we still could have written some new content in an
> > > + * already mapped buffer. That means we need to extent i_disksize here
> > > + */
> >
> > In this case(when extend the file without need block allocation),
> > wouldn't make sense to update the i_disksize at write_end() time? So
> > that the window of i_size different from i_disksize could be much
> > smaller in this case.
> >
> >
> > Something like below? (untested)
>
> In this case you will have to start a transaction in write_begin . With
> the below code transaction is started inside page_lock. Also I don't
> think we need needed_blocks credit just 1 should be enough because we
> are not doing any block allocation here. We just need to update the
> inode block.
>
>
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 33f940b..bc925c5 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1770,6 +1770,7 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,
struct page *page;
pgoff_t index;
unsigned from, to;
+ handle_t *handle;
struct inode *inode = mapping->host;
index = pos >> PAGE_CACHE_SHIFT;
@@ -1777,6 +1778,17 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,
to = from + len;
retry:
+ /*
+ * If we are writing towards the end of an already mapped
+ * buffer_head, we don't do any block allocation. But we
+ * need to update i_disksize.
+ */
+ handle = ext4_journal_start(inode, 1);
+ if (IS_ERR(handle)) {
+ ret = PTR_ERR(handle);
+ goto out;
+ }
+
page = __grab_cache_page(mapping, index);
if (!page)
return -ENOMEM;
@@ -1786,15 +1798,63 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping,
ext4_da_get_block_prep);
if (ret < 0) {
unlock_page(page);
+ ext4_journal_stop(handle);
page_cache_release(page);
}
if (ret == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
goto retry;
+out:
return ret;
}
+static int ext4_da_write_end(struct file *file,
+ struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
+{
+ loff_t new_i_size;
+ unsigned from, to;
+ int ret = 0, ret2;
+ struct inode *inode = mapping->host;
+ handle_t *handle = ext4_journal_current_handle();
+
+ from = pos & (PAGE_CACHE_SIZE - 1);
+ to = from + len;
+
+ /*
+ * generic_write_end() will run mark_inode_dirty() if i_size
+ * changes. So let's piggyback the i_disksize mark_inode_dirty
+ * into that.
+ */
+
+ new_i_size = pos + copied;
+ if (new_i_size > EXT4_I(inode)->i_disksize)
+ if (!walk_page_buffers(NULL, page_buffers(page),
+ 0, len, NULL,
+ ext4_bh_unmapped_or_delay)) {
+ /*
+ * Updating i_disksize when extending file without
+ * need block allocation
+ */
+ if (ext4_should_order_data(inode))
+ ret = ext4_jbd2_file_inode(handle, inode);
+
+ EXT4_I(inode)->i_disksize = new_i_size;
+ }
+ ret2 = generic_write_end(file, mapping, pos,
+ len, copied, page, fsdata);
+ copied = ret2;
+ if (ret2 < 0)
+ ret = ret2;
+ ret2 = ext4_journal_stop(handle);
+ if (!ret)
+ ret = ret2;
+
+ return ret ? ret : copied;
+}
+
static void ext4_da_invalidatepage(struct page *page, unsigned long offset)
{
/*
@@ -2250,7 +2310,7 @@ static int ext4_journalled_set_page_dirty(struct page *page)
.writepages = ext4_da_writepages,
.sync_page = block_sync_page,
.write_begin = ext4_da_write_begin,
- .write_end = generic_write_end,
+ .write_end = ext4_da_write_end,
.bmap = ext4_bmap,
.invalidatepage = ext4_da_invalidatepage,
.releasepage = ext4_releasepage,
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
