On Wed, Jun 09, 2010 at 04:48:20PM +0800, Tao Ma wrote:
> Nick has changed the turncate sequence and now inode_setattr
> is deprecated. ocfs2 has done the most work of i_size change
> before calling inode_setattr, so we are safe to just call
> simple_setsize to update the i_size in vfs inode and then
> generic_setattr.
>
> As for dlmfs, we don't allow inode size change, so it is safe
> for us to just call generic_setattr.
>
> Cc: Nick Piggin <npiggin@xxxxxxx>
> Signed-off-by: Tao Ma <tao.ma@xxxxxxxxxx>
> ---
> fs/ocfs2/dlmfs/dlmfs.c | 2 +-
> fs/ocfs2/file.c | 18 +++++++++---------
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/fs/ocfs2/dlmfs/dlmfs.c b/fs/ocfs2/dlmfs/dlmfs.c
> index b83d610..e959b88 100644
> --- a/fs/ocfs2/dlmfs/dlmfs.c
> +++ b/fs/ocfs2/dlmfs/dlmfs.c
> @@ -215,7 +215,7 @@ static int dlmfs_file_setattr(struct dentry *dentry, struct iattr *attr)
> attr->ia_valid &= ~ATTR_SIZE;
> error = inode_change_ok(inode, attr);
> if (!error)
> - error = inode_setattr(inode, attr);
> + generic_setattr(inode, attr);
This brings up my question again about simply _ignoring_ ATTR_SIZE changes.
One problem is that truncate calls which send down ATTR_SIZE might also
be sending down ATTR_MODE or ATTR_?TIME which should not be applied if
the ATTR_SIZE fails, one would think.
The other thing is that shouldn't we return -EINVAL or somesuch rather
than silently ignoring?
Anyway, several other pseudo filesystems do other stuff like tihs so
it's not something to bother with in this patch.
>
> return error;
> }
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index 6a13ea6..06fd85c 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -1052,17 +1052,17 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
> }
>
> /*
> - * This will intentionally not wind up calling simple_setsize(),
> - * since all the work for a size change has been done above.
> - * Otherwise, we could get into problems with truncate as
> - * ip_alloc_sem is used there to protect against i_size
> - * changes.
> + * Since all the work for a size change has been done above,
> + * we only need to call simple_setsize to update i_size.
> */
> - status = inode_setattr(inode, attr);
> - if (status < 0) {
> - mlog_errno(status);
> - goto bail_commit;
> + if (attr->ia_valid & ATTR_SIZE) {
> + status = simple_setsize(inode, attr->ia_size);
> + if (status < 0) {
> + mlog_errno(status);
> + goto bail_commit;
> + }
> }
> + generic_setattr(inode, attr);
>
> status = ocfs2_mark_inode_dirty(handle, inode, bh);
> if (status < 0)
simple_setsize of course calls inode_newsize_ok, which may fail. If
you're committing your on-disk truncates at this point, you will
prefer to hoist that check as early as possible so you cannot fail
here.
Just open code the truncate part of simple_setsize for now.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
