configfs: Rework configfs_depend_item() locking and make lockdep happy

configfs_depend_item() recursively locks all inodes mutex from configfs root to
the target item, which makes lockdep unhappy. The purpose of this recursive
locking is to ensure that the item tree can be safely parsed and that the target
item, if found, is not about to leave.

This patch reworks configfs_depend_item() locking using configfs_dirent_lock.
Since configfs_dirent_lock protects all changes to the configfs_dirent tree, and
protects tagging of items to be removed, this lock can be used instead of the
inodes mutex lock chain.
This needs that the check for dependents be done atomically with
CONFIGFS_USET_DROPPING tagging.

Now lockdep looks happy with configfs.

[ Lifted the setting of s_type into configfs_new_dirent() to satisfy the
atomic setting of CONFIGFS_USET_CREATING -- Joel ]

Signed-off-by: Louis Rilling <louis.rilling@kerlabs.com>
Signed-off-by: Joel Becker <joel.becker@oracle.com>

authored by Louis Rilling and committed by Joel Becker 420118ca e74cc06d

+47 -59
+47 -59
fs/configfs/dir.c
··· 167 167 /* 168 168 * Allocates a new configfs_dirent and links it to the parent configfs_dirent 169 169 */ 170 - static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent * parent_sd, 171 - void * element) 170 + static struct configfs_dirent *configfs_new_dirent(struct configfs_dirent *parent_sd, 171 + void *element, int type) 172 172 { 173 173 struct configfs_dirent * sd; 174 174 ··· 180 180 INIT_LIST_HEAD(&sd->s_links); 181 181 INIT_LIST_HEAD(&sd->s_children); 182 182 sd->s_element = element; 183 + sd->s_type = type; 183 184 configfs_init_dirent_depth(sd); 184 185 spin_lock(&configfs_dirent_lock); 185 186 if (parent_sd->s_type & CONFIGFS_USET_DROPPING) { ··· 226 225 { 227 226 struct configfs_dirent * sd; 228 227 229 - sd = configfs_new_dirent(parent_sd, element); 228 + sd = configfs_new_dirent(parent_sd, element, type); 230 229 if (IS_ERR(sd)) 231 230 return PTR_ERR(sd); 232 231 233 232 sd->s_mode = mode; 234 - sd->s_type = type; 235 233 sd->s_dentry = dentry; 236 234 if (dentry) { 237 235 dentry->d_fsdata = configfs_get(sd); ··· 1006 1006 * Note, btw, that this can be called at *any* time, even when a configfs 1007 1007 * subsystem isn't registered, or when configfs is loading or unloading. 1008 1008 * Just like configfs_register_subsystem(). So we take the same 1009 - * precautions. We pin the filesystem. We lock each i_mutex _in_order_ 1010 - * on our way down the tree. If we can find the target item in the 1009 + * precautions. We pin the filesystem. We lock configfs_dirent_lock. 1010 + * If we can find the target item in the 1011 1011 * configfs tree, it must be part of the subsystem tree as well, so we 1012 - * do not need the subsystem semaphore. Holding the i_mutex chain locks 1013 - * out mkdir() and rmdir(), who might be racing us. 1012 + * do not need the subsystem semaphore. Holding configfs_dirent_lock helps 1013 + * locking out mkdir() and rmdir(), who might be racing us. 1014 1014 */ 1015 1015 1016 1016 /* ··· 1023 1023 * do that so we can unlock it if we find nothing. 1024 1024 * 1025 1025 * Here we do a depth-first search of the dentry hierarchy looking for 1026 - * our object. We take i_mutex on each step of the way down. IT IS 1027 - * ESSENTIAL THAT i_mutex LOCKING IS ORDERED. If we come back up a branch, 1028 - * we'll drop the i_mutex. 1026 + * our object. 1027 + * We deliberately ignore items tagged as dropping since they are virtually 1028 + * dead, as well as items in the middle of attachment since they virtually 1029 + * do not exist yet. This completes the locking out of racing mkdir() and 1030 + * rmdir(). 1031 + * Note: subdirectories in the middle of attachment start with s_type = 1032 + * CONFIGFS_DIR|CONFIGFS_USET_CREATING set by create_dir(). When 1033 + * CONFIGFS_USET_CREATING is set, we ignore the item. The actual set of 1034 + * s_type is in configfs_new_dirent(), which has configfs_dirent_lock. 1029 1035 * 1030 - * If the target is not found, -ENOENT is bubbled up and we have released 1031 - * all locks. If the target was found, the locks will be cleared by 1032 - * configfs_depend_rollback(). 1036 + * If the target is not found, -ENOENT is bubbled up. 1033 1037 * 1034 1038 * This adds a requirement that all config_items be unique! 1035 1039 * 1036 - * This is recursive because the locking traversal is tricky. There isn't 1040 + * This is recursive. There isn't 1037 1041 * much on the stack, though, so folks that need this function - be careful 1038 1042 * about your stack! Patches will be accepted to make it iterative. 1039 1043 */ ··· 1049 1045 1050 1046 BUG_ON(!origin || !sd); 1051 1047 1052 - /* Lock this guy on the way down */ 1053 - mutex_lock(&sd->s_dentry->d_inode->i_mutex); 1054 1048 if (sd->s_element == target) /* Boo-yah */ 1055 1049 goto out; 1056 1050 1057 1051 list_for_each_entry(child_sd, &sd->s_children, s_sibling) { 1058 - if (child_sd->s_type & CONFIGFS_DIR) { 1052 + if ((child_sd->s_type & CONFIGFS_DIR) && 1053 + !(child_sd->s_type & CONFIGFS_USET_DROPPING) && 1054 + !(child_sd->s_type & CONFIGFS_USET_CREATING)) { 1059 1055 ret = configfs_depend_prep(child_sd->s_dentry, 1060 1056 target); 1061 1057 if (!ret) ··· 1064 1060 } 1065 1061 1066 1062 /* We looped all our children and didn't find target */ 1067 - mutex_unlock(&sd->s_dentry->d_inode->i_mutex); 1068 1063 ret = -ENOENT; 1069 1064 1070 1065 out: 1071 1066 return ret; 1072 - } 1073 - 1074 - /* 1075 - * This is ONLY called if configfs_depend_prep() did its job. So we can 1076 - * trust the entire path from item back up to origin. 1077 - * 1078 - * We walk backwards from item, unlocking each i_mutex. We finish by 1079 - * unlocking origin. 1080 - */ 1081 - static void configfs_depend_rollback(struct dentry *origin, 1082 - struct config_item *item) 1083 - { 1084 - struct dentry *dentry = item->ci_dentry; 1085 - 1086 - while (dentry != origin) { 1087 - mutex_unlock(&dentry->d_inode->i_mutex); 1088 - dentry = dentry->d_parent; 1089 - } 1090 - 1091 - mutex_unlock(&origin->d_inode->i_mutex); 1092 1067 } 1093 1068 1094 1069 int configfs_depend_item(struct configfs_subsystem *subsys, ··· 1110 1127 1111 1128 /* Ok, now we can trust subsys/s_item */ 1112 1129 1113 - /* Scan the tree, locking i_mutex recursively, return 0 if found */ 1130 + spin_lock(&configfs_dirent_lock); 1131 + /* Scan the tree, return 0 if found */ 1114 1132 ret = configfs_depend_prep(subsys_sd->s_dentry, target); 1115 1133 if (ret) 1116 - goto out_unlock_fs; 1134 + goto out_unlock_dirent_lock; 1117 1135 1118 - /* We hold all i_mutexes from the subsystem down to the target */ 1136 + /* 1137 + * We are sure that the item is not about to be removed by rmdir(), and 1138 + * not in the middle of attachment by mkdir(). 1139 + */ 1119 1140 p = target->ci_dentry->d_fsdata; 1120 1141 p->s_dependent_count += 1; 1121 1142 1122 - configfs_depend_rollback(subsys_sd->s_dentry, target); 1123 - 1143 + out_unlock_dirent_lock: 1144 + spin_unlock(&configfs_dirent_lock); 1124 1145 out_unlock_fs: 1125 1146 mutex_unlock(&configfs_sb->s_root->d_inode->i_mutex); 1126 1147 ··· 1149 1162 struct configfs_dirent *sd; 1150 1163 1151 1164 /* 1152 - * Since we can trust everything is pinned, we just need i_mutex 1153 - * on the item. 1165 + * Since we can trust everything is pinned, we just need 1166 + * configfs_dirent_lock. 1154 1167 */ 1155 - mutex_lock(&target->ci_dentry->d_inode->i_mutex); 1168 + spin_lock(&configfs_dirent_lock); 1156 1169 1157 1170 sd = target->ci_dentry->d_fsdata; 1158 1171 BUG_ON(sd->s_dependent_count < 1); ··· 1163 1176 * After this unlock, we cannot trust the item to stay alive! 1164 1177 * DO NOT REFERENCE item after this unlock. 1165 1178 */ 1166 - mutex_unlock(&target->ci_dentry->d_inode->i_mutex); 1179 + spin_unlock(&configfs_dirent_lock); 1167 1180 } 1168 1181 EXPORT_SYMBOL(configfs_undepend_item); 1169 1182 ··· 1363 1376 if (sd->s_type & CONFIGFS_USET_DEFAULT) 1364 1377 return -EPERM; 1365 1378 1366 - /* 1367 - * Here's where we check for dependents. We're protected by 1368 - * i_mutex. 1369 - */ 1370 - if (sd->s_dependent_count) 1371 - return -EBUSY; 1372 - 1373 1379 /* Get a working ref until we have the child */ 1374 1380 parent_item = configfs_get_config_item(dentry->d_parent); 1375 1381 subsys = to_config_group(parent_item)->cg_subsys; ··· 1386 1406 1387 1407 mutex_lock(&configfs_symlink_mutex); 1388 1408 spin_lock(&configfs_dirent_lock); 1389 - ret = configfs_detach_prep(dentry, &wait_mutex); 1390 - if (ret) 1391 - configfs_detach_rollback(dentry); 1409 + /* 1410 + * Here's where we check for dependents. We're protected by 1411 + * configfs_dirent_lock. 1412 + * If no dependent, atomically tag the item as dropping. 1413 + */ 1414 + ret = sd->s_dependent_count ? -EBUSY : 0; 1415 + if (!ret) { 1416 + ret = configfs_detach_prep(dentry, &wait_mutex); 1417 + if (ret) 1418 + configfs_detach_rollback(dentry); 1419 + } 1392 1420 spin_unlock(&configfs_dirent_lock); 1393 1421 mutex_unlock(&configfs_symlink_mutex); 1394 1422 ··· 1507 1519 */ 1508 1520 err = -ENOENT; 1509 1521 if (configfs_dirent_is_ready(parent_sd)) { 1510 - file->private_data = configfs_new_dirent(parent_sd, NULL); 1522 + file->private_data = configfs_new_dirent(parent_sd, NULL, 0); 1511 1523 if (IS_ERR(file->private_data)) 1512 1524 err = PTR_ERR(file->private_data); 1513 1525 else