From 0203c144cb44823eeae58b12cb3c6614a124e0dc Mon Sep 17 00:00:00 2001 From: yangsheng Date: Wed, 15 Feb 2012 13:57:50 +0800 Subject: [PATCH] LU-1069 mdc: Avoid doing permission check again. inode_setattr() replaced by simple_setattr(). The latter will invoke inode_change_ok(). It causes a -EPERM error when we want to update inode timestamps. Signed-off-by: yang sheng Change-Id: I29c58d7027df1bf94db58df82e5214fd12ea5c87 Reviewed-on: http://review.whamcloud.com/2145 Tested-by: Hudson Tested-by: Maloo Reviewed-by: Fan Yong Reviewed-by: Oleg Drokin --- lustre/include/linux/lustre_compat25.h | 6 ++++++ lustre/llite/llite_lib.c | 35 +++++++++++++++++++++------------- lustre/lvfs/fsfilt_ext3.c | 4 +++- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/lustre/include/linux/lustre_compat25.h b/lustre/include/linux/lustre_compat25.h index 470b2b7..d6e7cfe 100644 --- a/lustre/include/linux/lustre_compat25.h +++ b/lustre/include/linux/lustre_compat25.h @@ -930,5 +930,11 @@ static inline int ll_quota_off(struct super_block *sb, int off, int remount) # define ext2_find_next_zero_bit find_next_zero_bit_le #endif +#ifdef ATTR_TIMES_SET +# define TIMES_SET_FLAGS (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET) +#else +# define TIMES_SET_FLAGS (ATTR_MTIME_SET | ATTR_ATIME_SET) +#endif + #endif /* __KERNEL__ */ #endif /* _COMPAT25_H */ diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c index 4e49985..815cf6c 100644 --- a/lustre/llite/llite_lib.c +++ b/lustre/llite/llite_lib.c @@ -1200,7 +1200,7 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data, struct inode *inode = dentry->d_inode; struct ll_sb_info *sbi = ll_i2sbi(inode); struct ptlrpc_request *request = NULL; - int rc; + int rc, ia_valid; ENTRY; op_data = ll_prep_md_op_data(op_data, inode, NULL, NULL, 0, 0, @@ -1217,8 +1217,12 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data, /* Unlinked special device node? Or just a race? * Pretend we done everything. */ if (!S_ISREG(inode->i_mode) && - !S_ISDIR(inode->i_mode)) + !S_ISDIR(inode->i_mode)) { + ia_valid = op_data->op_attr.ia_valid; + op_data->op_attr.ia_valid &= ~TIMES_SET_FLAGS; rc = simple_setattr(dentry, &op_data->op_attr); + op_data->op_attr.ia_valid = ia_valid; + } } else if (rc != -EPERM && rc != -EACCES && rc != -ETXTBSY) { CERROR("md_setattr fails: rc = %d\n", rc); } @@ -1232,12 +1236,16 @@ int ll_md_setattr(struct dentry *dentry, struct md_op_data *op_data, RETURN(rc); } - /* We call inode_setattr to adjust timestamps. + /* We want to adjust timestamps. * If there is at least some data in file, we cleared ATTR_SIZE - * above to avoid invoking vmtruncate, otherwise it is important - * to call vmtruncate in inode_setattr to update inode->i_size + * to avoid update size, otherwise it is important to do.(SOM case) * (bug 6196) */ + ia_valid = op_data->op_attr.ia_valid; + /* Since we set ATTR_*_SET flags above, and already done permission + * check, So don't let inode_change_ok() check it again. */ + op_data->op_attr.ia_valid &= ~TIMES_SET_FLAGS; rc = simple_setattr(dentry, &op_data->op_attr); + op_data->op_attr.ia_valid = ia_valid; /* Extract epoch data if obtained. */ op_data->op_handle = md.body->handle; @@ -1340,7 +1348,7 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr) } /* POSIX: check before ATTR_*TIME_SET set (from inode_change_ok) */ - if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET)) { + if (ia_valid & TIMES_SET_FLAGS) { if (cfs_curproc_fsuid() != inode->i_uid && !cfs_capable(CFS_CAP_FOWNER)) RETURN(-EPERM); @@ -1377,14 +1385,15 @@ int ll_setattr_raw(struct dentry *dentry, struct iattr *attr) if (op_data == NULL) RETURN(-ENOMEM); - UNLOCK_INODE_MUTEX(inode); - if (ia_valid & ATTR_SIZE) - UP_WRITE_I_ALLOC_SEM(inode); - if (!S_ISDIR(inode->i_mode)) + if (!S_ISDIR(inode->i_mode)) { + if (ia_valid & ATTR_SIZE) + UP_WRITE_I_ALLOC_SEM(inode); + UNLOCK_INODE_MUTEX(inode); cfs_down_write(&lli->lli_trunc_sem); - LOCK_INODE_MUTEX(inode); - if (ia_valid & ATTR_SIZE) - DOWN_WRITE_I_ALLOC_SEM(inode); + LOCK_INODE_MUTEX(inode); + if (ia_valid & ATTR_SIZE) + DOWN_WRITE_I_ALLOC_SEM(inode); + } memcpy(&op_data->op_attr, attr, sizeof(*attr)); diff --git a/lustre/lvfs/fsfilt_ext3.c b/lustre/lvfs/fsfilt_ext3.c index 5fbed73..08e09da 100644 --- a/lustre/lvfs/fsfilt_ext3.c +++ b/lustre/lvfs/fsfilt_ext3.c @@ -656,13 +656,15 @@ static int fsfilt_ext3_setattr(struct dentry *dentry, void *handle, /* We set these flags on the client, but have already checked perms * so don't confuse inode_change_ok. */ - iattr->ia_valid &= ~(ATTR_MTIME_SET | ATTR_ATIME_SET); + iattr->ia_valid &= ~TIMES_SET_FLAGS; if (inode->i_op->setattr) { rc = inode->i_op->setattr(dentry, iattr); } else { +#ifndef HAVE_SIMPLE_SETATTR /* simple_setattr() already call it */ rc = inode_change_ok(inode, iattr); if (!rc) +#endif rc = simple_setattr(dentry, iattr); } -- 1.8.3.1