LU-4973 mdd: only check links limit for non-directories

Since directory(for ldiskfs) might exceed maximum link count, so we
do not check maximum link count for dir in MDD layer.

Also add a few comments to mdd_may_xxx() to describe the input
parameters and return values.

Clean up a couple of error messages that are spewed when doing
invalid namespace operations.

Signed-off-by: Andreas Dilger <andreas.dilger@intel.com>
Signed-off-by: wang di <di.wang@intel.com>
Change-Id: I36c354f6f05f1c0fb8f632dfc23f8e2ef63ebbe5
Reviewed-on: http://review.whamcloud.com/10150
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Mike Pershin <mike.pershin@intel.com>

diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c
index da658b5..636270c 100644 (file)
--- a/lustre/mdc/mdc_locks.c
+++ b/lustre/mdc/mdc_locks.c
@@ -898,9 +898,8 @@ resend:
        mdc_put_rpc_lock(obddev->u.cli.cl_rpc_lock, it);
 
        if (rc < 0) {
-               CDEBUG_LIMIT((rc == -EACCES || rc == -EIDRM) ? D_INFO : D_ERROR,
-                            "%s: ldlm_cli_enqueue failed: rc = %d\n",
-                            obddev->obd_name, rc);
+               CDEBUG(D_INFO, "%s: ldlm_cli_enqueue failed: rc = %d\n",
+                      obddev->obd_name, rc);
 
                mdc_clear_replay_flag(req, rc);
                ptlrpc_req_finished(req);

diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c
index 9244db3..d85dfef 100644 (file)
--- a/lustre/mdd/mdd_dir.c
+++ b/lustre/mdd/mdd_dir.c
@@ -295,6 +295,20 @@ static int mdd_dir_is_empty(const struct lu_env *env,
        RETURN(result);
 }
 
+/**
+ * Determine if the target object can be hard linked, and right now it only
+ * checks if the link count reach the maximum limit. Note: for ldiskfs, the
+ * directory nlink count might exceed the maximum link count(see
+ * osd_object_ref_add), so it only check nlink for non-directories.
+ *
+ * \param[in] env      thread environment
+ * \param[in] obj      object being linked to
+ * \param[in] la       attributes of \a obj
+ *
+ * \retval             0 if \a obj can be hard linked
+ * \retval             negative error if \a obj is a directory or has too
+ *                     many links
+ */
 static int __mdd_may_link(const struct lu_env *env, struct mdd_object *obj,
                          const struct lu_attr *la)
 {
@@ -303,25 +317,31 @@ static int __mdd_may_link(const struct lu_env *env, struct mdd_object *obj,
 
        LASSERT(la != NULL);
 
-       if (!S_ISDIR(la->la_mode))
-               RETURN(0);
-
-       /*
-        * Subdir count limitation can be broken through.
-        */
-       if (la->la_nlink >= m->mdd_dt_conf.ddp_max_nlink)
+       /* Subdir count limitation can be broken through
+        * (see osd_object_ref_add), so only check non-directory here. */
+       if (!S_ISDIR(la->la_mode) &&
+           la->la_nlink >= m->mdd_dt_conf.ddp_max_nlink)
                RETURN(-EMLINK);
-       else
-               RETURN(0);
+
+       RETURN(0);
 }
 
-/*
+/**
  * Check whether it may create the cobj under the pobj.
- * cobj maybe NULL
+ *
+ * \param[in] env      execution environment
+ * \param[in] pobj     the parent directory
+ * \param[in] pattr    the attribute of the parent directory
+ * \param[in] cobj     the child to be created
+ * \param[in] check_perm       if check WRITE|EXEC permission for parent
+ *
+ * \retval             = 0 create the child under this dir is allowed
+ * \retval              negative errno create the child under this dir is
+ *                      not allowed
  */
-int mdd_may_create(const struct lu_env *env,
-                  struct mdd_object *pobj, const struct lu_attr *pattr,
-                  struct mdd_object *cobj, bool check_perm, bool check_nlink)
+int mdd_may_create(const struct lu_env *env, struct mdd_object *pobj,
+                  const struct lu_attr *pattr, struct mdd_object *cobj,
+                  bool check_perm)
 {
        struct mdd_thread_info *info = mdd_env_info(env);
        struct lu_buf   *xbuf;
@@ -351,9 +371,6 @@ int mdd_may_create(const struct lu_env *env,
                rc = mdd_permission_internal_locked(env, pobj, pattr,
                                                    MAY_WRITE | MAY_EXEC,
                                                    MOR_TGT_PARENT);
-       if (!rc && check_nlink)
-               rc = __mdd_may_link(env, pobj, pattr);
-
        RETURN(rc);
 }
 
@@ -498,9 +515,20 @@ int mdd_may_delete(const struct lu_env *env, struct mdd_object *tpobj,
        RETURN(rc);
 }
 
-/*
- * tgt maybe NULL
- * has mdd_write_lock on src already, but not on tgt yet
+/**
+ * Check whether it can create the link file(linked to @src_obj) under
+ * the target directory(@tgt_obj), and src_obj has been locked by
+ * mdd_write_lock.
+ *
+ * \param[in] env      execution environment
+ * \param[in] tgt_obj  the target directory
+ * \param[in] tattr    attributes of target directory
+ * \param[in] lname    the link name
+ * \param[in] src_obj  source object for link
+ * \param[in] cattr    attributes for source object
+ *
+ * \retval             = 0 it is allowed to create the link file under tgt_obj
+ * \retval              negative error not allowed to create the link file
  */
 static int mdd_link_sanity_check(const struct lu_env *env,
                                 struct mdd_object *tgt_obj,
@@ -531,11 +559,9 @@ static int mdd_link_sanity_check(const struct lu_env *env,
                 RETURN(-EPERM);
 
        LASSERT(src_obj != tgt_obj);
-       if (tgt_obj) {
-               rc = mdd_may_create(env, tgt_obj, tattr, NULL, true, false);
-               if (rc)
-                       RETURN(rc);
-       }
+       rc = mdd_may_create(env, tgt_obj, tattr, NULL, true);
+       if (rc != 0)
+               RETURN(rc);
 
        rc = __mdd_may_link(env, src_obj, cattr);
 
@@ -1837,7 +1863,23 @@ static int mdd_object_initialize(const struct lu_env *env,
        RETURN(rc);
 }
 
-/* has not lock on pobj yet */
+/**
+ * This function checks whether it can create a file/dir under the
+ * directory(@pobj). The directory(@pobj) is not being locked by
+ * mdd lock.
+ *
+ * \param[in] env      execution environment
+ * \param[in] pobj     the directory to create files
+ * \param[in] pattr    the attributes of the directory
+ * \param[in] lname    the name of the created file/dir
+ * \param[in] cattr    the attributes of the file/dir
+ * \param[in] spec     create specification
+ *
+ * \retval             = 0 it is allowed to create file/dir under
+ *                      the directory
+ * \retval              negative error not allowed to create file/dir
+ *                      under the directory
+ */
 static int mdd_create_sanity_check(const struct lu_env *env,
                                    struct md_object *pobj,
                                   const struct lu_attr *pattr,
@@ -1849,6 +1891,7 @@ static int mdd_create_sanity_check(const struct lu_env *env,
        struct lu_fid     *fid       = &info->mti_fid;
        struct mdd_object *obj       = md2mdd_obj(pobj);
        struct mdd_device *m         = mdo2mdd(pobj);
+       bool            check_perm = true;
        int rc;
        ENTRY;
 
@@ -1871,12 +1914,15 @@ static int mdd_create_sanity_check(const struct lu_env *env,
                                  MAY_WRITE | MAY_EXEC);
                if (rc != -ENOENT)
                        RETURN(rc ? : -EEXIST);
-       } else {
-               rc = mdd_may_create(env, obj, pattr, NULL, true, false);
-               if (rc != 0)
-                       RETURN(rc);
+
+               /* Permission is already being checked in mdd_lookup */
+               check_perm = false;
        }
 
+       rc = mdd_may_create(env, obj, pattr, NULL, check_perm);
+       if (rc != 0)
+               RETURN(rc);
+
         /* sgid check */
        if (pattr->la_mode & S_ISGID) {
                cattr->la_gid = pattr->la_gid;
@@ -2466,9 +2512,9 @@ static int mdd_rename_sanity_check(const struct lu_env *env,
         * processed in cld_rename before mdd_rename and enable
         * MDS_PERM_BYPASS).
         * So check may_create, but not check may_unlink. */
-       if (!tobj)
+       if (tobj == NULL)
                rc = mdd_may_create(env, tgt_pobj, tpattr, NULL,
-                                   (src_pobj != tgt_pobj), false);
+                                   (src_pobj != tgt_pobj));
        else
                rc = mdd_may_delete(env, tgt_pobj, tpattr, tobj, tattr, cattr,
                                    (src_pobj != tgt_pobj), 1);

diff --git a/lustre/mdd/mdd_internal.h b/lustre/mdd/mdd_internal.h
index e014ffb..ea6d87d 100644 (file)
--- a/lustre/mdd/mdd_internal.h
+++ b/lustre/mdd/mdd_internal.h
@@ -199,7 +199,7 @@ int mdd_is_subdir(const struct lu_env *env, struct md_object *mo,
                   const struct lu_fid *fid, struct lu_fid *sfid);
 int mdd_may_create(const struct lu_env *env, struct mdd_object *pobj,
                   const struct lu_attr *pattr, struct mdd_object *cobj,
-                  bool check_perm, bool check_nlink);
+                  bool check_perm);
 int mdd_may_unlink(const struct lu_env *env, struct mdd_object *pobj,
                   const struct lu_attr *pattr, const struct lu_attr *attr);
 int mdd_may_delete(const struct lu_env *env, struct mdd_object *tpobj,

diff --git a/lustre/mdd/mdd_permission.c b/lustre/mdd/mdd_permission.c
index 42b80b5..0e597ad 100644 (file)
--- a/lustre/mdd/mdd_permission.c
+++ b/lustre/mdd/mdd_permission.c
@@ -355,9 +355,8 @@ int mdd_permission(const struct lu_env *env,
        rc = mdd_permission_internal_locked(env, mdd_cobj, cattr, mask,
                                        MOR_TGT_CHILD);
 
-       if (!rc && (check_create || check_link))
-               rc = mdd_may_create(env, mdd_pobj, pattr, mdd_cobj, true,
-                                   check_link);
+       if (!rc && check_create)
+               rc = mdd_may_create(env, mdd_pobj, pattr, mdd_cobj, true);
 
        if (!rc && check_unlink)
                rc = mdd_may_unlink(env, mdd_pobj, pattr, cattr);

diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c
index f5a12f9..6c5616f 100644 (file)
--- a/lustre/mdt/mdt_xattr.c
+++ b/lustre/mdt/mdt_xattr.c
@@ -127,10 +127,8 @@ mdt_getxattr_one(struct mdt_thread_info *info,
        CDEBUG(D_INODE, "getxattr %s\n", xattr_name);
 
        rc = mo_xattr_get(info->mti_env, next, buf, xattr_name);
-       if (rc < 0) {
-               CERROR("getxattr failed: %d\n", rc);
+       if (rc < 0)
                GOTO(out, rc);
-       }
 
        if (info->mti_body->mbo_valid &
            (OBD_MD_FLRMTLSETFACL | OBD_MD_FLRMTLGETFACL))

diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c
index a3dff28..cf6bccc 100644 (file)
--- a/lustre/osd-ldiskfs/osd_handler.c
+++ b/lustre/osd-ldiskfs/osd_handler.c
@@ -2804,7 +2804,8 @@ static int osd_object_ref_add(const struct lu_env *env,
         */
        spin_lock(&obj->oo_guard);
        ldiskfs_inc_count(oh->ot_handle, inode);
-       LASSERT(inode->i_nlink <= LDISKFS_LINK_MAX);
+       if (!S_ISDIR(inode->i_mode))
+               LASSERT(inode->i_nlink <= LDISKFS_LINK_MAX);
        spin_unlock(&obj->oo_guard);
 
        ll_dirty_inode(inode, I_DIRTY_DATASYNC);

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 74f0a69..43888b0 100644 (file)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -4069,6 +4069,23 @@ test_51d() {
 }
 run_test 51d "check object distribution ===================="
 
+test_51e() {
+       if [ "$(facet_fstype $SINGLEMDS)" != ldiskfs ]; then
+               skip "Only applicable to ldiskfs-based MDTs"
+               return
+       fi
+
+       test_mkdir -c1 $DIR/$tdir       || error "create $tdir failed"
+       test_mkdir -c1 $DIR/$tdir/d0    || error "create d0 failed"
+
+       touch $DIR/$tdir/d0/foo
+       createmany -l $DIR/$tdir/d0/foo $DIR/$tdir/d0/f- 65001 &&
+               error "file exceed 65000 nlink limit!"
+       unlinkmany $DIR/$tdir/d0/f- 65001
+       return 0
+}
+run_test 51e "check file nlink limit"
+
 test_52a() {
        [ -f $DIR/$tdir/foo ] && chattr -a $DIR/$tdir/foo
        test_mkdir -p $DIR/$tdir