[PATCH 47/58] convert exofs to ->evict_inode()

[PATCH 47/58] convert exofs to ->evict_inode()

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/exofs/exofs.h |    2 +-
 fs/exofs/inode.c |    8 ++++----
 fs/exofs/super.c |    2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
index 22721b2..1a566bf 100644
--- a/fs/exofs/exofs.h
+++ b/fs/exofs/exofs.h
@@ -264,7 +264,7 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
 extern struct inode *exofs_iget(struct super_block *, unsigned long);
 struct inode *exofs_new_inode(struct inode *, int);
 extern int exofs_write_inode(struct inode *, struct writeback_control *wbc);
-extern void exofs_delete_inode(struct inode *);
+extern void exofs_evict_inode(struct inode *);
 
 /* dir.c:                */
 int exofs_add_link(struct dentry *, struct inode *);
diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
index 4bfc1f4..0c9c23d 100644
--- a/fs/exofs/inode.c
+++ b/fs/exofs/inode.c
@@ -1335,7 +1335,7 @@ static void delete_done(struct exofs_io_state *ios, void *p)
  * from the OSD here.  We make sure the object was created before we try and
  * delete it.
  */
-void exofs_delete_inode(struct inode *inode)
+void exofs_evict_inode(struct inode *inode)
 {
 	struct exofs_i_info *oi = exofs_i(inode);
 	struct super_block *sb = inode->i_sb;
@@ -1345,7 +1345,7 @@ void exofs_delete_inode(struct inode *inode)
 
 	truncate_inode_pages(&inode->i_data, 0);
 
-	if (is_bad_inode(inode))
+	if (inode->i_nlink || is_bad_inode(inode))
 		goto no_delete;
 
 	mark_inode_dirty(inode);
@@ -1355,7 +1355,7 @@ void exofs_delete_inode(struct inode *inode)
 	if (inode->i_blocks)
 		exofs_truncate(inode);
 
-	clear_inode(inode);
+	end_writeback(inode);
 
 	ret = exofs_get_io_state(&sbi->layout, &ios);
 	if (unlikely(ret)) {
@@ -1384,5 +1384,5 @@ void exofs_delete_inode(struct inode *inode)
 	return;
 
 no_delete:
-	clear_inode(inode);
+	end_writeback(inode);
 }
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 03149b9..32cfd61 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -743,7 +743,7 @@ static const struct super_operations exofs_sops = {
 	.alloc_inode    = exofs_alloc_inode,
 	.destroy_inode  = exofs_destroy_inode,
 	.write_inode    = exofs_write_inode,
-	.delete_inode   = exofs_delete_inode,
+	.evict_inode    = exofs_evict_inode,
 	.put_super      = exofs_put_super,
 	.write_super    = exofs_write_super,
 	.sync_fs	= exofs_sync_fs,
-- 
1.5.6.5

Re: [PATCH 47/58] convert exofs to ->evict_inode()

[Boaz Harrosh]

On 06/09/2010 01:22 AM, Al Viro wrote:
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Al hi.

This patch is nice off course, but it clashes with a patch
I was intending to push to linux-next today. (For 2.6.36 kernel)

The patch is the one to convert to the new truncate sequence:
http://marc.info/?l=linux-fsdevel&m=127539189718732&w=2

In fact I was meaning to ask you if you could take this patch
through your tree, so Christoph could base his cleanup on this
as well.

I can re-do below patch on top of mine, or redo the truncate
on top of this. Please tell me what you prefer.

Thanks
Boaz

> ---
>  fs/exofs/exofs.h |    2 +-
>  fs/exofs/inode.c |    8 ++++----
>  fs/exofs/super.c |    2 +-
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
> index 22721b2..1a566bf 100644
> --- a/fs/exofs/exofs.h
> +++ b/fs/exofs/exofs.h
> @@ -264,7 +264,7 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
>  extern struct inode *exofs_iget(struct super_block *, unsigned long);
>  struct inode *exofs_new_inode(struct inode *, int);
>  extern int exofs_write_inode(struct inode *, struct writeback_control *wbc);
> -extern void exofs_delete_inode(struct inode *);
> +extern void exofs_evict_inode(struct inode *);
>  
>  /* dir.c:                */
>  int exofs_add_link(struct dentry *, struct inode *);
> diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
> index 4bfc1f4..0c9c23d 100644
> --- a/fs/exofs/inode.c
> +++ b/fs/exofs/inode.c
> @@ -1335,7 +1335,7 @@ static void delete_done(struct exofs_io_state *ios, void *p)
>   * from the OSD here.  We make sure the object was created before we try and
>   * delete it.
>   */
> -void exofs_delete_inode(struct inode *inode)
> +void exofs_evict_inode(struct inode *inode)
>  {
>  	struct exofs_i_info *oi = exofs_i(inode);
>  	struct super_block *sb = inode->i_sb;
> @@ -1345,7 +1345,7 @@ void exofs_delete_inode(struct inode *inode)
>  
>  	truncate_inode_pages(&inode->i_data, 0);
>  
> -	if (is_bad_inode(inode))
> +	if (inode->i_nlink || is_bad_inode(inode))
>  		goto no_delete;
>  
>  	mark_inode_dirty(inode);
> @@ -1355,7 +1355,7 @@ void exofs_delete_inode(struct inode *inode)
>  	if (inode->i_blocks)
>  		exofs_truncate(inode);
>  
> -	clear_inode(inode);
> +	end_writeback(inode);
>  
>  	ret = exofs_get_io_state(&sbi->layout, &ios);
>  	if (unlikely(ret)) {
> @@ -1384,5 +1384,5 @@ void exofs_delete_inode(struct inode *inode)
>  	return;
>  
>  no_delete:
> -	clear_inode(inode);
> +	end_writeback(inode);
>  }
> diff --git a/fs/exofs/super.c b/fs/exofs/super.c
> index 03149b9..32cfd61 100644
> --- a/fs/exofs/super.c
> +++ b/fs/exofs/super.c
> @@ -743,7 +743,7 @@ static const struct super_operations exofs_sops = {
>  	.alloc_inode    = exofs_alloc_inode,
>  	.destroy_inode  = exofs_destroy_inode,
>  	.write_inode    = exofs_write_inode,
> -	.delete_inode   = exofs_delete_inode,
> +	.evict_inode    = exofs_evict_inode,
>  	.put_super      = exofs_put_super,
>  	.write_super    = exofs_write_super,
>  	.sync_fs	= exofs_sync_fs,

Re: [PATCH 47/58] convert exofs to ->evict_inode()

[Christoph Hellwig]

On Wed, Jun 09, 2010 at 12:05:36PM +0300, Boaz Harrosh wrote:
> On 06/09/2010 01:22 AM, Al Viro wrote:
> > 
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 
> Al hi.
> 
> This patch is nice off course, but it clashes with a patch
> I was intending to push to linux-next today. (For 2.6.36 kernel)
> 
> The patch is the one to convert to the new truncate sequence:
> http://marc.info/?l=linux-fsdevel&m=127539189718732&w=2
> 
> In fact I was meaning to ask you if you could take this patch
> through your tree, so Christoph could base his cleanup on this
> as well.

The new truncate sequence bits already are in Al's tree.  And as
I said a few times before life would really be a lot simpler if
you just base things ontop of that.  It makes the API for the
file system a lot better.

Re: [PATCH 47/58] convert exofs to ->evict_inode()

[Boaz Harrosh]

On 06/09/2010 01:02 PM, Christoph Hellwig wrote:
> On Wed, Jun 09, 2010 at 12:05:36PM +0300, Boaz Harrosh wrote:
>> On 06/09/2010 01:22 AM, Al Viro wrote:
>>>
>>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>>
>> Al hi.
>>
>> This patch is nice off course, but it clashes with a patch
>> I was intending to push to linux-next today. (For 2.6.36 kernel)
>>
>> The patch is the one to convert to the new truncate sequence:
>> http://marc.info/?l=linux-fsdevel&m=127539189718732&w=2
>>
>> In fact I was meaning to ask you if you could take this patch
>> through your tree, so Christoph could base his cleanup on this
>> as well.
> 
> The new truncate sequence bits already are in Al's tree.  And as
> I said a few times before life would really be a lot simpler if
> you just base things ontop of that.  It makes the API for the
> file system a lot better.
> 

Will do. Patch based on vfs/for-next posted as reply.
(Please review?)

Al's patch thing still holds, Al?

Boaz

[PATCH ver4] exofs: New truncate sequence

[Boaz Harrosh]

These changes are crafted based on the similar
conversion done to ext2 by Nick Piggin.

* Remove the deprecated ->truncate vector. Let exofs_setattr
  take care of on-disk size updates.
* Call truncate_pagecache on the unused pages if
  write_begin/end fails.
* Cleanup exofs_delete_inode that did stupid inode
  writes and updates on an inode that will be
  removed.
* And finally get rid of exofs_get_block. We never
  had any blocks it was all for calling nobh_truncate_page.
  nobh_truncate_page is not actually needed in exofs since
  the last page is complete and gone, just like all the other
  pages. There is no partial blocks in exofs.

I've tested with this patch, and there are no apparent
failures, so far.

CC: Nick Piggin <npiggin@suse.de>
CC: Christoph Hellwig <hch@lst.de>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/exofs.h |    1 -
 fs/exofs/file.c  |    1 -
 fs/exofs/inode.c |  114 ++++++++++++++++++++----------------------------------
 3 files changed, 42 insertions(+), 74 deletions(-)

diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
index 22721b2..0706ce9 100644
--- a/fs/exofs/exofs.h
+++ b/fs/exofs/exofs.h
@@ -256,7 +256,6 @@ static inline int exofs_oi_read(struct exofs_i_info *oi,
 }
 
 /* inode.c               */
-void exofs_truncate(struct inode *inode);
 int exofs_setattr(struct dentry *, struct iattr *);
 int exofs_write_begin(struct file *file, struct address_space *mapping,
 		loff_t pos, unsigned len, unsigned flags,
diff --git a/fs/exofs/file.c b/fs/exofs/file.c
index fef6899..f9bfe2b 100644
--- a/fs/exofs/file.c
+++ b/fs/exofs/file.c
@@ -86,6 +86,5 @@ const struct file_operations exofs_file_operations = {
 };
 
 const struct inode_operations exofs_file_inode_operations = {
-	.truncate	= exofs_truncate,
 	.setattr	= exofs_setattr,
 };
diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
index 4bfc1f4..5510a5e 100644
--- a/fs/exofs/inode.c
+++ b/fs/exofs/inode.c
@@ -710,7 +710,7 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
 					 fsdata);
 		if (ret) {
 			EXOFS_DBGMSG("simple_write_begin faild\n");
-			return ret;
+			goto out;
 		}
 
 		page = *pagep;
@@ -725,7 +725,14 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
 			EXOFS_DBGMSG("__readpage_filler faild\n");
 		}
 	}
+out:
+	if (unlikely(ret)) {
+		struct inode *inode = mapping->host;
+		loff_t to = pos + len;
 
+		if (to > inode->i_size)
+			truncate_pagecache(inode, to, inode->i_size);
+	}
 	return ret;
 }
 
@@ -750,6 +757,10 @@ static int exofs_write_end(struct file *file, struct address_space *mapping,
 	int ret;
 
 	ret = simple_write_end(file, mapping,pos, len, copied, page, fsdata);
+	if (unlikely(ret && pos + len > inode->i_size))
+		truncate_pagecache(inode, pos + len, inode->i_size);
+
+	/* TODO: once simple_write_end marks inode dirty remove */
 	if (i_size != inode->i_size)
 		mark_inode_dirty(inode);
 	return ret;
@@ -808,91 +819,53 @@ static inline int exofs_inode_is_fast_symlink(struct inode *inode)
 	return S_ISLNK(inode->i_mode) && (oi->i_data[0] != 0);
 }
 
-/*
- * get_block_t - Fill in a buffer_head
- * An OSD takes care of block allocation so we just fake an allocation by
- * putting in the inode's sector_t in the buffer_head.
- * TODO: What about the case of create==0 and @iblock does not exist in the
- * object?
- */
-static int exofs_get_block(struct inode *inode, sector_t iblock,
-		    struct buffer_head *bh_result, int create)
-{
-	map_bh(bh_result, inode->i_sb, iblock);
-	return 0;
-}
-
 const struct osd_attr g_attr_logical_length = ATTR_DEF(
 	OSD_APAGE_OBJECT_INFORMATION, OSD_ATTR_OI_LOGICAL_LENGTH, 8);
 
-static int _do_truncate(struct inode *inode)
+static int _do_truncate(struct inode *inode, loff_t newsize)
 {
 	struct exofs_i_info *oi = exofs_i(inode);
-	loff_t isize = i_size_read(inode);
 	int ret;
 
 	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 
-	nobh_truncate_page(inode->i_mapping, isize, exofs_get_block);
+	ret = exofs_oi_truncate(oi, (u64)newsize);
+	if (likely(!ret))
+		truncate_setsize(inode, newsize);
 
-	ret = exofs_oi_truncate(oi, (u64)isize);
-	EXOFS_DBGMSG("(0x%lx) size=0x%llx\n", inode->i_ino, isize);
+	EXOFS_DBGMSG("(0x%lx) size=0x%llx ret=>%d\n",
+		     inode->i_ino, newsize, ret);
 	return ret;
 }
 
 /*
- * Truncate a file to the specified size - all we have to do is set the size
- * attribute.  We make sure the object exists first.
- */
-void exofs_truncate(struct inode *inode)
-{
-	struct exofs_i_info *oi = exofs_i(inode);
-	int ret;
-
-	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)
-	     || S_ISLNK(inode->i_mode)))
-		return;
-	if (exofs_inode_is_fast_symlink(inode))
-		return;
-	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
-		return;
-
-	/* if we are about to truncate an object, and it hasn't been
-	 * created yet, wait
-	 */
-	if (unlikely(wait_obj_created(oi)))
-		goto fail;
-
-	ret = _do_truncate(inode);
-	if (ret)
-		goto fail;
-
-out:
-	mark_inode_dirty(inode);
-	return;
-fail:
-	make_bad_inode(inode);
-	goto out;
-}
-
-/*
- * Set inode attributes - just call generic functions.
+ * Set inode attributes - update size attribute on OSD if needed,
+ *                        otherwise just call generic functions.
  */
 int exofs_setattr(struct dentry *dentry, struct iattr *iattr)
 {
 	struct inode *inode = dentry->d_inode;
 	int error;
 
+	/* if we are about to modify an object, and it hasn't been
+	 * created yet, wait
+	 */
+	error = wait_obj_created(exofs_i(inode));
+	if (unlikely(error))
+		return error;
+
 	error = inode_change_ok(inode, iattr);
-	if (error)
+	if (unlikely(error))
 		return error;
 
 	if ((iattr->ia_valid & ATTR_SIZE) &&
 	    iattr->ia_size != i_size_read(inode)) {
-		int error;
+		error = inode_newsize_ok(inode, iattr->ia_size);
+		if (unlikely(error))
+			return error;
 
-		error = vmtruncate(inode, iattr->ia_size);
-		if (error)
+		error = _do_truncate(inode, iattr->ia_size);
+		if (unlikely(error))
 			return error;
 	}
 
@@ -1345,28 +1318,25 @@ void exofs_delete_inode(struct inode *inode)
 
 	truncate_inode_pages(&inode->i_data, 0);
 
+	/* TODO: should do better here */
 	if (is_bad_inode(inode))
 		goto no_delete;
 
-	mark_inode_dirty(inode);
-	exofs_update_inode(inode, inode_needs_sync(inode));
-
 	inode->i_size = 0;
-	if (inode->i_blocks)
-		exofs_truncate(inode);
-
 	clear_inode(inode);
 
-	ret = exofs_get_io_state(&sbi->layout, &ios);
-	if (unlikely(ret)) {
-		EXOFS_ERR("%s: exofs_get_io_state failed\n", __func__);
-		return;
-	}
-
 	/* if we are deleting an obj that hasn't been created yet, wait */
 	if (!obj_created(oi)) {
 		BUG_ON(!obj_2bcreated(oi));
 		wait_event(oi->i_wq, obj_created(oi));
+		/* ignore the error attempt a remove anyway */
+	}
+
+	/* Now Remove the OSD objects */
+	ret = exofs_get_io_state(&sbi->layout, &ios);
+	if (unlikely(ret)) {
+		EXOFS_ERR("%s: exofs_get_io_state failed\n", __func__);
+		return;
 	}
 
 	ios->obj.id = exofs_oi_objno(oi);
-- 
1.6.6.1

Re: [PATCH ver4] exofs: New truncate sequence

[Boaz Harrosh]

On 06/09/2010 02:10 PM, Boaz Harrosh wrote:
> 
> These changes are crafted based on the similar
> conversion done to ext2 by Nick Piggin.
> 
> * Remove the deprecated ->truncate vector. Let exofs_setattr
>   take care of on-disk size updates.
> * Call truncate_pagecache on the unused pages if
>   write_begin/end fails.
> * Cleanup exofs_delete_inode that did stupid inode
>   writes and updates on an inode that will be
>   removed.
> * And finally get rid of exofs_get_block. We never
>   had any blocks it was all for calling nobh_truncate_page.
>   nobh_truncate_page is not actually needed in exofs since
>   the last page is complete and gone, just like all the other
>   pages. There is no partial blocks in exofs.
> 
> I've tested with this patch, and there are no apparent
> failures, so far.
> 
> CC: Nick Piggin <npiggin@suse.de>
> CC: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>

Al hi.

Please take this through your tree. And/or tell me if you need it
reorder with your ->evict patches.

Thanks
Boaz

> ---
>  fs/exofs/exofs.h |    1 -
>  fs/exofs/file.c  |    1 -
>  fs/exofs/inode.c |  114 ++++++++++++++++++++----------------------------------
>  3 files changed, 42 insertions(+), 74 deletions(-)
> 
> diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
> index 22721b2..0706ce9 100644
> --- a/fs/exofs/exofs.h
> +++ b/fs/exofs/exofs.h
> @@ -256,7 +256,6 @@ static inline int exofs_oi_read(struct exofs_i_info *oi,
>  }
>  
>  /* inode.c               */
> -void exofs_truncate(struct inode *inode);
>  int exofs_setattr(struct dentry *, struct iattr *);
>  int exofs_write_begin(struct file *file, struct address_space *mapping,
>  		loff_t pos, unsigned len, unsigned flags,
> diff --git a/fs/exofs/file.c b/fs/exofs/file.c
> index fef6899..f9bfe2b 100644
> --- a/fs/exofs/file.c
> +++ b/fs/exofs/file.c
> @@ -86,6 +86,5 @@ const struct file_operations exofs_file_operations = {
>  };
>  
>  const struct inode_operations exofs_file_inode_operations = {
> -	.truncate	= exofs_truncate,
>  	.setattr	= exofs_setattr,
>  };
> diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
> index 4bfc1f4..5510a5e 100644
> --- a/fs/exofs/inode.c
> +++ b/fs/exofs/inode.c
> @@ -710,7 +710,7 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
>  					 fsdata);
>  		if (ret) {
>  			EXOFS_DBGMSG("simple_write_begin faild\n");
> -			return ret;
> +			goto out;
>  		}
>  
>  		page = *pagep;
> @@ -725,7 +725,14 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
>  			EXOFS_DBGMSG("__readpage_filler faild\n");
>  		}
>  	}
> +out:
> +	if (unlikely(ret)) {
> +		struct inode *inode = mapping->host;
> +		loff_t to = pos + len;
>  
> +		if (to > inode->i_size)
> +			truncate_pagecache(inode, to, inode->i_size);
> +	}
>  	return ret;
>  }
>  
> @@ -750,6 +757,10 @@ static int exofs_write_end(struct file *file, struct address_space *mapping,
>  	int ret;
>  
>  	ret = simple_write_end(file, mapping,pos, len, copied, page, fsdata);
> +	if (unlikely(ret && pos + len > inode->i_size))
> +		truncate_pagecache(inode, pos + len, inode->i_size);
> +
> +	/* TODO: once simple_write_end marks inode dirty remove */
>  	if (i_size != inode->i_size)
>  		mark_inode_dirty(inode);
>  	return ret;
> @@ -808,91 +819,53 @@ static inline int exofs_inode_is_fast_symlink(struct inode *inode)
>  	return S_ISLNK(inode->i_mode) && (oi->i_data[0] != 0);
>  }
>  
> -/*
> - * get_block_t - Fill in a buffer_head
> - * An OSD takes care of block allocation so we just fake an allocation by
> - * putting in the inode's sector_t in the buffer_head.
> - * TODO: What about the case of create==0 and @iblock does not exist in the
> - * object?
> - */
> -static int exofs_get_block(struct inode *inode, sector_t iblock,
> -		    struct buffer_head *bh_result, int create)
> -{
> -	map_bh(bh_result, inode->i_sb, iblock);
> -	return 0;
> -}
> -
>  const struct osd_attr g_attr_logical_length = ATTR_DEF(
>  	OSD_APAGE_OBJECT_INFORMATION, OSD_ATTR_OI_LOGICAL_LENGTH, 8);
>  
> -static int _do_truncate(struct inode *inode)
> +static int _do_truncate(struct inode *inode, loff_t newsize)
>  {
>  	struct exofs_i_info *oi = exofs_i(inode);
> -	loff_t isize = i_size_read(inode);
>  	int ret;
>  
>  	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
>  
> -	nobh_truncate_page(inode->i_mapping, isize, exofs_get_block);
> +	ret = exofs_oi_truncate(oi, (u64)newsize);
> +	if (likely(!ret))
> +		truncate_setsize(inode, newsize);
>  
> -	ret = exofs_oi_truncate(oi, (u64)isize);
> -	EXOFS_DBGMSG("(0x%lx) size=0x%llx\n", inode->i_ino, isize);
> +	EXOFS_DBGMSG("(0x%lx) size=0x%llx ret=>%d\n",
> +		     inode->i_ino, newsize, ret);
>  	return ret;
>  }
>  
>  /*
> - * Truncate a file to the specified size - all we have to do is set the size
> - * attribute.  We make sure the object exists first.
> - */
> -void exofs_truncate(struct inode *inode)
> -{
> -	struct exofs_i_info *oi = exofs_i(inode);
> -	int ret;
> -
> -	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)
> -	     || S_ISLNK(inode->i_mode)))
> -		return;
> -	if (exofs_inode_is_fast_symlink(inode))
> -		return;
> -	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> -		return;
> -
> -	/* if we are about to truncate an object, and it hasn't been
> -	 * created yet, wait
> -	 */
> -	if (unlikely(wait_obj_created(oi)))
> -		goto fail;
> -
> -	ret = _do_truncate(inode);
> -	if (ret)
> -		goto fail;
> -
> -out:
> -	mark_inode_dirty(inode);
> -	return;
> -fail:
> -	make_bad_inode(inode);
> -	goto out;
> -}
> -
> -/*
> - * Set inode attributes - just call generic functions.
> + * Set inode attributes - update size attribute on OSD if needed,
> + *                        otherwise just call generic functions.
>   */
>  int exofs_setattr(struct dentry *dentry, struct iattr *iattr)
>  {
>  	struct inode *inode = dentry->d_inode;
>  	int error;
>  
> +	/* if we are about to modify an object, and it hasn't been
> +	 * created yet, wait
> +	 */
> +	error = wait_obj_created(exofs_i(inode));
> +	if (unlikely(error))
> +		return error;
> +
>  	error = inode_change_ok(inode, iattr);
> -	if (error)
> +	if (unlikely(error))
>  		return error;
>  
>  	if ((iattr->ia_valid & ATTR_SIZE) &&
>  	    iattr->ia_size != i_size_read(inode)) {
> -		int error;
> +		error = inode_newsize_ok(inode, iattr->ia_size);
> +		if (unlikely(error))
> +			return error;
>  
> -		error = vmtruncate(inode, iattr->ia_size);
> -		if (error)
> +		error = _do_truncate(inode, iattr->ia_size);
> +		if (unlikely(error))
>  			return error;
>  	}
>  
> @@ -1345,28 +1318,25 @@ void exofs_delete_inode(struct inode *inode)
>  
>  	truncate_inode_pages(&inode->i_data, 0);
>  
> +	/* TODO: should do better here */
>  	if (is_bad_inode(inode))
>  		goto no_delete;
>  
> -	mark_inode_dirty(inode);
> -	exofs_update_inode(inode, inode_needs_sync(inode));
> -
>  	inode->i_size = 0;
> -	if (inode->i_blocks)
> -		exofs_truncate(inode);
> -
>  	clear_inode(inode);
>  
> -	ret = exofs_get_io_state(&sbi->layout, &ios);
> -	if (unlikely(ret)) {
> -		EXOFS_ERR("%s: exofs_get_io_state failed\n", __func__);
> -		return;
> -	}
> -
>  	/* if we are deleting an obj that hasn't been created yet, wait */
>  	if (!obj_created(oi)) {
>  		BUG_ON(!obj_2bcreated(oi));
>  		wait_event(oi->i_wq, obj_created(oi));
> +		/* ignore the error attempt a remove anyway */
> +	}
> +
> +	/* Now Remove the OSD objects */
> +	ret = exofs_get_io_state(&sbi->layout, &ios);
> +	if (unlikely(ret)) {
> +		EXOFS_ERR("%s: exofs_get_io_state failed\n", __func__);
> +		return;
>  	}
>  
>  	ios->obj.id = exofs_oi_objno(oi);

Re: [PATCH ver4] exofs: New truncate sequence

[Al Viro]

On Wed, Jun 09, 2010 at 02:13:00PM +0300, Boaz Harrosh wrote:

> Al hi.
> 
> Please take this through your tree. And/or tell me if you need it
> reorder with your ->evict patches.

Um...  Don't we need to free on-disk data blocks when inode gets deleted?

Re: [PATCH ver4] exofs: New truncate sequence

[Boaz Harrosh]

On 06/09/2010 03:45 PM, Al Viro wrote:
> On Wed, Jun 09, 2010 at 02:13:00PM +0300, Boaz Harrosh wrote:
> 
>> Al hi.
>>
>> Please take this through your tree. And/or tell me if you need it
>> reorder with your ->evict patches.
> 
> Um...  Don't we need to free on-disk data blocks when inode gets deleted?

No, the OSD device takes care of that when you send it an OSD_REMOVE
command.

Thanks
Boaz

Re: [PATCH ver4] exofs: New truncate sequence

[Christoph Hellwig]

> +	if (unlikely(ret)) {
> +		struct inode *inode = mapping->host;
> +		loff_t to = pos + len;
>  
> +		if (to > inode->i_size)
> +			truncate_pagecache(inode, to, inode->i_size);
> +	}

Might be worth to add a exofs_write_failed helper similar to ext2 that
can be shared with write_end, and once added ->direct_IO?

> +		error = inode_newsize_ok(inode, iattr->ia_size);
> +		if (unlikely(error))
> +			return error;

This has been folded into inode_change_ok in vfs.git #for_next, you
can drop it.

[PATCH ver5] exofs: New truncate sequence

[Boaz Harrosh]

These changes are crafted based on the similar
conversion done to ext2 by Nick Piggin.

* Remove the deprecated ->truncate vector. Let exofs_setattr
  take care of on-disk size updates.
* Call truncate_pagecache on the unused pages if
  write_begin/end fails.
* Cleanup exofs_delete_inode that did stupid inode
  writes and updates on an inode that will be
  removed.
* And finally get rid of exofs_get_block. We never
  had any blocks it was all for calling nobh_truncate_page.
  nobh_truncate_page is not actually needed in exofs since
  the last page is complete and gone, just like all the other
  pages. There is no partial blocks in exofs.

I've tested with this patch, and there are no apparent
failures, so far.

CC: Nick Piggin <npiggin@suse.de>
CC: Christoph Hellwig <hch@lst.de>
Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 fs/exofs/exofs.h |    1 -
 fs/exofs/file.c  |    1 -
 fs/exofs/inode.c |  115 ++++++++++++++++++++----------------------------------
 3 files changed, 42 insertions(+), 75 deletions(-)

diff --git a/fs/exofs/exofs.h b/fs/exofs/exofs.h
index 22721b2..0706ce9 100644
--- a/fs/exofs/exofs.h
+++ b/fs/exofs/exofs.h
@@ -256,7 +256,6 @@ static inline int exofs_oi_read(struct exofs_i_info *oi,
 }
 
 /* inode.c               */
-void exofs_truncate(struct inode *inode);
 int exofs_setattr(struct dentry *, struct iattr *);
 int exofs_write_begin(struct file *file, struct address_space *mapping,
 		loff_t pos, unsigned len, unsigned flags,
diff --git a/fs/exofs/file.c b/fs/exofs/file.c
index fef6899..f9bfe2b 100644
--- a/fs/exofs/file.c
+++ b/fs/exofs/file.c
@@ -86,6 +86,5 @@ const struct file_operations exofs_file_operations = {
 };
 
 const struct inode_operations exofs_file_inode_operations = {
-	.truncate	= exofs_truncate,
 	.setattr	= exofs_setattr,
 };
diff --git a/fs/exofs/inode.c b/fs/exofs/inode.c
index 4bfc1f4..ccd0ce3 100644
--- a/fs/exofs/inode.c
+++ b/fs/exofs/inode.c
@@ -697,6 +697,13 @@ static int exofs_writepage(struct page *page, struct writeback_control *wbc)
 	return write_exec(&pcol);
 }
 
+/* i_mutex held using inode->i_size directly */
+static void _write_failed(struct inode *inode, loff_t to)
+{
+	if (to > inode->i_size)
+		truncate_pagecache(inode, to, inode->i_size);
+}
+
 int exofs_write_begin(struct file *file, struct address_space *mapping,
 		loff_t pos, unsigned len, unsigned flags,
 		struct page **pagep, void **fsdata)
@@ -710,7 +717,7 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
 					 fsdata);
 		if (ret) {
 			EXOFS_DBGMSG("simple_write_begin faild\n");
-			return ret;
+			goto out;
 		}
 
 		page = *pagep;
@@ -725,6 +732,9 @@ int exofs_write_begin(struct file *file, struct address_space *mapping,
 			EXOFS_DBGMSG("__readpage_filler faild\n");
 		}
 	}
+out:
+	if (unlikely(ret))
+		_write_failed(mapping->host, pos + len);
 
 	return ret;
 }
@@ -750,6 +760,10 @@ static int exofs_write_end(struct file *file, struct address_space *mapping,
 	int ret;
 
 	ret = simple_write_end(file, mapping,pos, len, copied, page, fsdata);
+	if (unlikely(ret))
+		_write_failed(inode, pos + len);
+
+	/* TODO: once simple_write_end marks inode dirty remove */
 	if (i_size != inode->i_size)
 		mark_inode_dirty(inode);
 	return ret;
@@ -808,91 +822,49 @@ static inline int exofs_inode_is_fast_symlink(struct inode *inode)
 	return S_ISLNK(inode->i_mode) && (oi->i_data[0] != 0);
 }
 
-/*
- * get_block_t - Fill in a buffer_head
- * An OSD takes care of block allocation so we just fake an allocation by
- * putting in the inode's sector_t in the buffer_head.
- * TODO: What about the case of create==0 and @iblock does not exist in the
- * object?
- */
-static int exofs_get_block(struct inode *inode, sector_t iblock,
-		    struct buffer_head *bh_result, int create)
-{
-	map_bh(bh_result, inode->i_sb, iblock);
-	return 0;
-}
-
 const struct osd_attr g_attr_logical_length = ATTR_DEF(
 	OSD_APAGE_OBJECT_INFORMATION, OSD_ATTR_OI_LOGICAL_LENGTH, 8);
 
