mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Frederic Weisbecker <fweisbec@gmail.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Jeff Mahoney <jeffm@suse.com>,
	Chris Mason <chris.mason@oracle.com>, Ingo Molnar <mingo@elte.hu>,
	Alexander Beregalov <a.beregalov@gmail.com>,
	Laurent Riffard <laurent.riffard@free.fr>
Subject: [PATCH] kill-the-bkl/reiserfs: fix reiserfs lock to cpu_add_remove_lock dependency
Date: Tue, 13 Oct 2009 00:25:02 +0200	[thread overview]
Message-ID: <1255386309-4992-2-git-send-email-fweisbec@gmail.com> (raw)
In-Reply-To: <1255386309-4992-1-git-send-email-fweisbec@gmail.com>

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: Alexander Beregalov <a.beregalov@gmail.com>
Tested-by: Alexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: Frederic 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>
---
 fs/reiserfs/journal.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c
index 04e3c42..2f8a7e7 100644
--- a/fs/reiserfs/journal.c
+++ b/fs/reiserfs/journal.c
@@ -2933,8 +2933,11 @@ int journal_init(struct super_block *sb, const char *j_dev_name,
 	}
 
 	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");
+		reiserfs_write_lock(sb);
+	}
 
 	INIT_DELAYED_WORK(&journal->j_work, flush_async_commits);
 	journal->j_work_sb = sb;
-- 
1.6.2.3


  reply	other threads:[~2009-10-12 22:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-12 22:25 [GIT PULL] tracing/kprobes: Syntax updates, introduce perf probe Frederic Weisbecker
2009-10-12 22:25 ` Frederic Weisbecker [this message]
2009-10-12 22:30   ` [PATCH] kill-the-bkl/reiserfs: fix reiserfs lock to cpu_add_remove_lock dependency Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 1/7] tracing/kprobes: Add $ prefix to special variables Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 2/7] tracing/kprobes: Remove '$ra' special variable Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 3/7] tracing/kprobes: Make special variable names more self-explainable Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 4/7] tracing/kprobes: Avoid field name confliction Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 5/7] tracing/kprobes: Robustify fixed field names against variable field names conflicts Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 6/7] perf: Add perf probe subcommand, a kprobe-event setup helper Frederic Weisbecker
2009-10-12 22:25 ` [PATCH 7/7] perf probe: Add perf probe command support without libdwarf Frederic Weisbecker
2009-10-13  6:13 ` [GIT PULL] tracing/kprobes: Syntax updates, introduce perf probe Ingo Molnar
2009-10-13  6:22   ` Ingo Molnar
2009-10-13 13:00     ` Masami Hiramatsu
2009-10-13  7:48   ` Frederic Weisbecker
2009-10-13 10:26     ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2009-08-25  2:32 [PATCH 0/4] kill-the-bkl/reiserfs: fix some lock dependency inversions Frederic Weisbecker
2009-08-26 20:13 ` Alexander Beregalov
2009-09-14 20:37   ` Frederic Weisbecker
2009-09-14 21:33     ` Alexander Beregalov
2009-09-16 20:37       ` Frederic Weisbecker
2009-09-16 23:37         ` Alexander Beregalov
2009-09-17  5:06           ` [PATCH] kill-the-bkl/reiserfs: Fix induced mm->mmap_sem to sysfs_mutex dependency Frederic Weisbecker
2009-09-22 13:55             ` Alexander Beregalov
2009-09-29  7:46               ` Frederic Weisbecker
2009-09-29 10:22                 ` Alexander Beregalov
2009-10-05 18:12                   ` [PATCH] kill-the-bkl/reiserfs: fix reiserfs lock to cpu_add_remove_lock dependency Frederic Weisbecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1255386309-4992-2-git-send-email-fweisbec@gmail.com \
    --to=fweisbec@gmail.com \
    --cc=a.beregalov@gmail.com \
    --cc=chris.mason@oracle.com \
    --cc=jeffm@suse.com \
    --cc=laurent.riffard@free.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome