From: Kit Westneat Date: Wed, 20 Apr 2016 14:12:42 +0000 (-0400) Subject: LU-5092 nodemap: users of ted_nodemap should take ref X-Git-Tag: 2.8.54~29 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=34ba26ea9884df0758f8ec8dd1047b4cf474af09 LU-5092 nodemap: users of ted_nodemap should take ref In order to quickly map clients, the client's assigned nodemap is saved on the export data struct. When mapping occurs, a reference to that nodemap is used for mapping. If no reference is taken during mapping and the nodemap is put elsewhere, then the nodemap could be destroyed before the mapping is finished. This patch ensures that a reference is taken before mapping. Signed-off-by: Kit Westneat Change-Id: I5c4d707970b1a86fb061a501240182ff3dc7be51 Reviewed-on: http://review.whamcloud.com/19674 Reviewed-by: John L. Hammond Reviewed-by: Andreas Dilger Tested-by: Jenkins Reviewed-by: James Simmons Tested-by: Maloo Reviewed-by: Oleg Drokin --- diff --git a/lustre/include/lustre_export.h b/lustre/include/lustre_export.h index be33ad1..31a1e81 100644 --- a/lustre/include/lustre_export.h +++ b/lustre/include/lustre_export.h @@ -69,7 +69,18 @@ struct tg_export_data { /** Client index in last_rcvd file */ int ted_lr_idx; - /** nodemap this export is a member of */ + /** + * ted_nodemap_lock is used to ensure that the nodemap is not destroyed + * between the time that ted_nodemap is checked for NULL, and a + * reference is taken. Modifications to ted_nodemap require that the + * active_config_lock and the nodemap(s)'s nm_member_list_lock be + * taken, as well as ted_nodemap_lock, so the export can be properly + * added to or removed from the nodemap's member list. When an export + * is added to a nodemap, a reference on that nodemap must be taken. + * That reference can be put only after ted_nodemap no longer refers to + * it. + */ + spinlock_t ted_nodemap_lock; struct lu_nodemap *ted_nodemap; struct list_head ted_nodemap_member; diff --git a/lustre/include/lustre_nodemap.h b/lustre/include/lustre_nodemap.h index e5b26f7..53cb965 100644 --- a/lustre/include/lustre_nodemap.h +++ b/lustre/include/lustre_nodemap.h @@ -135,4 +135,6 @@ struct nm_config_file *nm_config_file_register(const struct lu_env *env, struct dt_object *obj); void nm_config_file_deregister(const struct lu_env *env, struct nm_config_file *ncf); +struct lu_nodemap *nodemap_get_from_exp(struct obd_export *exp); +void nodemap_putref(struct lu_nodemap *nodemap); #endif /* _LUSTRE_NODEMAP_H */ diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c index e997be8..290b130 100644 --- a/lustre/mdt/mdt_handler.c +++ b/lustre/mdt/mdt_handler.c @@ -501,9 +501,9 @@ int mdt_pack_acl2body(struct mdt_thread_info *info, struct mdt_body *repbody, void mdt_pack_attr2body(struct mdt_thread_info *info, struct mdt_body *b, const struct lu_attr *attr, const struct lu_fid *fid) { - struct md_attr *ma = &info->mti_attr; - struct obd_export *exp = info->mti_exp; - struct lu_nodemap *nodemap = exp->exp_target_data.ted_nodemap; + struct md_attr *ma = &info->mti_attr; + struct obd_export *exp = info->mti_exp; + struct lu_nodemap *nodemap = NULL; LASSERT(ma->ma_valid & MA_INODE); @@ -527,6 +527,11 @@ void mdt_pack_attr2body(struct mdt_thread_info *info, struct mdt_body *b, b->mbo_nlink = attr->la_nlink; b->mbo_valid |= OBD_MD_FLNLINK; } + if (attr->la_valid & (LA_UID|LA_GID)) { + nodemap = nodemap_get_from_exp(exp); + if (IS_ERR(nodemap)) + goto out; + } if (attr->la_valid & LA_UID) { b->mbo_uid = nodemap_map_id(nodemap, NODEMAP_UID, NODEMAP_FS_TO_CLIENT, @@ -539,6 +544,7 @@ void mdt_pack_attr2body(struct mdt_thread_info *info, struct mdt_body *b, attr->la_gid); b->mbo_valid |= OBD_MD_FLGID; } + b->mbo_mode = attr->la_mode; if (attr->la_valid & LA_MODE) b->mbo_valid |= OBD_MD_FLMODE; @@ -588,6 +594,10 @@ void mdt_pack_attr2body(struct mdt_thread_info *info, struct mdt_body *b, if (fid != NULL && (b->mbo_valid & OBD_MD_FLSIZE)) CDEBUG(D_VFSTRACE, DFID": returning size %llu\n", PFID(fid), (unsigned long long)b->mbo_size); + +out: + if (!IS_ERR_OR_NULL(nodemap)) + nodemap_putref(nodemap); } static inline int mdt_body_has_lov(const struct lu_attr *la, @@ -915,7 +925,6 @@ static int mdt_getattr_internal(struct mdt_thread_info *info, struct mdt_body *repbody; struct lu_buf *buffer = &info->mti_buf; struct obd_export *exp = info->mti_exp; - struct lu_nodemap *nodemap = exp->exp_target_data.ted_nodemap; int rc; int is_root; ENTRY; @@ -1122,8 +1131,14 @@ static int mdt_getattr_internal(struct mdt_thread_info *info, } #ifdef CONFIG_FS_POSIX_ACL else if ((exp_connect_flags(req->rq_export) & OBD_CONNECT_ACL) && - (reqbody->mbo_valid & OBD_MD_FLACL)) + (reqbody->mbo_valid & OBD_MD_FLACL)) { + struct lu_nodemap *nodemap = nodemap_get_from_exp(exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + rc = mdt_pack_acl2body(info, repbody, o, nodemap); + nodemap_putref(nodemap); + } #endif out: @@ -2021,7 +2036,7 @@ static int mdt_quotactl(struct tgt_session_info *tsi) int id, rc; struct mdt_device *mdt = mdt_exp2dev(exp); struct lu_device *qmt = mdt->mdt_qmt_dev; - struct lu_nodemap *nodemap = exp->exp_target_data.ted_nodemap; + struct lu_nodemap *nodemap; ENTRY; oqctl = req_capsule_client_get(pill, &RMF_OBD_QUOTACTL); @@ -2032,23 +2047,27 @@ static int mdt_quotactl(struct tgt_session_info *tsi) if (rc) RETURN(err_serious(rc)); + nodemap = nodemap_get_from_exp(exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + switch (oqctl->qc_cmd) { /* master quotactl */ case Q_SETINFO: case Q_SETQUOTA: if (!nodemap_can_setquota(nodemap)) - RETURN(-EPERM); + GOTO(out_nodemap, rc = -EPERM); case Q_GETINFO: case Q_GETQUOTA: if (qmt == NULL) - RETURN(-EOPNOTSUPP); + GOTO(out_nodemap, rc = -EOPNOTSUPP); /* slave quotactl */ case Q_GETOINFO: case Q_GETOQUOTA: break; default: CERROR("Unsupported quotactl command: %d\n", oqctl->qc_cmd); - RETURN(-EFAULT); + GOTO(out_nodemap, rc = -EFAULT); } /* map uid/gid for remote client */ @@ -2060,7 +2079,7 @@ static int mdt_quotactl(struct tgt_session_info *tsi) if (unlikely(oqctl->qc_cmd != Q_GETQUOTA && oqctl->qc_cmd != Q_GETINFO)) - RETURN(-EPERM); + GOTO(out_nodemap, rc = -EPERM); if (oqctl->qc_type == USRQUOTA) id = lustre_idmap_lookup_uid(NULL, idmap, 0, @@ -2069,11 +2088,11 @@ static int mdt_quotactl(struct tgt_session_info *tsi) id = lustre_idmap_lookup_gid(NULL, idmap, 0, oqctl->qc_id); else - RETURN(-EINVAL); + GOTO(out_nodemap, rc = -EINVAL); if (id == CFS_IDMAP_NOTFOUND) { CDEBUG(D_QUOTA, "no mapping for id %u\n", oqctl->qc_id); - RETURN(-EACCES); + GOTO(out_nodemap, rc = -EACCES); } } @@ -2086,7 +2105,7 @@ static int mdt_quotactl(struct tgt_session_info *tsi) repoqc = req_capsule_server_get(pill, &RMF_OBD_QUOTACTL); if (repoqc == NULL) - RETURN(err_serious(-EFAULT)); + GOTO(out_nodemap, rc = err_serious(-EFAULT)); if (oqctl->qc_id != id) swap(oqctl->qc_id, id); @@ -2110,14 +2129,20 @@ static int mdt_quotactl(struct tgt_session_info *tsi) default: CERROR("Unsupported quotactl command: %d\n", oqctl->qc_cmd); - RETURN(-EFAULT); + GOTO(out_nodemap, rc = -EFAULT); } if (oqctl->qc_id != id) swap(oqctl->qc_id, id); *repoqc = *oqctl; - RETURN(rc); + + EXIT; + +out_nodemap: + nodemap_putref(nodemap); + + return rc; } /** clone llog ctxt from child (mdd) diff --git a/lustre/mdt/mdt_lib.c b/lustre/mdt/mdt_lib.c index b018176..1cc6603 100644 --- a/lustre/mdt/mdt_lib.c +++ b/lustre/mdt/mdt_lib.c @@ -464,20 +464,12 @@ out: } static int old_init_ucred_common(struct mdt_thread_info *info, - bool drop_fs_cap) + struct lu_nodemap *nodemap, + bool drop_fs_cap) { struct lu_ucred *uc = mdt_ucred(info); struct mdt_device *mdt = info->mti_mdt; struct md_identity *identity = NULL; - struct lu_nodemap *nodemap = - info->mti_exp->exp_target_data.ted_nodemap; - - if (nodemap == NULL) { - CDEBUG(D_SEC, "%s: cli %s/%p nodemap not set.\n", - mdt2obd_dev(mdt)->obd_name, - info->mti_exp->exp_client_uuid.uuid, info->mti_exp); - RETURN(-EACCES); - } if (!is_identity_get_disabled(mdt->mdt_identity_cache)) { identity = mdt_identity_get(mdt->mdt_identity_cache, @@ -495,7 +487,7 @@ static int old_init_ucred_common(struct mdt_thread_info *info, } uc->uc_identity = identity; - if (uc->uc_o_uid == nodemap->nm_squash_uid) { + if (nodemap && uc->uc_o_uid == nodemap->nm_squash_uid) { uc->uc_fsuid = nodemap->nm_squash_uid; uc->uc_fsgid = nodemap->nm_squash_gid; uc->uc_cap = 0; @@ -518,12 +510,15 @@ static int old_init_ucred_common(struct mdt_thread_info *info, static int old_init_ucred(struct mdt_thread_info *info, struct mdt_body *body, bool drop_fs_cap) { - struct lu_ucred *uc = mdt_ucred(info); - struct lu_nodemap *nodemap = - info->mti_exp->exp_target_data.ted_nodemap; - int rc; + struct lu_ucred *uc = mdt_ucred(info); + struct lu_nodemap *nodemap; + int rc; ENTRY; + nodemap = nodemap_get_from_exp(info->mti_exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + body->mbo_uid = nodemap_map_id(nodemap, NODEMAP_UID, NODEMAP_CLIENT_TO_FS, body->mbo_uid); body->mbo_gid = nodemap_map_id(nodemap, NODEMAP_GID, @@ -544,19 +539,23 @@ static int old_init_ucred(struct mdt_thread_info *info, uc->uc_ginfo = NULL; uc->uc_cap = body->mbo_capability; - rc = old_init_ucred_common(info, drop_fs_cap); + rc = old_init_ucred_common(info, nodemap, drop_fs_cap); + nodemap_putref(nodemap); RETURN(rc); } static int old_init_ucred_reint(struct mdt_thread_info *info) { - struct lu_ucred *uc = mdt_ucred(info); - struct lu_nodemap *nodemap = - info->mti_exp->exp_target_data.ted_nodemap; - int rc; + struct lu_ucred *uc = mdt_ucred(info); + struct lu_nodemap *nodemap; + int rc; ENTRY; + nodemap = nodemap_get_from_exp(info->mti_exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + LASSERT(uc != NULL); uc->uc_fsuid = nodemap_map_id(nodemap, NODEMAP_UID, @@ -569,7 +568,8 @@ static int old_init_ucred_reint(struct mdt_thread_info *info) uc->uc_o_gid = uc->uc_o_fsgid = uc->uc_gid = uc->uc_fsgid; uc->uc_ginfo = NULL; - rc = old_init_ucred_common(info, true); /* drop_fs_cap = true */ + rc = old_init_ucred_common(info, nodemap, true); /* drop_fs_cap=true */ + nodemap_putref(nodemap); RETURN(rc); } @@ -914,8 +914,7 @@ static int mdt_setattr_unpack_rec(struct mdt_thread_info *info) struct req_capsule *pill = info->mti_pill; struct mdt_reint_record *rr = &info->mti_rr; struct mdt_rec_setattr *rec; - struct lu_nodemap *nodemap = - info->mti_exp->exp_target_data.ted_nodemap; + struct lu_nodemap *nodemap; ENTRY; CLASSERT(sizeof(struct mdt_rec_setattr)== sizeof(struct mdt_rec_reint)); @@ -934,10 +933,17 @@ static int mdt_setattr_unpack_rec(struct mdt_thread_info *info) la->la_valid = mdt_attr_valid_xlate(rec->sa_valid, rr, ma); la->la_mode = rec->sa_mode; la->la_flags = rec->sa_attr_flags; + + nodemap = nodemap_get_from_exp(info->mti_exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + la->la_uid = nodemap_map_id(nodemap, NODEMAP_UID, NODEMAP_CLIENT_TO_FS, rec->sa_uid); la->la_gid = nodemap_map_id(nodemap, NODEMAP_GID, NODEMAP_CLIENT_TO_FS, rec->sa_gid); + nodemap_putref(nodemap); + la->la_size = rec->sa_size; la->la_blocks = rec->sa_blocks; la->la_ctime = rec->sa_ctime; diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c index de291a4..06347ff 100644 --- a/lustre/mdt/mdt_open.c +++ b/lustre/mdt/mdt_open.c @@ -550,9 +550,16 @@ static int mdt_finish_open(struct mdt_thread_info *info, } } #ifdef CONFIG_FS_POSIX_ACL - else if (exp_connect_flags(exp) & OBD_CONNECT_ACL) - rc = mdt_pack_acl2body(info, repbody, o, - exp->exp_target_data.ted_nodemap); + else if (exp_connect_flags(exp) & OBD_CONNECT_ACL) { + struct lu_nodemap *nodemap = nodemap_get_from_exp(exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + + rc = mdt_pack_acl2body(info, repbody, o, nodemap); + nodemap_putref(nodemap); + if (rc) + RETURN(rc); + } #endif /* diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c index 2dfce43..2e08a58 100644 --- a/lustre/mdt/mdt_xattr.c +++ b/lustre/mdt/mdt_xattr.c @@ -374,7 +374,6 @@ int mdt_reint_setxattr(struct mdt_thread_info *info, struct mdt_object *obj; struct md_object *child; struct obd_export *exp = info->mti_exp; - struct lu_nodemap *nodemap = exp->exp_target_data.ted_nodemap; __u64 valid = attr->la_valid; const char *xattr_name = rr->rr_name.ln_name; int xattr_len = rr->rr_eadatalen; @@ -431,13 +430,19 @@ int mdt_reint_setxattr(struct mdt_thread_info *info, } else if ((valid & OBD_MD_FLXATTR) && (strcmp(xattr_name, XATTR_NAME_ACL_ACCESS) == 0 || strcmp(xattr_name, XATTR_NAME_ACL_DEFAULT) == 0)) { + struct lu_nodemap *nodemap; /* currently lustre limit acl access size */ if (xattr_len > LUSTRE_POSIX_ACL_MAX_SIZE) GOTO(out, rc = -ERANGE); + nodemap = nodemap_get_from_exp(exp); + if (IS_ERR(nodemap)) + GOTO(out, rc = PTR_ERR(nodemap)); + rc = nodemap_map_acl(nodemap, rr->rr_eadata, xattr_len, NODEMAP_CLIENT_TO_FS); + nodemap_putref(nodemap); if (rc < 0) GOTO(out, rc); diff --git a/lustre/ofd/ofd_dev.c b/lustre/ofd/ofd_dev.c index c9de7e9..28bbf03 100644 --- a/lustre/ofd/ofd_dev.c +++ b/lustre/ofd/ofd_dev.c @@ -2209,11 +2209,10 @@ static int ofd_ladvise_hdl(struct tgt_session_info *tsi) */ static int ofd_quotactl(struct tgt_session_info *tsi) { - struct obd_quotactl *oqctl, *repoqc; - struct lu_nodemap *nodemap = - tsi->tsi_exp->exp_target_data.ted_nodemap; - int id; - int rc; + struct obd_quotactl *oqctl, *repoqc; + struct lu_nodemap *nodemap; + int id; + int rc; ENTRY; @@ -2227,6 +2226,10 @@ static int ofd_quotactl(struct tgt_session_info *tsi) *repoqc = *oqctl; + nodemap = nodemap_get_from_exp(tsi->tsi_exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); + id = repoqc->qc_id; if (oqctl->qc_type == USRQUOTA) id = nodemap_map_id(nodemap, NODEMAP_UID, @@ -2237,6 +2240,8 @@ static int ofd_quotactl(struct tgt_session_info *tsi) NODEMAP_CLIENT_TO_FS, repoqc->qc_id); + nodemap_putref(nodemap); + if (repoqc->qc_id != id) swap(repoqc->qc_id, id); diff --git a/lustre/ptlrpc/nodemap_handler.c b/lustre/ptlrpc/nodemap_handler.c index ec7dda2..28f34c9 100644 --- a/lustre/ptlrpc/nodemap_handler.c +++ b/lustre/ptlrpc/nodemap_handler.c @@ -89,7 +89,7 @@ static void nodemap_destroy(struct lu_nodemap *nodemap) /** * Functions used for the cfs_hash */ -static void nodemap_getref(struct lu_nodemap *nodemap) +void nodemap_getref(struct lu_nodemap *nodemap) { atomic_inc(&nodemap->nm_refcount); } @@ -100,12 +100,15 @@ static void nodemap_getref(struct lu_nodemap *nodemap) */ void nodemap_putref(struct lu_nodemap *nodemap) { - LASSERT(nodemap != NULL); + if (!nodemap) + return; + LASSERT(atomic_read(&nodemap->nm_refcount) > 0); if (atomic_dec_and_test(&nodemap->nm_refcount)) nodemap_destroy(nodemap); } +EXPORT_SYMBOL(nodemap_putref); static __u32 nodemap_hashfn(struct cfs_hash *hash_body, const void *key, unsigned mask) @@ -350,7 +353,7 @@ EXPORT_SYMBOL(nodemap_parse_idmap); */ int nodemap_add_member(lnet_nid_t nid, struct obd_export *exp) { - struct lu_nodemap *nodemap; + struct lu_nodemap *nodemap; int rc; mutex_lock(&active_config_lock); @@ -375,10 +378,34 @@ EXPORT_SYMBOL(nodemap_add_member); */ void nodemap_del_member(struct obd_export *exp) { - struct lu_nodemap *nodemap = exp->exp_target_data.ted_nodemap; + struct lu_nodemap *nodemap; + + ENTRY; + + /* using ac lock to prevent nodemap reclassification while deleting */ + mutex_lock(&active_config_lock); + + /* use of ted_nodemap is protected by active_config_lock. we take an + * extra reference to make sure nodemap isn't destroyed under + * active_config_lock + */ + nodemap = exp->exp_target_data.ted_nodemap; + if (nodemap == NULL) + goto out; + else + nodemap_getref(nodemap); + + mutex_lock(&nodemap->nm_member_list_lock); + nm_member_del(nodemap, exp); + mutex_unlock(&nodemap->nm_member_list_lock); + +out: + mutex_unlock(&active_config_lock); - if (nodemap != NULL) - nm_member_del(nodemap, exp); + if (nodemap) + nodemap_putref(nodemap); + + EXIT; } EXPORT_SYMBOL(nodemap_del_member); @@ -492,6 +519,50 @@ out: EXPORT_SYMBOL(nodemap_del_idmap); /** + * Get nodemap assigned to given export. Takes a reference on the nodemap. + * + * Note that this function may return either NULL, or an ERR_PTR() + * or a valid nodemap pointer. All of the functions accessing the + * returned nodemap can check IS_ERR(nodemap) to see if an error is + * returned. NULL is not considered an error, which is OK since this + * is a valid case if nodemap are not in use. All nodemap handling + * functions must check for nodemap == NULL and do nothing, and the + * nodemap returned from this function should not be dereferenced. + * + * \param export export to get nodemap for + * + * \retval pointer to nodemap on success + * \retval NULL nodemap subsystem disabled + * \retval -EACCES export does not have nodemap assigned + */ +struct lu_nodemap *nodemap_get_from_exp(struct obd_export *exp) +{ + struct lu_nodemap *nodemap; + + ENTRY; + + if (!nodemap_active) + RETURN(NULL); + + spin_lock(&exp->exp_target_data.ted_nodemap_lock); + nodemap = exp->exp_target_data.ted_nodemap; + if (nodemap) + nodemap_getref(nodemap); + spin_unlock(&exp->exp_target_data.ted_nodemap_lock); + + if (!nodemap) { + CDEBUG(D_INFO, "%s: nodemap null on export %s (at %s)\n", + exp->exp_obd->obd_name, + obd_uuid2str(&exp->exp_client_uuid), + obd_export_nid2str(exp)); + RETURN(ERR_PTR(-EACCES)); + } + + RETURN(nodemap); +} +EXPORT_SYMBOL(nodemap_get_from_exp); + +/** * mapping function for nodemap idmaps * * \param nodemap lu_nodemap structure defining nodemap @@ -523,6 +594,8 @@ __u32 nodemap_map_id(struct lu_nodemap *nodemap, struct lu_idmap *idmap = NULL; __u32 found_id; + ENTRY; + if (!nodemap_active) goto out; @@ -554,15 +627,15 @@ __u32 nodemap_map_id(struct lu_nodemap *nodemap, else found_id = idmap->id_fs; read_unlock(&nodemap->nm_idmap_lock); - return found_id; + RETURN(found_id); squash: if (id_type == NODEMAP_UID) - return nodemap->nm_squash_uid; + RETURN(nodemap->nm_squash_uid); else - return nodemap->nm_squash_gid; + RETURN(nodemap->nm_squash_gid); out: - return id; + RETURN(id); } EXPORT_SYMBOL(nodemap_map_id); @@ -986,7 +1059,7 @@ EXPORT_SYMBOL(nodemap_set_squash_gid); */ bool nodemap_can_setquota(const struct lu_nodemap *nodemap) { - return !nodemap_active || nodemap->nmf_allow_root_access; + return !nodemap_active || (nodemap && nodemap->nmf_allow_root_access); } EXPORT_SYMBOL(nodemap_can_setquota); @@ -1071,6 +1144,15 @@ int nodemap_del(const char *nodemap_name) */ lprocfs_nodemap_remove(nodemap->nm_pde_data); nodemap->nm_pde_data = NULL; + + /* reclassify all member exports from nodemap, so they put their refs */ + down_read(&active_config->nmc_range_tree_lock); + nm_member_reclassify_nodemap(nodemap); + up_read(&active_config->nmc_range_tree_lock); + + if (!list_empty(&nodemap->nm_member_list)) + CWARN("nodemap_del failed to reclassify all members\n"); + mutex_unlock(&active_config_lock); nodemap_putref(nodemap); diff --git a/lustre/ptlrpc/nodemap_internal.h b/lustre/ptlrpc/nodemap_internal.h index 991564f..d683d0d 100644 --- a/lustre/ptlrpc/nodemap_internal.h +++ b/lustre/ptlrpc/nodemap_internal.h @@ -35,6 +35,12 @@ #define DEFAULT_NODEMAP "default" +/* Turn on proc debug interface to allow OSS and + * MDS nodes to configure nodemap independently of + * MGS (since the nodemap distribution is not written + * yet */ +#define NODEMAP_PROC_DEBUG 1 + /* Default nobody uid and gid values */ #define NODEMAP_NOBODY_UID 99 @@ -192,6 +198,7 @@ int nodemap_add_range_helper(struct nodemap_config *config, struct rb_node *nm_rb_next_postorder(const struct rb_node *node); struct rb_node *nm_rb_first_postorder(const struct rb_root *root); +void nodemap_getref(struct lu_nodemap *nodemap); void nodemap_putref(struct lu_nodemap *nodemap); #define nm_rbtree_postorder_for_each_entry_safe(pos, n, \ diff --git a/lustre/ptlrpc/nodemap_lproc.c b/lustre/ptlrpc/nodemap_lproc.c index a77c478..ce47326 100644 --- a/lustre/ptlrpc/nodemap_lproc.c +++ b/lustre/ptlrpc/nodemap_lproc.c @@ -37,12 +37,6 @@ #include #include "nodemap_internal.h" -/* Turn on proc debug interface to allow OSS and - * MDS nodes to configure nodemap independently of - * MGS (since the nodemap distribution is not written - * yet */ -#define NODEMAP_PROC_DEBUG 1 - static LIST_HEAD(nodemap_pde_list); /** diff --git a/lustre/ptlrpc/nodemap_member.c b/lustre/ptlrpc/nodemap_member.c index 7ec2fd6..ce97c47 100644 --- a/lustre/ptlrpc/nodemap_member.c +++ b/lustre/ptlrpc/nodemap_member.c @@ -32,25 +32,44 @@ #define HASH_NODEMAP_MEMBER_CUR_BITS 3 #define HASH_NODEMAP_MEMBER_MAX_BITS 7 + /** - * Delete a member from a member list + * Delete an export from a nodemap's member list. Called after client + * disconnects, or during system shutdown. + * + * Requires active_config_lock and nodemap's nm_member_list_lock. * * \param nodemap nodemap containing list * \param exp export member to delete */ void nm_member_del(struct lu_nodemap *nodemap, struct obd_export *exp) { - mutex_lock(&nodemap->nm_member_list_lock); + ENTRY; + + /* because all changes to ted_nodemap are with active_config_lock */ + LASSERT(exp->exp_target_data.ted_nodemap == nodemap); + + /* protected by nm_member_list_lock */ list_del_init(&exp->exp_target_data.ted_nodemap_member); - mutex_unlock(&nodemap->nm_member_list_lock); + spin_lock(&exp->exp_target_data.ted_nodemap_lock); exp->exp_target_data.ted_nodemap = NULL; + spin_unlock(&exp->exp_target_data.ted_nodemap_lock); + + /* ref formerly held by ted_nodemap */ + nodemap_putref(nodemap); + + /* ref formerly held by ted_nodemap_member */ class_export_put(exp); + + EXIT; } /** * Delete a member list from a nodemap * + * Requires active config lock. + * * \param nodemap nodemap to remove the list from */ void nm_member_delete_list(struct lu_nodemap *nodemap) @@ -60,17 +79,16 @@ void nm_member_delete_list(struct lu_nodemap *nodemap) mutex_lock(&nodemap->nm_member_list_lock); list_for_each_entry_safe(exp, tmp, &nodemap->nm_member_list, - exp_target_data.ted_nodemap_member) { - exp->exp_target_data.ted_nodemap = NULL; - list_del_init(&exp->exp_target_data.ted_nodemap_member); - class_export_put(exp); - } + exp_target_data.ted_nodemap_member) + nm_member_del(nodemap, exp); mutex_unlock(&nodemap->nm_member_list_lock); } /** * Add a member export to a nodemap * + * Must be called under active_config_lock. + * * \param nodemap nodemap to add to * \param exp obd_export to add * \retval -EEXIST export is already part of a different nodemap @@ -78,17 +96,22 @@ void nm_member_delete_list(struct lu_nodemap *nodemap) */ int nm_member_add(struct lu_nodemap *nodemap, struct obd_export *exp) { + ENTRY; + if (exp == NULL) { CWARN("attempted to add null export to nodemap %s\n", nodemap->nm_name); - return -EINVAL; + RETURN(-EINVAL); } + mutex_lock(&nodemap->nm_member_list_lock); if (exp->exp_target_data.ted_nodemap != NULL && !list_empty(&exp->exp_target_data.ted_nodemap_member)) { + mutex_unlock(&nodemap->nm_member_list_lock); + /* export is already member of nodemap */ if (exp->exp_target_data.ted_nodemap == nodemap) - return 0; + RETURN(0); /* possibly reconnecting while about to be reclassified */ CWARN("export %p %s already hashed, failed to add to " @@ -97,17 +120,20 @@ int nm_member_add(struct lu_nodemap *nodemap, struct obd_export *exp) nodemap->nm_name, (exp->exp_target_data.ted_nodemap == NULL) ? "unknown" : exp->exp_target_data.ted_nodemap->nm_name); - return -EEXIST; + RETURN(-EEXIST); } class_export_get(exp); + nodemap_getref(nodemap); + /* ted_nodemap changes also require ac lock, member_list_lock */ + spin_lock(&exp->exp_target_data.ted_nodemap_lock); exp->exp_target_data.ted_nodemap = nodemap; - mutex_lock(&nodemap->nm_member_list_lock); + spin_unlock(&exp->exp_target_data.ted_nodemap_lock); list_add(&exp->exp_target_data.ted_nodemap_member, &nodemap->nm_member_list); mutex_unlock(&nodemap->nm_member_list_lock); - return 0; + RETURN(0); } /** @@ -144,39 +170,55 @@ void nm_member_reclassify_nodemap(struct lu_nodemap *nodemap) struct obd_export *tmp; struct lu_nodemap *new_nodemap; + ENTRY; + mutex_lock(&nodemap->nm_member_list_lock); + list_for_each_entry_safe(exp, tmp, &nodemap->nm_member_list, exp_target_data.ted_nodemap_member) { - struct ptlrpc_connection *conn = exp->exp_connection; + lnet_nid_t nid; /* if no conn assigned to this exp, reconnect will reclassify */ - if (conn) - /* nodemap_classify_nid requires nmc_range_tree_lock */ - new_nodemap = nodemap_classify_nid(conn->c_peer.nid); - else + spin_lock(&exp->exp_lock); + if (exp->exp_connection) { + nid = exp->exp_connection->c_peer.nid; + } else { + spin_unlock(&exp->exp_lock); continue; + } + spin_unlock(&exp->exp_lock); + + /* nodemap_classify_nid requires nmc_range_tree_lock */ + new_nodemap = nodemap_classify_nid(nid); if (new_nodemap != nodemap) { + /* could deadlock if new_nodemap also reclassifying, + * active_config_lock serializes reclassifies + */ + mutex_lock(&new_nodemap->nm_member_list_lock); + /* don't use member_del because ted_nodemap - * should never be null + * should never be NULL with a live export */ list_del_init(&exp->exp_target_data.ted_nodemap_member); + + /* keep the new_nodemap ref from classify */ + spin_lock(&exp->exp_target_data.ted_nodemap_lock); exp->exp_target_data.ted_nodemap = new_nodemap; + spin_unlock(&exp->exp_target_data.ted_nodemap_lock); + nodemap_putref(nodemap); - /* could deadlock if new_nodemap also reclassifying */ - mutex_lock(&new_nodemap->nm_member_list_lock); list_add(&exp->exp_target_data.ted_nodemap_member, &new_nodemap->nm_member_list); mutex_unlock(&new_nodemap->nm_member_list_lock); nm_member_exp_revoke(exp); + } else { + nodemap_putref(new_nodemap); } - - /* This put won't destroy new_nodemap because any nodemap_del - * call done on new_nodemap blocks on our active_config_lock - */ - nodemap_putref(new_nodemap); } mutex_unlock(&nodemap->nm_member_list_lock); + + EXIT; } /** diff --git a/lustre/ptlrpc/nodemap_storage.c b/lustre/ptlrpc/nodemap_storage.c index dc956a8..cd396a9 100644 --- a/lustre/ptlrpc/nodemap_storage.c +++ b/lustre/ptlrpc/nodemap_storage.c @@ -648,7 +648,6 @@ static int nodemap_load_entries(const struct lu_env *env, /* acquires active config lock */ new_config = nodemap_config_alloc(); - if (IS_ERR(new_config)) { rc = PTR_ERR(new_config); new_config = NULL; diff --git a/lustre/target/tgt_handler.c b/lustre/target/tgt_handler.c index e2b9ab3..c7157cc 100644 --- a/lustre/target/tgt_handler.c +++ b/lustre/target/tgt_handler.c @@ -255,7 +255,9 @@ static int tgt_ost_body_unpack(struct tgt_session_info *tsi, __u32 flags) if (rc) RETURN(rc); - nodemap = tsi->tsi_exp->exp_target_data.ted_nodemap; + nodemap = nodemap_get_from_exp(tsi->tsi_exp); + if (IS_ERR(nodemap)) + RETURN(PTR_ERR(nodemap)); body->oa.o_uid = nodemap_map_id(nodemap, NODEMAP_UID, NODEMAP_CLIENT_TO_FS, @@ -263,6 +265,7 @@ static int tgt_ost_body_unpack(struct tgt_session_info *tsi, __u32 flags) body->oa.o_gid = nodemap_map_id(nodemap, NODEMAP_GID, NODEMAP_CLIENT_TO_FS, body->oa.o_gid); + nodemap_putref(nodemap); tsi->tsi_ost_body = body; tsi->tsi_fid = body->oa.o_oi.oi_fid; diff --git a/lustre/target/tgt_lastrcvd.c b/lustre/target/tgt_lastrcvd.c index 74e28f8..7b0cf2b 100644 --- a/lustre/target/tgt_lastrcvd.c +++ b/lustre/target/tgt_lastrcvd.c @@ -378,6 +378,9 @@ int tgt_client_alloc(struct obd_export *exp) ENTRY; LASSERT(exp != exp->exp_obd->obd_self_export); + spin_lock_init(&exp->exp_target_data.ted_nodemap_lock); + INIT_LIST_HEAD(&exp->exp_target_data.ted_nodemap_member); + OBD_ALLOC_PTR(exp->exp_target_data.ted_lcd); if (exp->exp_target_data.ted_lcd == NULL) RETURN(-ENOMEM); diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh index 7fc381d..d9e7af8 100755 --- a/lustre/tests/sanity-sec.sh +++ b/lustre/tests/sanity-sec.sh @@ -1079,7 +1079,6 @@ do_fops_quota_test() { local qused_high=$((qused_orig + quota_fuzz)) local qused_low=$((qused_orig - quota_fuzz)) local testfile=$DIR/$tdir/$tfile - chmod 777 $DIR/$tdir $run_u dd if=/dev/zero of=$testfile bs=1M count=1 >& /dev/null sync; sync_all_data || true @@ -1222,6 +1221,22 @@ test_fops() { do_create_delete "$run_u" "$key" done + # set test dir to 777 for quota test + do_facet mgs $LCTL nodemap_modify \ + --name c$cli_i \ + --property admin \ + --value 1 + do_servers_not_mgs $LCTL set_param \ + nodemap.c$cli_i.admin_nodemap=1 + do_node $client chmod 777 $DIR/$tdir || + error unable to chmod 777 $DIR/$tdir + do_facet mgs $LCTL nodemap_modify \ + --name c$cli_i \ + --property admin \ + --value $admin + do_servers_not_mgs $LCTL set_param \ + nodemap.c$cli_i.admin_nodemap=$admin + # check quota do_fops_quota_test "$run_u" done