(1) drop unnecessary mdd_read_lock.

(2) add some comment.
(3) cleanup the code.

diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c
index 76b1de5..3a85560 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);
 }
-