Bug 15142
| Summary: | sysfs-related lockdep warning in __blkdev_get | ||
|---|---|---|---|
| Product: | IO/Storage | Reporter: | Matti Aarnio (matti.aarnio--kernel-bugzilla) |
| Component: | MD | Assignee: | io_md |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | kernel, neilb, rjw |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.33-0.18.rc4.git7.fc13.x86_64 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
| Bug Depends on: | |||
| Bug Blocks: | 14885 | ||
| Attachments: | sysfs: differentiate between locking links and non-links for sysfs from Neil Brown, remade for -rc8 | ||
|
Description
Matti Aarnio
2010-01-25 13:38:57 UTC
This trace seems to suggest that there is a lock called 's_active' in sysfs. However the only 's_active' in sysfs is an atomic_t. It did used to be a rwsem, but that was back in 2.6.22. Can you check what patches that kernel has which are not in mainline? And find out what s_active might be? NeilBrown I think that may be an upshot of commit 846f99749ab68bbc7f75c74fec305de675b1a1bf. Thanks. I've spent a while looking at this dependency loop and other related one. This particular one I would like to *not* fix in md. The loop is reported because while writing to an attribute file in sysfs, I might delete a symlink in sysfs. This is not really a dependency loop. If the lockdep mechanism were taught do differentiate between files and symlinks in sysfs it would not detect a loop here. I will suggest an appropriate modification as below. There are other dependency issues in md which 's_active' triggers that are genuine. I have a patch ready which address those. Author: NeilBrown <neilb@suse.de> Date: Wed Feb 10 09:43:45 2010 +1100 sysfs: differentiate between locking links and non-links for sysfs symlinks and non-symlink is sysfs are very different. A symlink can never be locked (active) while an attribute modification routine is running. So removing symlink from an attribute 'store' routine should be permitted without any lockdep warnings. So split the lockdep context for 's_active' in two, one for symlinks and other for everything else. Signed-off-by: NeilBrown <neilb@suse.de> diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 699f371..e6eeaf6 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -354,7 +354,10 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) atomic_set(&sd->s_count, 1); atomic_set(&sd->s_active, 0); - sysfs_dirent_init_lockdep(sd); + if (type & SYSFS_KOBJ_LINK) + sysfs_dirent_init_lockdep(sd, "link"); + else + sysfs_dirent_init_lockdep(sd, "non_link"); sd->s_name = name; sd->s_mode = mode; diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index cdd9377..a83a02b 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -89,11 +89,11 @@ static inline unsigned int sysfs_type(struct sysfs_dirent *sd) } #ifdef CONFIG_DEBUG_LOCK_ALLOC -#define sysfs_dirent_init_lockdep(sd) \ +#define sysfs_dirent_init_lockdep(sd, type) \ do { \ static struct lock_class_key __key; \ \ - lockdep_init_map(&sd->dep_map, "s_active", &__key, 0); \ + lockdep_init_map(&sd->dep_map, "s_active_" type, &__key, 0); \ } while(0) #else #define sysfs_dirent_init_lockdep(sd) do {} while(0) lines 1-51/51 (END) I was able to reproduce the lockdep warning reliably by just issuing mdadm --stop to a raid0 array, see http://nerdbynature.de/bits/2.6.33-rc8/mdadm/ for details. With Neil's patch applied (to latest -git from Feb 16) the lockdep warning goes away. Created attachment 25085 [details]
sysfs: differentiate between locking links and non-links for sysfs from Neil Brown, remade for -rc8
Handled-By : NeilBrown <neilb@suse.de> Patch : https://bugzilla.kernel.org/attachment.cgi?id=25085 This was fixed upstream by a somewhat different approach. I think commit a2db6842873c8e5a70652f278d469128cb52db70 is probably the key part of the upstream fix. On Sun, 21 Mar 2010 at 12:38, bugzilla-daemon@bugzilla.kernel.org wrote: > What |Removed |Added > ---------------------------------------------------------------------------- > Status|NEW |RESOLVED > Resolution| |PATCH_ALREADY_AVAILABLE Indeed, a patch is available, but I fail to see if it was included into mainline. Neil: fs/sysfs/dir.c and sysfs/sysfs.h look all different now, did the patch make it into the kernel some other way? Thanks, Christian. |