Commit 48f6ba5e authored by Frederic Weisbecker's avatar Frederic Weisbecker

kill-the-bkl/reiserfs: fix reiserfs lock to cpu_add_remove_lock dependency

While creating the reiserfs workqueue during the journal
initialization, we are holding the reiserfs lock, but
create_workqueue() also holds the cpu_add_remove_lock, creating
then the following dependency:

- reiserfs lock -> cpu_add_remove_lock

But we also have the following existing dependencies:

- mm->mmap_sem -> reiserfs lock
- cpu_add_remove_lock -> cpu_hotplug.lock -> slub_lock -> sysfs_mutex

The merged dependency chain then becomes:

- mm->mmap_sem -> reiserfs lock -> cpu_add_remove_lock ->
	cpu_hotplug.lock -> slub_lock -> sysfs_mutex

But when we fill a dir entry in sysfs_readir(), we are holding the
sysfs_mutex and we also might fault while copying the directory entry
to the user, leading to the following dependency:

- sysfs_mutex -> mm->mmap_sem

The end result is then a lock inversion between sysfs_mutex and
mm->mmap_sem, as reported in the following lockdep warning:

[ INFO: possible circular locking dependency detected ]
2.6.31-07095-g25a3912 #4
-------------------------------------------------------
udevadm/790 is trying to acquire lock:
 (&mm->mmap_sem){++++++}, at: [<c1098942>] might_fault+0x72/0xc0

but task is already holding lock:
 (sysfs_mutex){+.+.+.}, at: [<c110813c>] sysfs_readdir+0x7c/0x260

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #5 (sysfs_mutex){+.+.+.}:
      [...]

-> #4 (slub_lock){+++++.}:
      [...]

-> #3 (cpu_hotplug.lock){+.+.+.}:
      [...]

-> #2 (cpu_add_remove_lock){+.+.+.}:
      [...]

-> #1 (&REISERFS_SB(s)->lock){+.+.+.}:
      [...]

-> #0 (&mm->mmap_sem){++++++}:
      [...]

This can be fixed by relaxing the reiserfs lock while creating the
workqueue.
This is fine to relax the lock here, we just keep it around to pass
through reiserfs lock checks and for paranoid reasons.
Reported-by: default avatarAlexander Beregalov <a.beregalov@gmail.com>
Tested-by: default avatarAlexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: default avatarFrederic Weisbecker <fweisbec@gmail.com>
Cc: Jeff Mahoney <jeffm@suse.com>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Alexander Beregalov <a.beregalov@gmail.com>
Cc: Laurent Riffard <laurent.riffard@free.fr>
parent 193be0ee
...@@ -2933,8 +2933,11 @@ int journal_init(struct super_block *sb, const char *j_dev_name, ...@@ -2933,8 +2933,11 @@ int journal_init(struct super_block *sb, const char *j_dev_name,
} }
reiserfs_mounted_fs_count++; reiserfs_mounted_fs_count++;
if (reiserfs_mounted_fs_count <= 1) if (reiserfs_mounted_fs_count <= 1) {
reiserfs_write_unlock(sb);
commit_wq = create_workqueue("reiserfs"); commit_wq = create_workqueue("reiserfs");
reiserfs_write_lock(sb);
}
INIT_DELAYED_WORK(&journal->j_work, flush_async_commits); INIT_DELAYED_WORK(&journal->j_work, flush_async_commits);
journal->j_work_sb = sb; journal->j_work_sb = sb;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment