[PATCH] exofs: simple_write_end does not mark_inode_dirty

[PATCH] exofs: simple_write_end does not mark_inode_dirty

[Boaz Harrosh]

exofs uses simple_write_end() for it's .write_end handler. But
it is not enough because simple_write_end() does not call
mark_inode_dirty() when it extends i_size. So even if we do
call mark_inode_dirty at beginning of write out, with a very
long IO and a saturated system we might get the .write_inode()
called while still extend-writing to file and miss out on the last
i_size updates.

So override .write_end, call simple_write_end(), and afterwords if
i_size was changed call mark_inode_dirty().

It stands to logic that since simple_write_end() was the one extending
i_size it should also call mark_inode_dirty(). But it looks like all
users of simple_write_end() are memory-bound pseudo filesystems, who
could careless about mark_inode_dirty(). I might submit a
warning-comment patch to simple_write_end() in future.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/inode.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
index 0b6f318..a8de6ae 100644
--- a/fs/exofs/inode.c
+++ b/fs/exofs/inode.c
@@ -738,13 +738,28 @@ static int exofs_write_begin_export(struct file *file,
 					fsdata);
 }
 
+static int exofs_write_end(struct file *file, struct address_space *mapping,
+			loff_t pos, unsigned len, unsigned copied,
+			struct page *page, void *fsdata)
+{
+	struct inode *inode = mapping->host;
+	/* According to comment in simple_write_end i_mutex is held */
+	loff_t i_size = inode->i_size;
+	int ret;
+
+	ret = simple_write_end(file, mapping,pos, len, copied, page, fsdata);
+	if (i_size != inode->i_size)
+		mark_inode_dirty(inode);
+	return ret;
+}
+
 const struct address_space_operations exofs_aops = {
 	.readpage	= exofs_readpage,
 	.readpages	= exofs_readpages,
 	.writepage	= exofs_writepage,
 	.writepages	= exofs_writepages,
 	.write_begin	= exofs_write_begin_export,
-	.write_end	= simple_write_end,
+	.write_end	= exofs_write_end,
 };
 
 /******************************************************************************

Re: [PATCH] exofs: simple_write_end does not mark_inode_dirty

[Andrew Morton]

On Sun, 27 Dec 2009 18:44:21 +0200
Boaz Harrosh <bharrosh@panasas.com> wrote:

> 
> exofs uses simple_write_end() for it's .write_end handler. But
> it is not enough because simple_write_end() does not call
> mark_inode_dirty() when it extends i_size. So even if we do
> call mark_inode_dirty at beginning of write out, with a very
> long IO and a saturated system we might get the .write_inode()
> called while still extend-writing to file and miss out on the last
> i_size updates.
> 
> So override .write_end, call simple_write_end(), and afterwords if
> i_size was changed call mark_inode_dirty().
> 
> It stands to logic that since simple_write_end() was the one extending
> i_size it should also call mark_inode_dirty(). But it looks like all
> users of simple_write_end() are memory-bound pseudo filesystems, who
> could careless about mark_inode_dirty(). I might submit a
> warning-comment patch to simple_write_end() in future.
> 
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

You added this to linux-next without a "Cc: <stable@kernel.org>" to the
changelog, so it won't be backported.  However this patch should be
backported, IMO.

Re: [PATCH] exofs: simple_write_end does not mark_inode_dirty

[Boaz Harrosh]

On 01/05/2010 01:42 AM, Andrew Morton wrote:
> On Sun, 27 Dec 2009 18:44:21 +0200
> Boaz Harrosh <bharrosh@panasas.com> wrote:
> 
>>
>> exofs uses simple_write_end() for it's .write_end handler. But
>> it is not enough because simple_write_end() does not call
>> mark_inode_dirty() when it extends i_size. So even if we do
>> call mark_inode_dirty at beginning of write out, with a very
>> long IO and a saturated system we might get the .write_inode()
>> called while still extend-writing to file and miss out on the last
>> i_size updates.
>>
>> So override .write_end, call simple_write_end(), and afterwords if
>> i_size was changed call mark_inode_dirty().
>>
>> It stands to logic that since simple_write_end() was the one extending
>> i_size it should also call mark_inode_dirty(). But it looks like all
>> users of simple_write_end() are memory-bound pseudo filesystems, who
>> could careless about mark_inode_dirty(). I might submit a
>> warning-comment patch to simple_write_end() in future.
>>
>> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
> 

OK, Thanks Andrew
I'll redo it ASAP before pushing to Linus

> You added this to linux-next without a "Cc: <stable@kernel.org>" to the
> changelog, so it won't be backported.  However this patch should be
> backported, IMO.
> 
> 

Boaz