sysfs: Add lockdep annotations for the sysfs active reference

Holding locks over device_del -> kobject_del -> sysfs_deactivate can
cause deadlocks if those same locks are grabbed in sysfs show or store
methods.

The I model s_active count + completion as a sleeping read/write lock.
I describe to lockdep sysfs_get_active as a read_trylock,
sysfs_put_active as a read_unlock, and sysfs_deactivate as a
write_lock and write_unlock pair. This seems to capture the essence
for purposes of finding deadlocks, and in my testing gives finds real
issues and ignores non-issues.

This brings us back to holding locks over kobject_del is a problem
that ideally we should find a way of addressing, but at least lockdep
can tell us about the problems instead of requiring developers to debug
rare strange system deadlocks, that happen when sysfs files are removed
while being written to.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by Eric W. Biederman and committed by Linus Torvalds 846f9974 3e27249c

+27 -2
+12 -2
fs/sysfs/dir.c
··· 106 106 return NULL; 107 107 108 108 t = atomic_cmpxchg(&sd->s_active, v, v + 1); 109 - if (likely(t == v)) 109 + if (likely(t == v)) { 110 + rwsem_acquire_read(&sd->dep_map, 0, 1, _RET_IP_); 110 111 return sd; 112 + } 111 113 if (t < 0) 112 114 return NULL; 113 115 ··· 132 130 if (unlikely(!sd)) 133 131 return; 134 132 133 + rwsem_release(&sd->dep_map, 1, _RET_IP_); 135 134 v = atomic_dec_return(&sd->s_active); 136 135 if (likely(v != SD_DEACTIVATED_BIAS)) 137 136 return; ··· 197 194 BUG_ON(sd->s_sibling || !(sd->s_flags & SYSFS_FLAG_REMOVED)); 198 195 sd->s_sibling = (void *)&wait; 199 196 197 + rwsem_acquire(&sd->dep_map, 0, 0, _RET_IP_); 200 198 /* atomic_add_return() is a mb(), put_active() will always see 201 199 * the updated sd->s_sibling. 202 200 */ 203 201 v = atomic_add_return(SD_DEACTIVATED_BIAS, &sd->s_active); 204 202 205 - if (v != SD_DEACTIVATED_BIAS) 203 + if (v != SD_DEACTIVATED_BIAS) { 204 + lock_contended(&sd->dep_map, _RET_IP_); 206 205 wait_for_completion(&wait); 206 + } 207 207 208 208 sd->s_sibling = NULL; 209 + 210 + lock_acquired(&sd->dep_map, _RET_IP_); 211 + rwsem_release(&sd->dep_map, 1, _RET_IP_); 209 212 } 210 213 211 214 static int sysfs_alloc_ino(ino_t *pino) ··· 354 345 355 346 atomic_set(&sd->s_count, 1); 356 347 atomic_set(&sd->s_active, 0); 348 + sysfs_dirent_init_lockdep(sd); 357 349 358 350 sd->s_name = name; 359 351 sd->s_mode = mode;
+15
fs/sysfs/sysfs.h
··· 8 8 * This file is released under the GPLv2. 9 9 */ 10 10 11 + #include <linux/lockdep.h> 11 12 #include <linux/fs.h> 12 13 13 14 struct sysfs_open_dirent; ··· 51 50 struct sysfs_dirent { 52 51 atomic_t s_count; 53 52 atomic_t s_active; 53 + #ifdef CONFIG_DEBUG_LOCK_ALLOC 54 + struct lockdep_map dep_map; 55 + #endif 54 56 struct sysfs_dirent *s_parent; 55 57 struct sysfs_dirent *s_sibling; 56 58 const char *s_name; ··· 87 83 { 88 84 return sd->s_flags & SYSFS_TYPE_MASK; 89 85 } 86 + 87 + #ifdef CONFIG_DEBUG_LOCK_ALLOC 88 + #define sysfs_dirent_init_lockdep(sd) \ 89 + do { \ 90 + static struct lock_class_key __key; \ 91 + \ 92 + lockdep_init_map(&sd->dep_map, "s_active", &__key, 0); \ 93 + } while(0) 94 + #else 95 + #define sysfs_dirent_init_lockdep(sd) do {} while(0) 96 + #endif 90 97 91 98 /* 92 99 * Context structure to be used while adding/removing nodes.