From d8bfc11e446e59abd960b4f3c05cafcbd4f9cd59 Mon Sep 17 00:00:00 2001 From: fanyong Date: Tue, 7 Nov 2006 06:51:37 +0000 Subject: [PATCH] (1) drop unnecessary mdd_read_lock. (2) add some comment. (3) cleanup the code. --- lustre/mdd/mdd_dir.c | 44 +++++++++++++++---------------------- lustre/mdd/mdd_internal.h | 7 +----- lustre/mdd/mdd_object.c | 22 +++++++------------ lustre/mdd/mdd_permission.c | 53 ++++++++++++++++++++++----------------------- 4 files changed, 52 insertions(+), 74 deletions(-) diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c index 76b1de5..3a85560 100644 --- a/lustre/mdd/mdd_dir.c +++ b/lustre/mdd/mdd_dir.c @@ -197,8 +197,8 @@ static int mdd_may_create(const struct lu_env *env, struct mdd_object *pobj, RETURN(-ENOENT); if (need_check) { - rc = mdd_permission_internal_locked(env, pobj, - (MAY_WRITE | MAY_EXEC)); + rc = mdd_permission_internal(env, pobj, NULL, + MAY_WRITE | MAY_EXEC, 1); } RETURN(rc); } @@ -221,9 +221,7 @@ static inline int mdd_is_sticky(const struct lu_env *env, } else if (tmp_la->la_uid == uc->mu_fsuid) { return 0; } else { - mdd_read_lock(env, pobj); rc = mdd_la_get(env, pobj, tmp_la, BYPASS_CAPA); - mdd_read_unlock(env, pobj); if (rc) return rc; else if (!(tmp_la->la_mode & S_ISVTX)) @@ -272,9 +270,8 @@ static int mdd_may_delete(const struct lu_env *env, RETURN(-EPERM); if (need_check) - rc = mdd_permission_internal_locked(env, pobj, - MAY_WRITE | - MAY_EXEC); + rc = mdd_permission_internal(env, pobj, NULL, + MAY_WRITE | MAY_EXEC, 1); } RETURN(rc); } @@ -607,10 +604,6 @@ out_trans: return rc; } -/* - * Partial operation. Be aware, this is called with write lock taken, so we use - * locksless version of __mdd_lookup() here. - */ static int mdd_ni_sanity_check(const struct lu_env *env, struct md_object *pobj, const char *name, @@ -624,10 +617,13 @@ static int mdd_ni_sanity_check(const struct lu_env *env, RETURN(-ENOENT); /* The exist of the name will be checked in _index_insert. */ - RETURN(mdd_permission_internal_locked(env, obj, - MAY_WRITE | MAY_EXEC)); + RETURN(mdd_permission_internal(env, obj, NULL, + MAY_WRITE | MAY_EXEC, 1)); } +/* + * Partial operation. + */ static int mdd_name_insert(const struct lu_env *env, struct md_object *pobj, const char *name, const struct lu_fid *fid, int is_dir) @@ -666,10 +662,6 @@ out_trans: return rc; } -/* - * Be aware, this is called with write lock taken, so we use locksless version - * of __mdd_lookup() here. - */ static int mdd_nr_sanity_check(const struct lu_env *env, struct md_object *pobj, const char *name) @@ -685,11 +677,13 @@ static int mdd_nr_sanity_check(const struct lu_env *env, } /* Name presense will be checked in _index_delete. */ - rc = mdd_permission_internal_locked(env, obj, - MAY_WRITE | MAY_EXEC); + rc = mdd_permission_internal(env, obj, NULL, MAY_WRITE | MAY_EXEC, 1); RETURN(rc); } +/* + * Partial operation. + */ static int mdd_name_remove(const struct lu_env *env, struct md_object *pobj, const char *name, int is_dir) @@ -919,7 +913,7 @@ __mdd_lookup(const struct lu_env *env, struct md_object *pobj, LBUG(); } - rc = mdd_permission_internal_locked(env, mdd_obj, mask); + rc = mdd_permission_internal(env, mdd_obj, NULL, mask, 1); if (rc) RETURN(rc); @@ -1014,15 +1008,13 @@ static int mdd_create_sanity_check(const struct lu_env *env, /* * Check if has WRITE permission for the parent. */ - rc = mdd_permission_internal_locked(env, obj, MAY_WRITE); + rc = mdd_permission_internal(env, obj, NULL, MAY_WRITE, 1); if (rc) RETURN(rc); } /* sgid check */ - mdd_read_lock(env, obj); rc = mdd_la_get(env, obj, la, BYPASS_CAPA); - mdd_read_unlock(env, obj); if (rc != 0) RETURN(rc); @@ -1308,8 +1300,8 @@ static int mdd_rename_sanity_check(const struct lu_env *env, RETURN(-ENOENT); /* The sobj maybe on the remote, check parent permission only here */ - rc = mdd_permission_internal_locked(env, src_pobj, - MAY_WRITE | MAY_EXEC); + rc = mdd_permission_internal(env, src_pobj, NULL, + MAY_WRITE | MAY_EXEC, 1); if (rc) RETURN(rc); @@ -1317,14 +1309,12 @@ static int mdd_rename_sanity_check(const struct lu_env *env, rc = mdd_may_create(env, tgt_pobj, NULL, (src_pobj != tgt_pobj)); } else { - mdd_read_lock(env, tobj); rc = mdd_may_delete(env, tgt_pobj, tobj, src_is_dir, (src_pobj != tgt_pobj)); if (rc == 0) if (S_ISDIR(mdd_object_type(tobj)) && mdd_dir_is_empty(env, tobj)) rc = -ENOTEMPTY; - mdd_read_unlock(env, tobj); } RETURN(rc); diff --git a/lustre/mdd/mdd_internal.h b/lustre/mdd/mdd_internal.h index 706a492..54b7d49 100644 --- a/lustre/mdd/mdd_internal.h +++ b/lustre/mdd/mdd_internal.h @@ -284,13 +284,8 @@ int __mdd_acl_init(const struct lu_env *env, struct mdd_object *obj, struct lu_buf *buf, __u32 *mode, struct thandle *handle); int mdd_acl_init(const struct lu_env *env, struct mdd_object *pobj, struct mdd_object *cobj, __u32 *mode, struct thandle *handle); -int __mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, - int mask, struct lu_attr *la); int mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, - int mask); -int mdd_permission_internal_locked(const struct lu_env *env, - struct mdd_object *obj, int mask); - + struct lu_attr *la, int mask, int needlock); int mdd_permission(const struct lu_env *env, struct md_object *obj, int mask); int mdd_capa_get(const struct lu_env *env, struct md_object *obj, struct lustre_capa *capa, int renewal); diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index 655a77b..f3a6ca8 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -227,9 +227,7 @@ int mdd_get_flags(const struct lu_env *env, struct mdd_object *obj) int rc; ENTRY; - mdd_read_lock(env, obj); rc = mdd_la_get(env, obj, la, BYPASS_CAPA); - mdd_read_unlock(env, obj); if (rc == 0) mdd_flags_xlate(obj, la->la_flags); RETURN(rc); @@ -524,15 +522,15 @@ static int __mdd_xattr_set(const struct lu_env *env, struct mdd_object *o, RETURN(rc); } -/* this gives the same functionality as the code between +/* + * This gives the same functionality as the code between * sys_chmod and inode_setattr * chown_common and inode_setattr * utimes and inode_setattr * This API is ported from mds_fix_attr but remove some unnecesssary stuff. - * and port to */ -int mdd_fix_attr(const struct lu_env *env, struct mdd_object *obj, - struct lu_attr *la) +static int mdd_fix_attr(const struct lu_env *env, struct mdd_object *obj, + struct lu_attr *la) { struct lu_attr *tmp_la = &mdd_env_info(env)->mti_la; struct md_ucred *uc = md_ucred(env); @@ -585,7 +583,7 @@ int mdd_fix_attr(const struct lu_env *env, struct mdd_object *obj, /* Check for setting the obj time. */ if ((la->la_valid & (LA_MTIME | LA_ATIME | LA_CTIME)) && !(la->la_valid & ~(LA_MTIME | LA_ATIME | LA_CTIME))) { - rc = __mdd_permission_internal(env, obj, MAY_WRITE, tmp_la); + rc = mdd_permission_internal(env, obj, tmp_la, MAY_WRITE, 1); if (rc) RETURN(rc); } @@ -671,7 +669,7 @@ int mdd_fix_attr(const struct lu_env *env, struct mdd_object *obj, /* For tuncate (or setsize), we should have MAY_WRITE perm */ if (la->la_valid & (LA_SIZE | LA_BLOCKS)) { - rc = __mdd_permission_internal(env, obj, MAY_WRITE, tmp_la); + rc = mdd_permission_internal(env, obj, tmp_la, MAY_WRITE, 1); if (rc) RETURN(rc); @@ -740,9 +738,7 @@ static int mdd_attr_set(const struct lu_env *env, struct md_object *obj, ma->ma_attr.la_mtime, ma->ma_attr.la_ctime); *la_copy = ma->ma_attr; - mdd_read_lock(env, mdd_obj); rc = mdd_fix_attr(env, mdd_obj, la_copy); - mdd_read_unlock(env, mdd_obj); if (rc) GOTO(cleanup, rc); @@ -809,9 +805,7 @@ static int mdd_xattr_sanity_check(const struct lu_env *env, if (mdd_is_immutable(obj) || mdd_is_append(obj)) RETURN(-EPERM); - mdd_read_lock(env, obj); rc = mdd_la_get(env, obj, tmp_la, BYPASS_CAPA); - mdd_read_unlock(env, obj); if (rc) RETURN(rc); @@ -1124,7 +1118,7 @@ static int mdd_open_sanity_check(const struct lu_env *env, RETURN(-EISDIR); if (!(flag & MDS_OPEN_CREATED)) { - rc = __mdd_permission_internal(env, obj, mode, tmp_la); + rc = mdd_permission_internal(env, obj, tmp_la, mode, 0); if (rc) RETURN(rc); } @@ -1229,7 +1223,7 @@ static int mdd_readpage_sanity_check(const struct lu_env *env, if (S_ISDIR(mdd_object_type(obj)) && dt_try_as_dir(env, next)) #if 0 - rc = mdd_permission_internal(env, obj, MAY_READ); + rc = mdd_permission_internal(env, obj, NULL, MAY_READ, 0); #else rc = 0; #endif diff --git a/lustre/mdd/mdd_permission.c b/lustre/mdd/mdd_permission.c index 3645b8e..ff963e4 100644 --- a/lustre/mdd/mdd_permission.c +++ b/lustre/mdd/mdd_permission.c @@ -204,7 +204,10 @@ check_perm: RETURN(-EACCES); } -/* get default acl EA only */ +/* + * Get default acl EA only. + * Hold read_lock for mdd_obj. + */ int mdd_acl_def_get(const struct lu_env *env, struct mdd_object *mdd_obj, struct md_attr *ma) { @@ -277,6 +280,9 @@ static int mdd_posix_acl_chmod_masq(posix_acl_xattr_entry *entry, return 0; } +/* + * Hold write_lock for o. + */ int mdd_acl_chmod(const struct lu_env *env, struct mdd_object *o, __u32 mode, struct thandle *handle) { @@ -378,6 +384,9 @@ static int mdd_posix_acl_create_masq(posix_acl_xattr_entry *entry, return not_equiv; } +/* + * Hold write_lock for obj. + */ int __mdd_acl_init(const struct lu_env *env, struct mdd_object *obj, struct lu_buf *buf, __u32 *mode, struct thandle *handle) { @@ -415,9 +424,12 @@ int __mdd_acl_init(const struct lu_env *env, struct mdd_object *obj, RETURN(rc); } +/* + * Hold read_lock for pobj. + * Hold write_lock for cobj. + */ int mdd_acl_init(const struct lu_env *env, struct mdd_object *pobj, - struct mdd_object *cobj, __u32 *mode, - struct thandle *handle) + struct mdd_object *cobj, __u32 *mode, struct thandle *handle) { struct dt_object *next = mdd_object_child(pobj); struct lu_buf *buf = &mdd_env_info(env)->mti_buf; @@ -443,8 +455,11 @@ int mdd_acl_init(const struct lu_env *env, struct mdd_object *pobj, } #endif +/* + * Hold read_lock for obj. + */ static int mdd_check_acl(const struct lu_env *env, struct mdd_object *obj, - struct lu_attr* la, int mask) + struct lu_attr *la, int mask) { #ifdef CONFIG_FS_POSIX_ACL struct dt_object *next; @@ -480,9 +495,8 @@ static int mdd_check_acl(const struct lu_env *env, struct mdd_object *obj, #endif } -int __mdd_permission_internal(const struct lu_env *env, - struct mdd_object *obj, - int mask, struct lu_attr *la) +int mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, + struct lu_attr *la, int mask, int needlock) { struct md_ucred *uc = md_ucred(env); __u32 mode; @@ -518,7 +532,11 @@ int __mdd_permission_internal(const struct lu_env *env, mode >>= 6; } else { if (mode & S_IRWXG) { + if (needlock) + mdd_read_lock(env, obj); rc = mdd_check_acl(env, obj, la, mask); + if (needlock) + mdd_read_unlock(env, obj); if (rc == -EACCES) goto check_capabilities; else if ((rc != -EAGAIN) && (rc != -EOPNOTSUPP) && @@ -546,31 +564,13 @@ check_capabilities: RETURN(-EACCES); } -int mdd_permission_internal(const struct lu_env *env, struct mdd_object *obj, - int mask) -{ - return __mdd_permission_internal(env, obj, mask, NULL); -} - -inline int mdd_permission_internal_locked(const struct lu_env *env, - struct mdd_object *obj, int mask) -{ - int rc; - - mdd_read_lock(env, obj); - rc = mdd_permission_internal(env, obj, mask); - mdd_read_unlock(env, obj); - - return rc; -} - int mdd_permission(const struct lu_env *env, struct md_object *obj, int mask) { struct mdd_object *mdd_obj = md2mdd_obj(obj); int rc; ENTRY; - rc = mdd_permission_internal_locked(env, mdd_obj, mask); + rc = mdd_permission_internal(env, mdd_obj, NULL, mask, 1); RETURN(rc); } @@ -598,4 +598,3 @@ int mdd_capa_get(const struct lu_env *env, struct md_object *obj, RETURN(rc); } - -- 1.8.3.1