From: Wang Di Date: Fri, 6 Jun 2014 08:54:33 +0000 (-0700) Subject: LU-4973 mdd: only check links limit for non-directories X-Git-Tag: 2.6.0-RC1~115 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=8d5344f3e4e2e055acbe0d6179abaf1811672557;ds=sidebyside 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 Signed-off-by: wang di Change-Id: I36c354f6f05f1c0fb8f632dfc23f8e2ef63ebbe5 Reviewed-on: http://review.whamcloud.com/10150 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Mike Pershin --- diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c index da658b5..636270c 100644 --- 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 --- 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 --- 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..0e597adc 100644 --- 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 --- 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 --- 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 --- 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