-static int _do_truncate(struct inode *inode)
+static int _do_truncate(struct inode *inode, loff_t newsize)
 {
 	struct exofs_i_info *oi = exofs_i(inode);
-	loff_t isize = i_size_read(inode);
 	int ret;
 
 	inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 
-	nobh_truncate_page(inode->i_mapping, isize, exofs_get_block);
+	ret = exofs_oi_truncate(oi, (u64)newsize);
+	if (likely(!ret))
+		truncate_setsize(inode, newsize);
 
-	ret = exofs_oi_truncate(oi, (u64)isize);
-	EXOFS_DBGMSG("(0x%lx) size=0x%llx\n", inode->i_ino, isize);
+	EXOFS_DBGMSG("(0x%lx) size=0x%llx ret=>%d\n",
+		     inode->i_ino, newsize, ret);
 	return ret;
 }
 
 /*
- * Truncate a file to the specified size - all we have to do is set the size
- * attribute.  We make sure the object exists first.
- */
-void exofs_truncate(struct inode *inode)
-{
-	struct exofs_i_info *oi = exofs_i(inode);
-	int ret;
-
-	if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)
-	     || S_ISLNK(inode->i_mode)))
-		return;
-	if (exofs_inode_is_fast_symlink(inode))
-		return;
-	if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
-		return;
-
-	/* if we are about to truncate an object, and it hasn't been
-	 * created yet, wait
-	 */
-	if (unlikely(wait_obj_created(oi)))
-		goto fail;
-
-	ret = _do_truncate(inode);
-	if (ret)
-		goto fail;
-
-out:
-	mark_inode_dirty(inode);
-	return;
-fail:
-	make_bad_inode(inode);
-	goto out;
-}
-
-/*
- * Set inode attributes - just call generic functions.
+ * Set inode attributes - update size attribute on OSD if needed,
+ *                        otherwise just call generic functions.
  */
 int exofs_setattr(struct dentry *dentry, struct iattr *iattr)
 {
 	struct inode *inode = dentry->d_inode;
 	int error;
 
+	/* if we are about to modify an object, and it hasn't been
+	 * created yet, wait
+	 */
+	error = wait_obj_created(exofs_i(inode));
+	if (unlikely(error))
+		return error;
+
 	error = inode_change_ok(inode, iattr);
-	if (error)
+	if (unlikely(error))
 		return error;
 
 	if ((iattr->ia_valid & ATTR_SIZE) &&
 	    iattr->ia_size != i_size_read(inode)) {
-		int error;
-
-		error = vmtruncate(inode, iattr->ia_size);
-		if (error)
+		error = _do_truncate(inode, iattr->ia_size);
+		if (unlikely(error))
 			return error;
 	}
 
@@ -1345,28 +1317,25 @@ void exofs_delete_inode(struct inode *inode)
 
 	truncate_inode_pages(&inode->i_data, 0);
 
+	/* TODO: should do better here */
 	if (is_bad_inode(inode))
 		goto no_delete;
 
-	mark_inode_dirty(inode);
-	exofs_update_inode(inode, inode_needs_sync(inode));
-
 	inode->i_size = 0;
-	if (inode->i_blocks)
-		exofs_truncate(inode);
-
 	clear_inode(inode);
 
-	ret = exofs_get_io_state(&sbi->layout, &ios);
-	if (unlikely(ret)) {
-		EXOFS_ERR("%s: exofs_get_io_state failed\n", __func__);
-		return;
-	}
-
 	/* if we are deleting an obj that hasn't been created yet, wait */
 	if (!obj_created(oi)) {
 		BUG_ON(!obj_2bcreated(oi));
 		wait_event(oi->i_wq, obj_created(oi));
+		/* ignore the error attempt a remove anyway */
+	}
+
+	/* Now Remove the OSD objects */
+	ret = exofs_get_io_state(&sbi->layout, &ios);
+	if (unlikely(ret)) {
+		EXOFS_ERR("%s: exofs_get_io_state failed\n", __func__);
+		return;
 	}
 
 	ios->obj.id = exofs_oi_objno(oi);
-- 
1.6.6.1

Re: [PATCH ver4] exofs: New truncate sequence

[Boaz Harrosh]

On 06/09/2010 04:55 PM, Christoph Hellwig wrote:
>> +	if (unlikely(ret)) {
>> +		struct inode *inode = mapping->host;
>> +		loff_t to = pos + len;
>>  
>> +		if (to > inode->i_size)
>> +			truncate_pagecache(inode, to, inode->i_size);
>> +	}
> 
> Might be worth to add a exofs_write_failed helper similar to ext2 that
> can be shared with write_end, and once added ->direct_IO?
> 
>> +		error = inode_newsize_ok(inode, iattr->ia_size);
>> +		if (unlikely(error))
>> +			return error;
> 
> This has been folded into inode_change_ok in vfs.git #for_next, you
> can drop it.
> 

Christoph didn't you have a patch to simple_write_end, is it in vfs/for-next
This patch adds this comment:

+	/* TODO: once simple_write_end marks inode dirty remove */
 	if (i_size != inode->i_size)
 		mark_inode_dirty(inode);

Please CC me on that patch

Thanks for the review
Boaz

Re: [PATCH ver4] exofs: New truncate sequence

[Al Viro]

On Wed, Jun 09, 2010 at 04:43:23PM +0300, Boaz Harrosh wrote:
> On 06/09/2010 03:45 PM, Al Viro wrote:
> > On Wed, Jun 09, 2010 at 02:13:00PM +0300, Boaz Harrosh wrote:
> > 
> >> Al hi.
> >>
> >> Please take this through your tree. And/or tell me if you need it
> >> reorder with your ->evict patches.
> > 
> > Um...  Don't we need to free on-disk data blocks when inode gets deleted?
> 
> No, the OSD device takes care of that when you send it an OSD_REMOVE
> command.

OK, ver5 applied to for-next, evict_inode rediffed.  Should show up on
git.kernel.org shortly.

Re: [PATCH ver4] exofs: New truncate sequence

[Boaz Harrosh]

On 06/10/2010 12:46 AM, Al Viro wrote:
> On Wed, Jun 09, 2010 at 04:43:23PM +0300, Boaz Harrosh wrote:
>> On 06/09/2010 03:45 PM, Al Viro wrote:
>>> On Wed, Jun 09, 2010 at 02:13:00PM +0300, Boaz Harrosh wrote:
>>>
>>>> Al hi.
>>>>
>>>> Please take this through your tree. And/or tell me if you need it
>>>> reorder with your ->evict patches.
>>>
>>> Um...  Don't we need to free on-disk data blocks when inode gets deleted?
>>
>> No, the OSD device takes care of that when you send it an OSD_REMOVE
>> command.
> 
> OK, ver5 applied to for-next, evict_inode rediffed.  Should show up on
> git.kernel.org shortly.

Thank you Al.

I'm also publishing a few exofs patches on -next through the osd tree.

I have test merges of:
- Base Linus tree - as of today
+ merge vfs/for-next
+ merge vfs/evict_inode
+ merge osd/linux-next - based on Linus

By visual inspection of vfs/evict_inode and vfs/for-next they should not conflict
I will monitor linux-next carefully to make sure we are clean. If there are problems
I'll base my tree on vfs/for-next, which should be good as long as you don't rebase.

Thanks again
Boaz